fix(linter): add export {} when noUnusedImports removes last import in TypeScript#8977
Conversation
🦋 Changeset detectedLatest commit: 09109f8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThe noUnusedImports lint rule now preserves TypeScript module boundaries when removing imports. If removing an unused import would leave a TypeScript file with no imports or exports (and the file is not an embedded script in Vue/Svelte/Astro), the fixer replaces the removed import with an empty export clause ( Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs (1)
431-493:⚠️ Potential issue | 🟠 MajorDouble mutation conflict on
parentin TypeScript imports.When a file has a single import with leading blank lines and no siblings (triggering the blank-line-stripping path at line 423), the code removes the
parentimport statement and replaces the entire module with an empty one. However, theneeds_export_emptylogic that follows (line 452) then attempts to mutateparentagain at line 478 or 483. The mutation system's "last write wins" semantics means the second operation overwrites the first, defeating the blank-line handling.Either guard the
needs_export_emptyblock to skip it after the blank-line path handles removal, or reorganise to evaluateneeds_export_emptybefore the blank-line logic to decide the strategy upfront.
Merging this PR will not alter performance
Comparing Footnotes
|
…n TS
When noUnusedImports removes the last import in a TypeScript file that
has no other exports, the file becomes an ambient module (global scope).
This replaces the removed import with export {} to preserve module
context.
Skip this behavior for embedded scripts in Vue, Svelte, and Astro files,
which are already in a module context by nature of being inside <script>
tags.
9abad98 to
09109f8
Compare
|
Thanks for the review! I've pushed an update that:
Regarding the double mutation concern from CodeRabbit: the blank-line-stripping path (line ~423) only triggers when there's a |
Update test expected output files to match Biome 2.3.15 behavior.
Changes in Biome 2.3.15:
- noUnusedImports now adds "export {}" when removing the last import
in a TypeScript file to prevent it from becoming an ambient module
(biomejs/biome#8977)
Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Summary
Fixes #4888. When
noUnusedImportsremoves the last import statement in a TypeScript file that has no other exports, the file becomes an ambient module (global scope). This fix replaces the removed import withexport {}to preserve module context.This does not apply to embedded scripts in Vue, Svelte, or Astro files, which are already in a module context.
Test Plan
issue_4888.tstest case with a TS file containing only one unused import and adeclare moduleblockexport {}instead of just removing the importnoUnusedImportstests continue to passDocs
No documentation changes needed — the rule's existing documentation covers the behavior.