Warnings when using Vite 5 / Rollup 4

[elliots]

Reproduction

None

Describe the bug

Using vite 5, rollup 4 is complaining about some invalid @__NO_SIDE_EFFECTS__ comments

node_modules/@formkit/inputs/dist/index.mjs (276:0) A comment

"/**
 * Actions section that shows the action buttons
 *
 * @public
 * @__NO_SIDE_EFFECTS__
 */"

in "node_modules/@formkit/inputs/dist/index.mjs" contains an annotation that Rollup cannot interpret due to the position of the comment. The comment will be removed to avoid issues.
node_modules/@formkit/inputs/dist/index.mjs (287:0) A comment

"/**
 * Box section used for grouping options
 *
 * @public
 * @__NO_SIDE_EFFECTS__
 */"

in "node_modules/@formkit/inputs/dist/index.mjs" contains an annotation that Rollup cannot interpret due to the position of the comment. The comment will be removed to avoid issues.
node_modules/@formkit/inputs/dist/index.mjs (321:0) A comment

"/**
 * Option help section
 *
 * @public
 * @__NO_SIDE_EFFECTS__
 */"

in "node_modules/@formkit/inputs/dist/index.mjs" contains an annotation that Rollup cannot interpret due to the position of the comment. The comment will be removed to avoid issues.
node_modules/@formkit/inputs/dist/index.mjs (335:0) A comment

<snip>

They all seem to be when assigning a returned function to a const

e.g.

const textareaInput = createSection('input', () => ({

According to https://github.com/javascript-compiler-hints/compiler-notations-spec/blob/main/no-side-effects-notation-spec.md#2-const-variable-declaration maybe that's not supported? Happy to add a PR to remove them if that's the case, or maybe there's a better solution?

It is only a warning, and it does continue compiling.

Environment

• OS: N/A
• Browser: N/A
• Version: @formkit/inputs 1.3.0

[justin-schroeder]

Interesting. I was under the impression from antfu’s PR against rollup that it was supported in JSDoc/TSDoc style:

rollup/rollup#5024

We certainly want to mark these as side effect free if possible for better tree shaking. I’m happy for any further research/assistance in this area!

[fenilli]

@justin-schroeder maybe it needs a line after @public or it can't be after another @ comment, worth taking a look if its giving warnings.

It also has this comment rollup/rollup#5024 (comment)
It might be because of this?

[g1eny0ung]

I tried to investigate why these warnings occurred and found the doc says:

Such an annotation is considered valid if it directly precedes a function declaration or a constant variable declaration where the first declared variable is a function and is only separated from the declaration by white-space or comments.

I guess the reason is that even though createSection returns a function, it's a deep function call, and rollup cannot realize it with static analysis, it will treat this call as a constant declaration. I also tested in a real-build, updating a createSection() call to () => createSection()(), and then the related warning disappeared.

Code:

Shell output:

So, updating all sections with a pure function wrapper (() => createSection()()) may not be the best solution, but it is one possibility. If this verbose approach is acceptable, I think I'd like to create a PR to fix it. @justin-schroeder Have any suggestions?

Ref

In rollup source code, https://github.com/rollup/rollup/blob/0b619fd737c71822878c91657f471305ebb19e73/src/utils/convert-ast.ts#L1234, you can see AnnotationType‎ is not included in some custom types like VariableDeclaration, which also proves that the documentation.

[justin-schroeder]

Wow, great research @g1eny0ung 👍

It’s a bit annoying to need to include a dummy function call in there, but based on your looking into it that seems like the best approach at the moment, so yes — I would accept a PR for that ❤️

[g1eny0ung]

Ok, I will try to do it.

[g1eny0ung]

@justin-schroeder I've been thinking about this some more, and it seems like I'm complicating the issue because I saw the createSection already be marked as no side effect with /*@__NO_SIDE_EFFECTS__*/. So all calls of createSection will turn to side-effect free. At this point, we can already make each section completely side-effect free (because each section is a variable declaration). For example:

export const box = createSection(/* ... */)

// If we don't call `box()` anymore, the above declaration will be cut to:

createSection(/* ... */)

// Due to the `createSection` being a pure function, this declaration will be fully cut.

Finally, I just remove all @__NO_SIDE_EFFECTS__ comments, and update the rollup config (PluginPure) to add /* #__PURE__ */ before all calls of createSection for backward compatibility (rollup < v4).

PR: #1069

[justin-schroeder]

Confirmed that @g1eny0ung’s fix on the @next tag does resolve these errors.