fix: consider type-only imports as dev dependency #7171
Conversation
🦋 Changeset detectedLatest commit: 0ae5ff0 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 |
WalkthroughThis change updates the Assessment against linked issues
Suggested labels
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies/valid.ts (1)
41-42: Great addition to validate type-only import handling.This cleanly asserts the rule’s behaviour without coupling to specific exports. Consider adding a second variant (e.g.
import type * as ns from "type-fest";) in a follow-up to broaden coverage..changeset/two-badgers-cut.md (1)
9-9: Optional: tidy punctuation to match writing guidelines.The label lines end with a colon; the style guide asks for full stops. If you want to appease linters, swap the trailing colons for full stops.
Also applies to: 18-18
crates/biome_js_analyze/src/lint/correctness/no_undeclared_dependencies.rs (3)
45-50: Doc example is helpful; please clarify the option interaction.State explicitly that type-only imports are allowed even when
devDependenciesis disabled via rule options (if that’s the intended behaviour).Example tweak:
/// If you have declared `type-fest` in the `devDependencies` section: @@ /// import type { SetRequired } from "type-fest"; /// ``` +/// +/// Note: Type-only imports are always permitted and treated as `devDependencies`, +/// regardless of the `devDependencies` option for this rule.
147-158: Reuse the helper for consistency.You can use
is_type_import(node)here rather than duplicatingtype_token()access, improving cohesion.- if let Some(import_clause) = node.parent::<AnyJsImportClause>() - && import_clause.type_token().is_some() + if is_type_import(node) { let package_name = format!("@types/{package_name}");
269-274: Helper reads well; minor naming nit.Consider
is_type_only_importfor precision, but not a blocker.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies/valid.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (4)
.changeset/two-badgers-cut.md(1 hunks)crates/biome_js_analyze/src/lint/correctness/no_undeclared_dependencies.rs(3 hunks)crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies/valid.package.json(1 hunks)crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies/valid.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
crates/biome_*/**/*
📄 CodeRabbit Inference Engine (CLAUDE.md)
Core crates must be located in
/crates/biome_*/
Files:
crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies/valid.tscrates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies/valid.package.jsoncrates/biome_js_analyze/src/lint/correctness/no_undeclared_dependencies.rs
.changeset/*.md
📄 CodeRabbit Inference Engine (CONTRIBUTING.md)
.changeset/*.md: When creating a changeset, usejust new-changesetand do not create changeset files manually. Changesets are stored in the.changesetfolder.
Changeset descriptions should be user-facing, use past or present tense as appropriate, reference issues and rules with links, include code blocks when applicable, and end every sentence with a full stop.
Files:
.changeset/two-badgers-cut.md
**/*.{rs,toml}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Format code (Rust + TOML) using
just formatFormat Rust and TOML files using
just f(alias forjust format).
Files:
crates/biome_js_analyze/src/lint/correctness/no_undeclared_dependencies.rs
**/*.rs
📄 CodeRabbit Inference Engine (CONTRIBUTING.md)
**/*.rs: Use Rust's doctest feature by writing code examples in doc comments, which are run during the testing phase.
Use theinstacrate for snapshot testing in Rust, and manage snapshots withcargo insta accept,cargo insta reject, orcargo insta review.
Use thedbg!()macro in Rust for debugging during test runs, and pass--show-outputtocargoto display debug output.
If your PR involves new features or changes to existing features, update documentation accordingly, using inline rustdoc for rules, assists, and options.
When releasing a new version, update allversion: "next"fields in rule metadata to the new version number.
Files:
crates/biome_js_analyze/src/lint/correctness/no_undeclared_dependencies.rs
🪛 LanguageTool
.changeset/two-badgers-cut.md
[uncategorized] ~9-~9: Loose punctuation mark.
Context: ... no longer reported: package.json: ```json { "devDependencies": { "t...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~18-~18: Loose punctuation mark.
Context: ..."type-fest": "*" } } **`foo.ts`**:ts import type { SetRequired } from ...
(UNLIKELY_OPENING_PUNCTUATION)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (24)
- GitHub Check: Documentation
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Test Node.js API
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: autofix
🔇 Additional comments (2)
crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies/valid.ts (1)
39-40: Fixture import looks good.Covers a regular declared dependency case alongside the new type-only case. No concerns.
crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies/valid.package.json (1)
13-14: Dev dependency addition is appropriate.Adding
"type-fest"underdevDependenciesmatches the new rule semantics and the test.
| let is_dev_dependency_available = | ||
| // Type-only imports are always considered as dev dependencies. | ||
| is_type_import(node) || ctx.options().dev_dependencies.is_available(path); | ||
| let is_peer_dependency_available = ctx.options().peer_dependencies.is_available(path); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm intended semantics; consider tests for devDependencies: false.
is_dev_dependency_available becomes true for type-only imports unconditionally. This means type-only imports are allowed in files where devDependencies would otherwise be disallowed by options/globs. If intentional, add explicit tests to lock this in; if not, we should gate by options.
I can help add tests covering:
devDependencies: false(global) withimport type— should be allowed if declared in dev deps.- Glob-restricted availability where file does not match the glob — still allowed for
import type.
Run this to check if such tests already exist:
🏁 Script executed:
#!/bin/bash
# Find tests/options referencing noUndeclaredDependencies + devDependencies
rg -n 'noUndeclaredDependencies' -A 40 crates/ | rg -n '"devDependencies":'Length of output: 504
Add missing test for import type when devDependencies are disabled
We have specs for "devDependencies": false, but none exercise a type-only import. Since is_type_import(...) currently bypasses the dev-deps check unconditionally, please add a test in
crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies that covers:
• Valid: with "devDependencies": false, a file containing
import type { Foo } from 'dev-only-module';is allowed.
• (Optionally) A glob-restricted devDependency case still permitting import type.
If this behaviour isn’t intended, gate the is_type_import branch behind the devDependencies option instead.
🤖 Prompt for AI Agents
In crates/biome_js_analyze/src/lint/correctness/no_undeclared_dependencies.rs
around lines 118 to 121, the code currently allows type-only imports to bypass
the devDependencies check unconditionally. To fix this, add a test in
crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies that
verifies with "devDependencies": false, a file importing type-only from a
dev-only module is allowed. Optionally, add a test for glob-restricted
devDependencies allowing type-only imports. If this behavior is unintended,
modify the code to check the devDependencies option before allowing type-only
imports.
CodSpeed Performance ReportMerging #7171 will improve performances by 7.26%Comparing Summary
Benchmarks breakdown
|
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/two-badgers-cut.md (1)
5-5: Tighten phrasing and drop redundant colons for readability.Minor wordsmithing and formatting nits to make it crisper and also satisfy picky linters complaining about “loose punctuation”.
-Fixed [#7162](https://github.com/biomejs/biome/issues/7162): The `noUndeclaredDependencies` rule now considers a type-only import as a dev dependency. +Fixed [#7162](https://github.com/biomejs/biome/issues/7162): The `noUndeclaredDependencies` rule now treats type-only imports as dev dependencies. -**`package.json`**: +**`package.json`** -**`foo.ts`**: +**`foo.ts`** -Note that you still need to declare the package in the `devDependencies` section in `package.json`. +You still need to declare the package under `devDependencies` in `package.json`.Also applies to: 9-9, 18-18, 23-23
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.changeset/two-badgers-cut.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
.changeset/two-badgers-cut.md
[uncategorized] ~9-~9: Loose punctuation mark.
Context: ... no longer reported: package.json: ```json { "devDependencies": { "t...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~18-~18: Loose punctuation mark.
Context: ..."type-fest": "*" } } **`foo.ts`**:ts import type { SetRequired } from ...
(UNLIKELY_OPENING_PUNCTUATION)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (24)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Documentation
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Check Dependencies
- GitHub Check: autofix
- GitHub Check: Test Node.js API
🔇 Additional comments (1)
.changeset/two-badgers-cut.md (1)
1-24: Clear and helpful changeset.Nice, link is correct and the example is spot on. Reads well.
Summary
Fixed #7162
Type-only imports are no longer required to be declared in the
dependenciessection.Test Plan
Added a snapshot test.
Docs
Updated the doc comment.