fix: improve invalid annotation warnings#10185
Conversation
✅ Deploy Preview for rolldown-rs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
This PR improves the UX around INVALID_ANNOTATION (misplaced /*#__PURE__*/ / /* @__PURE__ */) diagnostics by making them render as warnings and by default only surfacing them for actionable, local project files (inside cwd, outside node_modules), while keeping dependencies / outside-cwd / virtual ids quiet unless explicitly opted in.
Changes:
- Render
INVALID_ANNOTATIONdiagnostics with warning severity and update help text to be more concise + actionable (including how to disable). - Implement a conservative default: warn only for local project files; treat
checks.invalidAnnotation: trueas “warn for all modules”, andfalseas disabling the check. - Add regression coverage (snapshots + integration fixtures + a targeted issue test for #10183).
Reviewed changes
Copilot reviewed 25 out of 28 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tasks/generator/src/generators/checks.rs | Updates generator to express the special default behavior for invalidAnnotation in generated docs. |
| packages/rolldown/src/options/generated/checks-options.ts | Documents the conservative default and explicit opt-in/disable semantics for checks.invalidAnnotation. |
| crates/rolldown/tests/rolldown/warnings/invalid_pure_annotation_var_declarator/artifacts.snap | Updates snapshot to reflect new concise help text formatting. |
| crates/rolldown/tests/rolldown/warnings/invalid_pure_annotation_statement/artifacts.snap | Updates snapshot to reflect new concise help text formatting. |
| crates/rolldown/tests/rolldown/warnings/invalid_pure_annotation_node_modules_no_warning/node_modules/dep/index.js | Adds dependency fixture demonstrating default suppression in node_modules. |
| crates/rolldown/tests/rolldown/warnings/invalid_pure_annotation_node_modules_no_warning/main.js | Adds entry fixture for the default-suppression test. |
| crates/rolldown/tests/rolldown/warnings/invalid_pure_annotation_node_modules_no_warning/_config.json | Adds config asserting no warning by default for dependency invalid annotations. |
| crates/rolldown/tests/rolldown/warnings/invalid_pure_annotation_node_modules_explicit_warning/node_modules/dep/index.js | Adds dependency fixture for explicit opt-in warning behavior. |
| crates/rolldown/tests/rolldown/warnings/invalid_pure_annotation_node_modules_explicit_warning/main.js | Adds entry fixture for explicit opt-in warning behavior. |
| crates/rolldown/tests/rolldown/warnings/invalid_pure_annotation_node_modules_explicit_warning/artifacts.snap | Adds snapshot asserting warning is emitted when explicitly enabled. |
| crates/rolldown/tests/rolldown/warnings/invalid_pure_annotation_node_modules_explicit_warning/_config.json | Adds config enabling checks.invalidAnnotation: true and expecting warnings. |
| crates/rolldown/tests/rolldown/warnings/invalid_pure_annotation_nested_function_declaration/artifacts.snap | Updates snapshot to reflect new concise help text formatting. |
| crates/rolldown/tests/rolldown/warnings/invalid_pure_annotation_expression/artifacts.snap | Updates snapshot to reflect new concise help text formatting. |
| crates/rolldown/tests/rolldown/warnings/invalid_pure_annotation_disabled/main.js | Adds fixture for explicit disable behavior. |
| crates/rolldown/tests/rolldown/warnings/invalid_pure_annotation_disabled/_config.json | Adds config enabling checks.invalidAnnotation: false and expecting no warnings. |
| crates/rolldown/tests/rolldown/warnings/invalid_pure_annotation_at_variant/artifacts.snap | Updates snapshot to reflect new concise help text formatting. |
| crates/rolldown/tests/rolldown/warnings/invalid_pure_annotation_alongside_valid/artifacts.snap | Updates snapshot to reflect new concise help text formatting. |
| crates/rolldown/tests/rolldown/issues/mod.rs | Registers the new regression test module for issue #10183. |
| crates/rolldown/tests/rolldown/issues/10183/project/main.js | Adds project-side entry fixture used to verify outside-cwd suppression. |
| crates/rolldown/tests/rolldown/issues/10183/mod.rs | Adds regression tests for default suppression vs explicit opt-in behavior (issue #10183). |
| crates/rolldown/tests/rolldown/issues/10183/dependency.js | Adds outside-cwd dependency fixture that triggers invalid annotation. |
| crates/rolldown/tests/esbuild/dce/remove_unused_pure_comment_calls/artifacts.snap | Updates snapshots to reflect new concise help text formatting. |
| crates/rolldown/src/utils/prepare_build_context.rs | Plumbs the “warn on all invalid annotations” opt-in into normalized options. |
| crates/rolldown/src/utils/pre_process_ecma_ast.rs | Gates invalid-annotation warning generation and forces warning severity. |
| crates/rolldown/src/utils/parse_to_ecma_ast.rs | Implements local-project-file classification (cwd + node_modules filtering) and opt-in semantics. |
| crates/rolldown_error/src/types/event_kind.rs | Documents the conservative default behavior on the Rust-side event kind docs. |
| crates/rolldown_error/src/build_diagnostic/events/invalid_annotation.rs | Updates diagnostic help text and adds a test for concise rendering. |
| crates/rolldown_common/src/inner_bundler_options/types/normalized_bundler_options.rs | Adds normalized option flag to support “explicit true => warn for all modules”. |
This is the only thing that feels a bit too implicit to me. If we have 3 modes (all, only "project-related", none) then we should consider expressing it that way (while keeping true/false as legacy options). |
Merging this PR will not alter performance
Comparing Footnotes
|
|
Merge activity
|
## Summary - render `INVALID_ANNOTATION` diagnostics with warning severity so they use warning colors - warn by default only for real local project files inside `cwd` and outside `node_modules` - keep dependencies, outside-`cwd` files, and virtual IDs quiet by default so builds do not report warnings users cannot act on - keep `checks.invalidAnnotation` as a normal boolean: omitted/`true` use the local project scope, while `false` disables the check - normalize parent-directory components and match `node_modules` case-insensitively to avoid false-positive local classification - show concise help for correct annotation placement and disabling the check - document the conservative default and add regression coverage ## Test plan - `cargo test -p rolldown --lib` - `cargo test -p rolldown_error` - `NEEDS_EXTENDED=false cargo test -p rolldown --test integration invalid_pure_annotation -- --ignored --nocapture` - `cargo test -p rolldown --test integration 10183 -- --nocapture` - `just t-run crates/rolldown/tests/esbuild/dce/remove_unused_pure_comment_calls/_config.json` - `cargo clippy -p rolldown -p rolldown_error -p generator --all-targets -- --deny warnings` - `just build-rolldown` - `just lint-node` - `just lint-repo` - end-to-end Node API check for the conservative default, boolean enable/disable behavior, yellow output, and concise help Fixes #10183
689d58a to
bb66570
Compare
## [1.1.5] - 2026-07-08 ### 🚀 Features - detect top-level import-binding reads as execution-order sensitive (#10180) by @hyf0 - sourcemap_filenames: add a sourcemapFileNames option (#9271) by @V1OL3TF0X - binding: record plugin hook result kind in tracing spans (#10154) by @IWANABETHATGUY - linking: skip side-effect-free modules in per-entry reachability (#10111) by @IWANABETHATGUY - improve error message for unresolved virtual imports (#10156) by @sapphi-red - add descriptive metadata to plugin API (#10106) by @sapphi-red - add `--configLoader=native` option (#10118) by @sapphi-red ### 🐛 Bug Fixes - improve invalid annotation warnings (#10185) by @hyf0 - keep deduplicated asset filenames stable once they can be observed (#10191) by @shulaoda - sourcemap_filenames: use public option name in pattern errors (#10188) by @IWANABETHATGUY - sourcemap_filenames: hash prepared sourcemap content (#10178) by @hyf0 - tree-shake unused circular declarators exported via export list (#10166) by @IWANABETHATGUY - dev: don't panic when an HMR rebuild hits an unresolved import (#10162) by @shulaoda - propagate errors from output.globals function (#9880) by @shulaoda - dev: revert cache mutations when a partial scan fails (#10110) by @shulaoda - dev: update importer relationships of cached modules in incremental build (#10107) by @shulaoda - hmr: fall back to full reload when a changed module is not registered as executed (#10132) by @shulaoda - chunk-optimizer: follow entry facade edges in runtime placement cycle check (#10101) by @hyf0 - dev: ignore watcher events after close (#10113) by @hyf0 - emit async wrapper for TLA modules under onDemandWrapping (#10086) by @IWANABETHATGUY - gate sideEffects:false modules' side effects on body demand (#10080) by @IWANABETHATGUY - rolldown_plugin_vite_resolve: return empty object for `browser: false` mapped modules (#10082) by @sapphi-red - reset the word-boundary state on newline in Hires::Boundary sourcemaps (#10025) by @shulaoda - trim an emptied chunk's outro/intro instead of skipping past it (#10029) by @shulaoda - test each edited chunk's own start against indent exclude ranges (#10026) by @shulaoda - preserve sourcemap mappings for indented lines when a CJS module shares the chunk (#10074) by @hyf0 ### 🚜 Refactor - separate tree-shaking side effects from execution order sensitivity (#10168) by @hyf0 - type construct_vite_preload_call to take an ObjectPattern (#10135) by @shulaoda - treeshake: single-source the own-export classification shared with the lazy-barrel loader (#10098) by @IWANABETHATGUY - dev: reuse Vite's bundledDev server (#10081) by @h-a-n-a - clippy: ban std HashMap/HashSet in favour of FxHashMap/FxHashSet (#10108) by @Boshen - treeshake: make body demand a second module bit instead of a stmt multimap (#10097) by @IWANABETHATGUY - seal used_symbol_refs by construction after its last writer (#10091) by @hyf0 - treeshake: replace inclusion mutual recursion with a worklist engine (#10096) by @IWANABETHATGUY - treeshake: split include_statements.rs into focused modules (#10095) by @IWANABETHATGUY - drop redundant is_user_defined filter on partitioned entries (#10050) by @shulaoda - project the retained export interface out of used_symbol_refs (#10089) by @hyf0 - track used external symbols separately from used_symbol_refs (#10088) by @hyf0 - make module namespace inclusion an explicit linking metadata field (#10087) by @hyf0 - rename statement evaluation metadata (#10078) by @hyf0 ### 📚 Documentation - virtual modules user-facing id convention (#10155) by @sapphi-red - cli: clarify disabling boolean/object flags like codeSplitting (#10153) by @IWANABETHATGUY - chore: remove Vite+ alpha banner (#10105) by @mdong1909 - write down the used_symbol_refs contract (#10090) by @hyf0 - dev/lazy: update design and implementation (#10079) by @h-a-n-a ### ⚡ Performance - ast_scanner: stop order-sensitivity checks once a module is flagged (#10190) by @IWANABETHATGUY - return impl ExactSizeIterator from slice-backed accessors (#10133) by @Boshen - binding: box dev and watcher napi futures (#10103) by @Boshen ### 🧪 Testing - move string_wizard replace unit tests to the JS magic-string suite (#10176) by @IWANABETHATGUY - dev: assert incremental scan state matches a fresh full build after each HMR step (#10115) by @shulaoda - dev: restore runtime assertions of delete_file_not_used_anymore (#10112) by @shulaoda - dev: fix flaky dev server tests in CI (#10152) by @h-a-n-a - add regression test for #10099 (lazyBarrel drops default-import binding but keeps its property reads) (#10109) by @IWANABETHATGUY ### ⚙️ Miscellaneous Tasks - deploy website to Void via GitHub OIDC (#10192) by @Boshen - deps: update oxc to 0.139.0 (#10161) by @shulaoda - deps: update test262 submodule for tests (#10160) by @rolldown-guard[bot] - rolldown_plugin_utils: remove dead asset-url and css scaffolding (#10131) by @shulaoda - deps: revert vite-plus to v0.2.1 (#10148) by @shulaoda - deps: update github actions (#10141) by @renovate[bot] - deps: update dependency rust to v1.96.1 (#10145) by @renovate[bot] - deps: update npm packages (#10142) by @renovate[bot] - deps: update rust crates (#10143) by @renovate[bot] - deps: update napi to v3.10.3 (#10121) by @renovate[bot] - rolldown_utils: remove unused time module (#10138) by @shulaoda - remove dead CopyModulePlugin::is_active method (#10129) by @shulaoda - remove dead LazyCompilationContext::is_lazy_module method (#10128) by @shulaoda - remove dead BuildDiagnostic::downcast_ref method (#10127) by @shulaoda - deps: update dependency vite-plus to v0.2.2 (#10084) by @renovate[bot] - deps: update rust crate oxc_sourcemap to v8.1.0 (#10122) by @renovate[bot] - deps: update crate-ci/typos action to v1.48.0 (#10124) by @renovate[bot] - enable more clippy restriction lints (#10114) by @Boshen - deps: update rust dependencies (#10100) by @Boshen - deps: update oxc resolver to v11.23.0 (#10083) by @renovate[bot] ###◀️ Revert - Revert "chore(deps): revert vite-plus to v0.2.1" (#10157) by @h-a-n-a - "fix(hmr): fall back to full reload when a changed module is not registered as executed (#10132)" (#10151) by @shulaoda ### ❤️ New Contributors * @V1OL3TF0X made their first contribution in [#9271](#9271) Co-authored-by: shulaoda <[email protected]>
Summary
INVALID_ANNOTATIONdiagnostics with warning severity so they use warning colorscwdand outsidenode_modulescwdfiles, and virtual IDs quiet by default so builds do not report warnings users cannot act onchecks.invalidAnnotationas a normal boolean: omitted/trueuse the local project scope, whilefalsedisables the checknode_modulescase-insensitively to avoid false-positive local classificationTest plan
cargo test -p rolldown --libcargo test -p rolldown_errorNEEDS_EXTENDED=false cargo test -p rolldown --test integration invalid_pure_annotation -- --ignored --nocapturecargo test -p rolldown --test integration 10183 -- --nocapturejust t-run crates/rolldown/tests/esbuild/dce/remove_unused_pure_comment_calls/_config.jsoncargo clippy -p rolldown -p rolldown_error -p generator --all-targets -- --deny warningsjust build-rolldownjust lint-nodejust lint-repoFixes #10183