Skip to content

Conversation

@JounQin
Copy link
Member

@JounQin JounQin commented Jun 18, 2025

close #258
related #385

https://eslint-react.xyz/docs/rules/prefer-react-namespace-import


Important

Introduces prefer-namespace-import rule to enforce namespace imports for specified modules, with documentation and tests.

  • New Rule:
    • Added prefer-namespace-import rule in src/rules/prefer-namespace-import.ts to enforce namespace imports for specified modules.
    • Rule is automatically fixable and configurable with patterns for module matching.
  • Documentation:
    • Added docs/rules/prefer-namespace-import.md with rule details and examples.
    • Updated README.md to include the new rule in the rules list.
  • Tests:
    • Added tests for prefer-namespace-import in test/rules/prefer-namespace-import.spec.ts to verify rule enforcement and autofix behavior.
  • Configuration:
    • Updated src/index.ts to include the new rule in the plugin's ruleset.

This description was created by Ellipsis for 6e30b35. You can customize this summary. It will automatically update as commits are pushed.


Summary by CodeRabbit

  • New Features
    • Introduced a new ESLint rule enforcing namespace imports for modules matching user-defined patterns.
  • Documentation
    • Added detailed documentation and usage examples for the new rule.
    • Updated the README to include the new rule entry.
  • Tests
    • Added comprehensive tests verifying rule enforcement and autofix functionality.

@JounQin JounQin requested a review from Copilot June 18, 2025 13:48
@JounQin JounQin self-assigned this Jun 18, 2025
@JounQin JounQin added the enhancement New feature or request label Jun 18, 2025
@changeset-bot
Copy link

changeset-bot bot commented Jun 18, 2025

🦋 Changeset detected

Latest commit: 6e30b35

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
eslint-plugin-import-x Minor

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

@codacy-production
Copy link

codacy-production bot commented Jun 18, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.03% (target: -1.00%) 100.00%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (10cf017) 3694 3531 95.59%
Head commit (6e30b35) 3720 (+26) 3557 (+26) 95.62% (+0.03%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#386) 26 26 100.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

@coderabbitai
Copy link

coderabbitai bot commented Jun 18, 2025

Walkthrough

A new ESLint rule, prefer-namespace-import, has been introduced to enforce namespace imports for modules matching configurable patterns. The rule, its documentation, and corresponding tests have been added. The rule is now included in the plugin's exported rules and is described in the README, with support for autofix and custom pattern matching.

Changes

File(s) Change Summary
README.md, docs/rules/prefer-namespace-import.md Added documentation for the new prefer-namespace-import rule, including usage, configuration, and schema.
src/rules/prefer-namespace-import.ts Implemented the new ESLint rule with pattern matching and autofix logic.
src/index.ts Registered the new rule in the exported rules object.
test/rules/prefer-namespace-import.spec.ts Added unit tests for the new rule, covering valid, invalid, and autofix scenarios.
src/rules/prefer-default-export.ts Changed MessageId type alias from private to exported (unrelated to new rule).
test/utils.ts Removed automatic injection of default parser options in test utility function.
.changeset/light-llamas-return.md Added changeset documenting the new rule migration and feature.

Sequence Diagram(s)

sequenceDiagram
    participant ESLint
    participant Rule as prefer-namespace-import Rule
    participant UserConfig

    ESLint->>Rule: Analyze import declarations
    Rule->>UserConfig: Retrieve patterns option
    Rule->>Rule: Match import source against patterns
    alt Default import matches pattern
        Rule->>ESLint: Report error, suggest namespace import
        alt Autofix enabled
            Rule->>ESLint: Replace/split import with namespace import
        end
    end
Loading

Assessment against linked issues

Objective Addressed Explanation
Migrate react-x/prefer-react-namespace-import to import-x/prefer-namespace-import with custom patterns (#258)
Support configuration via patterns (string/RegExp) to match modules (#258)
Provide auto-fix to convert default imports to namespace imports (#258)
Add documentation and usage examples for the new rule (#258)

Assessment against linked issues: Out-of-scope changes

Code Change (file_path) Explanation
Exported MessageId type in src/rules/prefer-default-export.ts This change is unrelated to the new rule and its migration; it alters export visibility for a different rule.
Removed default parserOptions injection in test/utils.ts This change affects test utility defaults and is unrelated to the new rule implementation or migration.

Suggested labels

feature, documentation, internal

Poem

In the warren of code, a new rule appears,
To tidy imports and calm devs' fears.
Patterns and namespaces, all in a row,
With autofix magic, your imports will glow!
🐇✨
Hop along, dear modules, in namespaced delight—
The code garden grows ever more right!

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

src/index.ts

Oops! Something went wrong! :(

ESLint: 9.29.0

Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/lib/index.js' imported from /eslint.config.js
at finalizeResolution (node:internal/modules/esm/resolve:274:11)
at moduleResolve (node:internal/modules/esm/resolve:859:10)
at defaultResolve (node:internal/modules/esm/resolve:983:11)
at ModuleLoader.defaultResolve (node:internal/modules/esm/loader:801:12)
at #cachedDefaultResolve (node:internal/modules/esm/loader:725:25)
at ModuleLoader.resolve (node:internal/modules/esm/loader:708:38)
at ModuleLoader.getModuleJobForImport (node:internal/modules/esm/loader:309:38)
at #link (node:internal/modules/esm/module_job:201:49)

src/rules/prefer-default-export.ts

Oops! Something went wrong! :(

ESLint: 9.29.0

Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/lib/index.js' imported from /eslint.config.js
at finalizeResolution (node:internal/modules/esm/resolve:274:11)
at moduleResolve (node:internal/modules/esm/resolve:859:10)
at defaultResolve (node:internal/modules/esm/resolve:983:11)
at ModuleLoader.defaultResolve (node:internal/modules/esm/loader:801:12)
at #cachedDefaultResolve (node:internal/modules/esm/loader:725:25)
at ModuleLoader.resolve (node:internal/modules/esm/loader:708:38)
at ModuleLoader.getModuleJobForImport (node:internal/modules/esm/loader:309:38)
at #link (node:internal/modules/esm/module_job:201:49)

src/rules/prefer-namespace-import.ts

Oops! Something went wrong! :(

ESLint: 9.29.0

Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/lib/index.js' imported from /eslint.config.js
at finalizeResolution (node:internal/modules/esm/resolve:274:11)
at moduleResolve (node:internal/modules/esm/resolve:859:10)
at defaultResolve (node:internal/modules/esm/resolve:983:11)
at ModuleLoader.defaultResolve (node:internal/modules/esm/loader:801:12)
at #cachedDefaultResolve (node:internal/modules/esm/loader:725:25)
at ModuleLoader.resolve (node:internal/modules/esm/loader:708:38)
at ModuleLoader.getModuleJobForImport (node:internal/modules/esm/loader:309:38)
at #link (node:internal/modules/esm/module_job:201:49)

  • 1 others
✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 18, 2025

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Copy link

Copilot AI left a comment

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 ports a new ESLint rule "prefer-namespace-import" which enforces using namespace imports for specific modules. Key changes include:

  • Adding unit tests in test/rules/prefer-namespace-import.spec.ts
  • Implementing the rule in src/rules/prefer-namespace-import.ts with an auto-fix feature
  • Integrating the rule in the index, updating related exports, and documenting it in docs and README

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/rules/prefer-namespace-import.spec.ts Adds test cases for the new rule enforcing namespace imports
src/rules/prefer-namespace-import.ts Implements the rule logic with error reporting and auto-fix support
src/rules/prefer-default-export.ts Updates the MessageId export for improved module interface
src/index.ts Integrates the new rule into the exported rules
docs/rules/prefer-namespace-import.md Provides documentation for the new rule
README.md Updates the rule table with an entry for prefer-namespace-import rule

@codecov
Copy link

codecov bot commented Jun 18, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.05%. Comparing base (10cf017) to head (6e30b35).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #386      +/-   ##
==========================================
+ Coverage   96.03%   96.05%   +0.02%     
==========================================
  Files          95       96       +1     
  Lines        4917     4947      +30     
  Branches     1848     1855       +7     
==========================================
+ Hits         4722     4752      +30     
  Misses        194      194              
  Partials        1        1              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Jun 18, 2025

Open in StackBlitz

npm i https://pkg.pr.new/eslint-plugin-import-x@386

commit: 6e30b35

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed everything up to 90c3115 in 1 minute and 46 seconds. Click for details.
  • Reviewed 282 lines of code in 6 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/rules/prefer-namespace-import.ts:78
  • Draft comment:
    Using substring extraction via indexOf('{') may be brittle for varied import formatting. Consider a token‐based approach to reliably capture the named specifiers.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 30% vs. threshold = 50% The code is part of an ESLint rule's fixer, which needs to be robust. Using string manipulation could break with different formatting (spaces, newlines, comments). However, the code is getting the text directly from the AST node via sourceCode.getText(), which should be well-formed. The ESLint infrastructure likely ensures proper formatting. The suggested token-based approach would be more complex without clear benefits. The comment raises a valid concern about code robustness. String manipulation can be fragile compared to proper AST traversal. Since we're working within ESLint's infrastructure and getting text directly from AST nodes, the string manipulation approach is likely safe enough for this specific use case. While the comment raises a theoretically valid point, the current approach is pragmatic and likely robust enough given the context of ESLint's AST handling.
2. test/rules/prefer-namespace-import.spec.ts:45
  • Draft comment:
    Test coverage looks good but consider adding cases for combined default and named imports (and possibly type imports) to ensure the auto-fix handles all variants.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_JCMijP5ARV87TINU

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (6)
src/rules/prefer-default-export.ts (1)

9-9: Exporting MessageId widens the public API surface

MessageId was previously an implementation detail of this rule; exporting it means it becomes part of the plugin’s public contract. If tests (or other rules) need the type, consider:

  1. Marking it /** @internal */ so tooling still hides it from users, or
  2. Re-exporting it under a namespaced name (e.g. PreferDefaultExportMessageId) to avoid potential clashes.

Not blocking, but worth a second look.

README.md (1)

256-259: Verify that the rules table is regenerated, not hand-edited

The rules list is usually auto-generated. If this was a manual edit, run the docs generation script (pnpm run docs:rules or similar) to avoid future drift and keep the emoji columns consistent.

docs/rules/prefer-namespace-import.md (1)

31-46: Minor doc clarity: highlight that regex entries must be strings

Because JSON doesn’t allow bare regex literals, emphasise that regex patterns must be passed as strings (e.g. "/^react-/"). Stating this explicitly avoids confusion.

-`patterns` is an array of strings or `RegExp` patterns
+`patterns` is an array of **strings**.  
+If a string starts and ends with `/`, it will be treated as a `RegExp`
+pattern (e.g. `"/^react-/"`).
test/rules/prefer-namespace-import.spec.ts (2)

7-10: RuleTester should declare the parser for ESLint v9 compatibility

@typescript-eslint/rule-tester currently defaults to the TS parser, but ESLint v9 may require it to be set explicitly. Consider:

-const ruleTester = new TSESLintRuleTester()
+const ruleTester = new TSESLintRuleTester({
+  parser: '@typescript-eslint/parser',
+})

45-49: Add a “valid with patterns” case to complete coverage

Right now the suite verifies:

  • no-option (patterns = []) cases ✅
  • option + invalid default import ❌

But it doesn’t assert that option + correct namespace import is accepted. Adding one keeps the regression net tight.

src/rules/prefer-namespace-import.ts (1)

67-70: Avoid .at(0) for broader Node-compatibility

String.prototype.at is only available from Node 16. Using bracket-indexing keeps the rule consumable by projects that are still on 14 LTS (which is still common in many CI environments).

-const quote = node.parent.source.raw.at(0) ?? "'"
+const quote = node.parent.source.raw?.[0] ?? "'"
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 10cf017 and 90c3115.

📒 Files selected for processing (6)
  • README.md (1 hunks)
  • docs/rules/prefer-namespace-import.md (1 hunks)
  • src/index.ts (2 hunks)
  • src/rules/prefer-default-export.ts (1 hunks)
  • src/rules/prefer-namespace-import.ts (1 hunks)
  • test/rules/prefer-namespace-import.spec.ts (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
test/rules/prefer-namespace-import.spec.ts (1)
test/utils.ts (1)
  • createRuleTestCaseFunctions (143-158)
⏰ Context from checks skipped due to timeout of 90000ms (21)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Lint and Test with Node.js 24 and ESLint 8 on windows-latest
  • GitHub Check: Lint and Test with Node.js lts/* and ESLint 9 on ubuntu-latest
  • GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on windows-latest
  • GitHub Check: Lint and Test with Node.js 24 and ESLint 9 on windows-latest
  • GitHub Check: Lint and Test with Node.js 24 and ESLint 8 on ubuntu-latest
  • GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on ubuntu-latest
  • GitHub Check: Lint and Test with Node.js 24 and ESLint 9 on ubuntu-latest
  • GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on ubuntu-latest
  • GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on windows-latest
  • GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on ubuntu-latest
  • GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on ubuntu-latest
  • GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on ubuntu-latest
  • GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on ubuntu-latest
  • GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on macos-latest
  • GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on macos-latest
  • GitHub Check: Lint and Test with Node.js 24 and ESLint 8 on macos-latest
  • GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on macos-latest
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on windows-latest
  • GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on macos-latest
🔇 Additional comments (3)
src/index.ts (2)

68-69: Import addition looks good and keeps alphabetical ordering
No issues spotted.


115-116: Rule registered correctly in the exported map

The key follows the kebab-case convention and sits in the correct alpha slot after prefer-default-export.

src/rules/prefer-namespace-import.ts (1)

110-112: Defaulting to the u flag silently changes user regexes

When a pattern string is of the form /foo/, the code appends 'u' if no flags are supplied.
That alters the behaviour of patterns that rely on legacy escapes or the . not matching astral symbols.

Recommend leaving flags empty when the user didn’t specify any:

-const [, pattern, flags = 'u'] = REGEXP_STR.exec(string) ?? []
+const [, pattern, flags = ''] = REGEXP_STR.exec(string) ?? []

@JounQin JounQin changed the title feat: port rule react-x/prefer-namespace-import feat: port react-x/prefer-react-namespace-import into prefer-namespace-import Jun 18, 2025
Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed 73bdcc9 in 1 minute and 0 seconds. Click for details.
  • Reviewed 84 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 5 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .changeset/light-llamas-return.md:5
  • Draft comment:
    Clear changeset message; ensure the new rule name (import-x/prefer-namespace-import) is referenced consistently.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
2. test/rules/prefer-namespace-import.spec.ts:14
  • Draft comment:
    Good to see valid test cases using concise single-line code strings for namespace imports.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
3. test/rules/prefer-namespace-import.spec.ts:25
  • Draft comment:
    Valid test for non-matching modules (e.g. 'other-name') confirms default imports remain unaltered when patterns don't match.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
4. test/rules/prefer-namespace-import.spec.ts:36
  • Draft comment:
    Invalid tests correctly flag default and mixed (default + named) imports for modules matching the specified patterns.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
5. test/rules/prefer-namespace-import.spec.ts:61
  • Draft comment:
    Auto-fix output appropriately splits mixed default and named imports into separate statements while preserving order.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_5exwDHjsHhIqS4NS

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Caution

Changes requested ❌

Reviewed 6cec75c in 2 minutes and 32 seconds. Click for details.
  • Reviewed 231 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 5 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/rules/prefer-namespace-import.ts:45
  • Draft comment:
    Using 'ImportDefaultSpecifier' directly simplifies node matching. Verify that this selector catches all intended default import cases.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
2. src/rules/prefer-namespace-import.ts:64
  • Draft comment:
    For single default imports, replacing only the default specifier ensures that existing 'import' or 'import type' keywords are preserved. Confirm this behavior in all formatting cases.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
3. src/rules/prefer-namespace-import.ts:91
  • Draft comment:
    The toRegExp function's comment suggests plain strings become /^foo$/ but the implementation uses strict equality instead. Update the documentation to accurately reflect that behavior.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. test/rules/prefer-namespace-import.spec.ts:18
  • Draft comment:
    Defining the constant 'options' for patterns improves maintainability. The test cases now comprehensively cover various import syntaxes including import assertions and TypeScript 'import type'.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
5. test/utils.ts:56
  • Draft comment:
    Removal of default languageOptions injection simplifies test fixture setup. Confirm that tests still run with the intended parserOptions when not explicitly provided.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_MStqxJ84oBlS3QM3

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Caution

Changes requested ❌

Reviewed 7a6b4eb in 2 minutes and 25 seconds. Click for details.
  • Reviewed 91 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/rules/prefer-namespace-import.ts:61
  • Draft comment:
    When an import has additional named specifiers (e.g. import Foo, { bar } from 'foo'), the auto‐fix splits it into two separate import declarations. This transformation (lines 61–84) may produce duplicate module source clauses and can break code that expects a single import. Consider disabling the auto‐fix or implementing a fix that converts the entire import into a pure namespace import.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. src/rules/prefer-namespace-import.ts:104
  • Draft comment:
    The helper toRegExp is acceptable overall, but if users supply an invalid regex string it silently falls back to simple equality. Consider adding validation or documentation that only valid regex strings (or plain strings) are supported.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50% None

Workflow ID: wflow_uXATLqJHcoHOPXzL

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/rules/prefer-namespace-import.ts (1)

70-83: String slicing is still brittle for exotic import syntaxes (duplicate)
Previous reviews already highlighted that slicing on '{'/'}' can mis-identify the named-specifier block when decorators, comments, or exotic syntax (e.g. import Foo, { type Bar } from …) are present. Building the list from node.parent.specifiers is more robust.

🧹 Nitpick comments (1)
src/rules/prefer-namespace-import.ts (1)

105-109: Avoid silently adding the u regex flag

toRegExp forces the u flag when the user omits flags. This changes the semantics of simple patterns ('.' matches astral characters differently). Retain the user-supplied flags verbatim and only add u when none are provided and you need Unicode mode.

-const [, pattern, flags = 'u'] = REGEXP_STR.exec(string) ?? []
+const [, pattern, flags = ''] = REGEXP_STR.exec(string) ?? []
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6cec75c and 7a6b4eb.

📒 Files selected for processing (3)
  • README.md (1 hunks)
  • docs/rules/prefer-namespace-import.md (1 hunks)
  • src/rules/prefer-namespace-import.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/rules/prefer-namespace-import.md
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/rules/prefer-namespace-import.ts (1)
src/utils/create-rule.ts (1)
  • createRule (70-70)
🪛 LanguageTool
README.md

[uncategorized] ~320-~320: You might be missing the article “a” here.
Context: ... | Prefer a default export if module exports a single name or multiple names...

(AI_EN_LECTOR_MISSING_DETERMINER_A)

⏰ Context from checks skipped due to timeout of 90000ms (21)
  • GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on ubuntu-latest
  • GitHub Check: Lint and Test with Node.js lts/* and ESLint 9 on ubuntu-latest
  • GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on ubuntu-latest
  • GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on ubuntu-latest
  • GitHub Check: Lint and Test with Node.js lts/* and ESLint 9 on ubuntu-latest
  • GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on windows-latest
  • GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on windows-latest
  • GitHub Check: Lint and Test with Node.js 24 and ESLint 9 on ubuntu-latest
  • GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on ubuntu-latest
  • GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on ubuntu-latest
  • GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on ubuntu-latest
  • GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on ubuntu-latest
  • GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on ubuntu-latest
  • GitHub Check: Lint and Test with Node.js 20 and ESLint 8 on ubuntu-latest
  • GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on ubuntu-latest
  • GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on macos-latest
  • GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on macos-latest
  • GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on macos-latest
  • GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on macos-latest
  • GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on macos-latest
  • GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (2)
src/rules/prefer-namespace-import.ts (1)

61-68: Nice, minimal fix for default-only imports
The replacement keeps the original declaration intact while swapping the specifier — concise and safe.

README.md (1)

301-322: Rule entry looks good – added row is correctly formatted, includes the 🔧 marker, and is placed in alphabetical order.

@JounQin JounQin force-pushed the prefer-namespace-import branch from 7a6b4eb to 0b6a12c Compare June 18, 2025 15:21
Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed 0b6a12c in 1 minute and 7 seconds. Click for details.
  • Reviewed 91 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/rules/prefer-namespace-import.ts:72
  • Draft comment:
    The fixer for multiple specifiers uses string slicing (indexOf('{') and indexOf('}')) which may be brittle for complex formatting. Consider using AST node data for a more robust fix.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. src/rules/prefer-namespace-import.ts:104
  • Draft comment:
    The comment for 'toRegExp' states that normal strings are converted to /^foo$/, but the implementation returns a test function doing strict equality. Consider updating the comment or implementation for clarity.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_eM0l51lsxoMrkUjw

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed 6e30b35 in 1 minute and 40 seconds. Click for details.
  • Reviewed 90 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 4 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. test/rules/prefer-namespace-import.spec.ts:18
  • Draft comment:
    Added 'specific' pattern to options. Ensure literal string patterns are clearly documented as exact matches (unlike regex strings).
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
2. test/rules/prefer-namespace-import.spec.ts:29
  • Draft comment:
    Valid test case for 'specific' using namespace import is added. Confirms that matching literal strings are not flagged.
  • Reason this comment was not posted:
    Confidence changes required: 10% <= threshold 50% None
3. test/rules/prefer-namespace-import.spec.ts:50
  • Draft comment:
    Invalid test now includes a default import from 'specific'. This is correctly flagged and auto-fixed to a namespace import. Verify that the transform handles literal matches consistently.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to verify the behavior of the code after a change, which is against the rules. It doesn't provide a specific suggestion or point out a clear issue.
4. test/rules/prefer-namespace-import.spec.ts:122
  • Draft comment:
    TypeScript invalid test case now checks a type import from 'specific'. The auto-fix should convert it to 'import type * as Name5 from "specific"'. Confirm that type-specific syntax is handled correctly.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.

Workflow ID: wflow_BEbksQJFEjijVDcx

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@JounQin
Copy link
Member Author

JounQin commented Jun 18, 2025

@coderabbitai full review

@coderabbitai
Copy link

coderabbitai bot commented Jun 18, 2025

✅ Actions performed

Full review triggered.

@codacy-production
Copy link

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.03% (target: -1.00%) 100.00%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (10cf017) 3694 3531 95.59%
Head commit (6e30b35) 3720 (+26) 3557 (+26) 95.62% (+0.03%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#386) 26 26 100.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (2)
src/rules/prefer-namespace-import.ts (2)

72-83: String-slicing fix is still brittle; relies on first } occurrence
(See previous discussion – re-flagging for visibility.)

If comments or string literals appear inside the named-import block the first } might belong to them, producing malformed fixes:

import Foo, { /* } */ Bar } from 'lib';

Consider reconstructing the named specifiers directly from node.parent.specifiers instead of substring slicing. This avoids false matches and preserves formatting reliably.


104-110: toRegExp will throw on invalid regex flags – confirm that fail-fast behaviour is acceptable

new RegExp(pattern, flags) will synchronously throw for invalid flag sets (e.g. /foo/z).
Because the rule is created during linting, this will abort the entire ESLint run rather than reporting a configuration error.

If the intent is truly “fail hard”, leave as is. Otherwise, wrap in a try/catch and surface an ESLint-style config error to the user.

🧹 Nitpick comments (2)
test/rules/prefer-namespace-import.spec.ts (2)

32-36: Duplicate local binding in “valid” fixture may break parsing

Name2 is imported twice in the same module sample. ECMAScript modules forbid duplicate local bindings, and some parsers (especially Babel/Hermes) will throw a parse error before your rule executes, causing the test to fail for that parser.

-          import * as Name2 from 'prefix-name';
-          import * as Name2 from 'specific';
+          import * as Name2 from 'prefix-name';
+          import * as Name3 from 'specific';

Update the identifier (or remove the second line) to avoid parser-level failures.


90-97: parserConfig is ignored for the TS parser – loop iteration is redundant

@typescript-eslint/rule-tester already sets @typescript-eslint/parser as the default parser.
Iterating over parsers.TS therefore re-runs the suite with exactly the same configuration, adding ~50 % test time for no extra coverage.

If that duplication is unintentional, drop parsers.TS from getNonDefaultParsers() or gate the loop with if (parser !== parsers.TS).

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 10cf017 and 6e30b35.

📒 Files selected for processing (8)
  • .changeset/light-llamas-return.md (1 hunks)
  • README.md (1 hunks)
  • docs/rules/prefer-namespace-import.md (1 hunks)
  • src/index.ts (2 hunks)
  • src/rules/prefer-default-export.ts (1 hunks)
  • src/rules/prefer-namespace-import.ts (1 hunks)
  • test/rules/prefer-namespace-import.spec.ts (1 hunks)
  • test/utils.ts (0 hunks)
💤 Files with no reviewable changes (1)
  • test/utils.ts
🧰 Additional context used
🧠 Learnings (1)
src/rules/prefer-namespace-import.ts (1)
Learnt from: JounQin
PR: un-ts/eslint-plugin-import-x#386
File: src/rules/prefer-namespace-import.ts:41-46
Timestamp: 2025-06-18T15:22:38.492Z
Learning: In eslint-plugin-import-x, JounQin prefers to throw on invalid rule options rather than handling them gracefully with try/catch blocks and reporting configuration errors.
🧬 Code Graph Analysis (2)
src/rules/prefer-namespace-import.ts (2)
src/rules/prefer-default-export.ts (2)
  • Options (5-7)
  • MessageId (9-9)
src/utils/create-rule.ts (1)
  • createRule (70-70)
test/rules/prefer-namespace-import.spec.ts (1)
test/utils.ts (4)
  • createRuleTestCaseFunctions (135-150)
  • getNonDefaultParsers (46-48)
  • parsers (19-24)
  • testFilePath (42-44)
🪛 LanguageTool
README.md

[uncategorized] ~320-~320: You might be missing the article “a” here.
Context: ... | Prefer a default export if module exports a single name or multiple names...

(AI_EN_LECTOR_MISSING_DETERMINER_A)

🔇 Additional comments (6)
.changeset/light-llamas-return.md (1)

1-6: Changeset is correct
The changeset properly documents the new prefer-namespace-import feature and targets a minor release. All fields and formatting follow the expected Changeset conventions.

src/rules/prefer-default-export.ts (1)

9-9: Expose MessageId type for external use
Exporting the previously internal MessageId type aligns this module with others and enables consistent typing in dependent code, without altering existing logic.

src/index.ts (2)

68-68: Import new prefer-namespace-import rule
The rule is correctly imported alongside existing rules, using a consistent naming convention.


115-115: Register prefer-namespace-import in the rules map
Adding the rule under the key 'prefer-namespace-import' aligns with the plugin’s export structure and naming patterns.

README.md (1)

321-321: The new table row for prefer-namespace-import matches the style guide and uses the imperative form consistently.

docs/rules/prefer-namespace-import.md (1)

1-7: Documentation is clear and comprehensive
The rule header, description, schema, and examples effectively illustrate configuration and usage, including autofix behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feat] migrate react-x/prefer-react-namespace-import to import-x/prefer-namespace-import Rule with Custom Patterns

3 participants