rolldown
Remove improper jsdoc of builtin define plugin
Remove lines at
https://github.com/rolldown/rolldown/blob/d53650c52948c411f2aaa9df535e8774f8bfa661/packages/rolldown/src/builtin-plugin/replace-plugin.ts#L13
https://github.com/rolldown/rolldown/blob/d53650c52948c411f2aaa9df535e8774f8bfa661/packages/rolldown/src/builtin-plugin/replace-plugin.ts#L22
---old
While the Rollup plugin supports not only plain strings but also functions returning a string (id as first argument) and the example JSdoc also indicate that, the rolldown plugin will fail when a function is being passed.
Either support it (would be preferred for feature parity I suppose) or make clear that functions are not supported.
Dig a little bit. I'm not sure performance influence. I looked at the source code of rollup define plugin, each passing function would be called per file. As I was writting here, I think it's not suggested to support this featrue.
So this issue becomes a issue to fix wrong jsdoc.
@TheAlexLichter I think removing the jsdoc is enough. The types should be able to tell that function is not supported.
@hyf0 Thanks for digging! Then we should also state the functional difference between rolldown & rollup replace plugin.
Do you have a suggestion how "migration" would look like for people using function calls? Not sure how common that is (I can check), but here is an example of Nitro using it.
@TheAlexLichter For your example, I don't really see a need to use function there. The point of using function is to delay the excution and return value dynamically based on outside variable or arguments.
Then we should also state the functional difference between rolldown & rollup replace plugin.
Do you mean adding a line in the jsdoc or something else?
Do you have a suggestion how "migration" would look like for people using function calls?
I mean you could always fallback to write your own define plugin or use rollup's directly. However, if this turns to be most requested feature, supporting it won't be technical problem. It's just manual work.
@TheAlexLichter For your example, I don't really see a need to use function there. The point of using function is to delay the excution and return value dynamically based on outside variable or arguments.
Maybe @pi0 has some insight on why he used a function there 🤔
I think the idea was indeed to delay execution later on to ensure that RUNTIME_CONFIG can be altered until being inserted.
Then we should also state the functional difference between rolldown & rollup replace plugin.
Do you mean adding a line in the jsdoc or something else?
No, I think that's fine as a comment in the docs. I'm planning a page (or part of a page) on rollup plugin compat so I can denote it there.
Do you have a suggestion how "migration" would look like for people using function calls?
I mean you could always fallback to write your own define plugin or use rollup's directly. However, if this turns to be most requested feature, supporting it won't be technical problem. It's just manual work.
Fair! Then we should be good on that 👍🏻
Nitro needs it indeed for dynamic updates (in dev, we change value of runtimeconfig)
But not a big deal we can implement runtime config differently having knowledge of this.
