Skip to content

fix: improve invalid annotation warnings#10185

Merged
graphite-app[bot] merged 1 commit into
mainfrom
fix/invalid-annotation-noise
Jul 8, 2026
Merged

fix: improve invalid annotation warnings#10185
graphite-app[bot] merged 1 commit into
mainfrom
fix/invalid-annotation-noise

Conversation

@hyfdev

@hyfdev hyfdev commented Jul 8, 2026

Copy link
Copy Markdown
Member

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

@netlify

netlify Bot commented Jul 8, 2026

Copy link
Copy Markdown

Deploy Preview for rolldown-rs ready!

Name Link
🔨 Latest commit bb66570
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/6a4e0b3ad276830008604faf
😎 Deploy Preview https://deploy-preview-10185--rolldown-rs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@hyfdev
hyfdev marked this pull request as ready for review July 8, 2026 06:16
@hyfdev
hyfdev requested a review from IWANABETHATGUY as a code owner July 8, 2026 06:16
Copilot AI review requested due to automatic review settings July 8, 2026 06:16
@hyfdev
hyfdev requested a review from shulaoda as a code owner July 8, 2026 06:16
@hyfdev
hyfdev requested a review from TheAlexLichter July 8, 2026 06:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_ANNOTATION diagnostics 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: true as “warn for all modules”, and false as 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”.

@TheAlexLichter

Copy link
Copy Markdown
Collaborator

treat explicit checks.invalidAnnotation: true as opting into all modules, while false disables the check

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).

@codspeed-hq

codspeed-hq Bot commented Jul 8, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 7 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing fix/invalid-annotation-noise (27c5e78) with main (460035d)

Open in CodSpeed

Footnotes

  1. 10 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@hyfdev

hyfdev commented Jul 8, 2026

Copy link
Copy Markdown
Member Author
  • Removed the implicit third mode instead of expanding the API for now.
  • Unset / true: local project files only.
  • false: disabled.
  • An explicit all-modules mode can be considered separately if needed later.

hyfdev commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

Merge activity

Comment thread crates/rolldown/src/utils/parse_to_ecma_ast.rs Outdated
Comment thread crates/rolldown/src/utils/parse_to_ecma_ast.rs Outdated
## 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
@graphite-app
graphite-app Bot force-pushed the fix/invalid-annotation-noise branch from 689d58a to bb66570 Compare July 8, 2026 08:32
@graphite-app
graphite-app Bot merged commit bb66570 into main Jul 8, 2026
33 of 34 checks passed
@graphite-app
graphite-app Bot deleted the fix/invalid-annotation-noise branch July 8, 2026 08:37
@rolldown-guard rolldown-guard Bot mentioned this pull request Jul 8, 2026
shulaoda added a commit that referenced this pull request Jul 8, 2026
## [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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Warnings for invalid pure annotations in node_modules are too noisy and look like errors

4 participants