Skip to content

Conversation

@JounQin
Copy link
Member

@JounQin JounQin commented May 27, 2025

close #347


Important

Enable nocomment option in minimatch for no-unassigned-import rule to support literal path matching.

  • Behavior:
    • Enables nocomment option in minimatch for no-unassigned-import rule's allow option in no-unassigned-import.ts.
    • Supports literal path matching in addition to glob patterns.
  • Tests:
    • Adds test cases in no-unassigned-import.spec.ts for exact path matching and glob pattern matching with nocomment option.
    • Validates behavior with paths like #app/side-effects.js and glob patterns like \#app/side-effects.js.

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

Summary by CodeRabbit

  • Bug Fixes
    • Improved the handling of import patterns in the import rule, ensuring that comments in glob patterns do not affect matching.
  • Tests
    • Added new test cases to verify correct behavior when allowing specific import patterns, including those with special characters.

@JounQin JounQin requested a review from Copilot May 27, 2025 00:19
@JounQin JounQin self-assigned this May 27, 2025
@JounQin JounQin added bug Something isn't working enhancement New feature or request labels May 27, 2025
@changeset-bot
Copy link

changeset-bot bot commented May 27, 2025

🦋 Changeset detected

Latest commit: 674a717

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 Patch

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

@coderabbitai
Copy link

coderabbitai bot commented May 27, 2025

Walkthrough

The changes update the no-unassigned-import rule in the eslint-plugin-import-x package to modify how glob patterns are matched against import paths. The minimatch library is now used with the nocomment: true option, and new test cases verify that import paths starting with a hash (#) are correctly recognized as allowed.

Changes

File(s) Change Summary
.changeset/wise-tables-stop.md Added changeset documenting the patch update and its rationale.
src/rules/no-unassigned-import.ts Updated minimatch calls to always use { nocomment: true }, ensuring hash-prefixed paths are matched.
test/rules/no-unassigned-import.spec.ts Added two valid test cases for imports with hash-prefixed paths, verifying correct allow-list matching.

Sequence Diagram(s)

sequenceDiagram
    participant ESLint
    participant Rule as no-unassigned-import Rule
    participant Minimatch

    ESLint->>Rule: Analyze import statement
    Rule->>Minimatch: Match import path with allow list (nocomment: true)
    Minimatch-->>Rule: Return match result
    Rule-->>ESLint: Allow or flag import based on result
Loading

Assessment against linked issues

Objective Addressed Explanation
Correctly recognize hash-prefixed import paths (e.g., #app/side-effects.js) in allow list (#347)

Poem

A hash at the start, a path in disguise,
Now matched with precision, to no one’s surprise.
With glob patterns tamed and comments ignored,
The imports you love won’t ever be floored.
🐇✨

Code hops along, with tests in tow—
The rule now lets your side effects flow!

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/rules/no-unassigned-import.ts

Oops! Something went wrong! :(

ESLint: 9.27.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:799:12)
at #cachedDefaultResolve (node:internal/modules/esm/loader:723:25)
at ModuleLoader.resolve (node:internal/modules/esm/loader:706:38)
at ModuleLoader.getModuleJobForImport (node:internal/modules/esm/loader:307:38)
at #link (node:internal/modules/esm/module_job:170:49)

test/rules/no-unassigned-import.spec.ts

Oops! Something went wrong! :(

ESLint: 9.27.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:799:12)
at #cachedDefaultResolve (node:internal/modules/esm/loader:723:25)
at ModuleLoader.resolve (node:internal/modules/esm/loader:706:38)
at ModuleLoader.getModuleJobForImport (node:internal/modules/esm/loader:307:38)
at #link (node:internal/modules/esm/module_job:170:49)


📜 Recent review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 12a7cda and 674a717.

📒 Files selected for processing (3)
  • .changeset/wise-tables-stop.md (1 hunks)
  • src/rules/no-unassigned-import.ts (1 hunks)
  • test/rules/no-unassigned-import.spec.ts (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (20)
  • GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on ubuntu-latest
  • GitHub Check: Codacy Static Code Analysis
  • 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 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 22 and ESLint 8 on windows-latest
  • GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on windows-latest
  • GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
  • 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 8 on windows-latest
  • GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on windows-latest
  • GitHub Check: Lint and Test with Node.js 20 and ESLint 8 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 20 and ESLint 8.56 on windows-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 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 22 and ESLint 9 on macos-latest
  • GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on macos-latest
  • GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on macos-latest
🔇 Additional comments (3)
src/rules/no-unassigned-import.ts (1)

24-28: LGTM! Proper fix for literal path matching with special characters.

The addition of { nocomment: true } to both minimatch calls correctly addresses the issue where paths containing # characters were being treated as comments. This allows the allow option to properly support literal paths that start with # (e.g., Node.js subpath imports).

The implementation maintains backward compatibility while fixing the specific edge case described in the PR.

test/rules/no-unassigned-import.spec.ts (1)

76-89: Excellent test coverage for the fix.

The two test cases comprehensively verify the fix:

  1. Literal path matching: Tests that #app/side-effects.js matches exactly when listed in the allow option
  2. Glob pattern matching: Tests that escaped glob patterns (\#app/side-effects.js) work correctly with the new nocomment behavior

The use of String.raw is appropriate to prevent double-escaping, and the documentation reference adds helpful context.

.changeset/wise-tables-stop.md (1)

1-6: Properly documented changeset for the fix.

The changeset correctly identifies this as a patch-level change and accurately describes the technical implementation. The description clearly states that the nocomment glob option is being enabled for the allow option, which aligns with the actual code changes.

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

@petercat-assistant
Copy link

Walkthrough

This pull request addresses issue #347 by modifying the no-unassigned-import rule to support literal paths in the allow option, in addition to glob patterns. This change enhances the flexibility of the rule by allowing exact path matches.

Changes

File Summary
.changeset/wise-tables-stop.md Added a changeset entry for the patch update.
src/rules/no-unassigned-import.ts Updated logic to support literal path matching alongside glob patterns.
test/rules/no-unassigned-import.spec.ts Added tests to verify the new literal path matching functionality.

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 fixes literal path support for the no-unassigned-import rule by checking for an exact file path match in addition to glob matching. Key changes include:

  • Adding new test cases in test/rules/no-unassigned-import.spec.ts to verify both literal and glob-based matching.
  • Modifying src/rules/no-unassigned-import.ts to include a strict equality check for literal paths.
  • Updating the changeset file to document the patch.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
test/rules/no-unassigned-import.spec.ts Added test cases for exact literal and glob match scenarios
src/rules/no-unassigned-import.ts Incorporated a literal equality check in the matching logic
.changeset/wise-tables-stop.md Recorded the changeset entry for the patch

@codesandbox-ci
Copy link

codesandbox-ci bot commented May 27, 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

@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 65a6616 in 56 seconds. Click for details.
  • Reviewed 48 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 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/wise-tables-stop.md:1
  • Draft comment:
    Changelog entry correctly documents the patch update.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
2. src/rules/no-unassigned-import.ts:24
  • Draft comment:
    Added literal equality check (filePath === glob) to allow exact matching of module paths like '#app/side-effects.js'. This change addresses the issue by short-circuiting glob matching when an exact literal is provided.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, explaining the change made in the code. It does not provide a suggestion, ask for confirmation, or point out a potential issue. According to the rules, purely informative comments should be removed.
3. test/rules/no-unassigned-import.spec.ts:77
  • Draft comment:
    New test cases ensure that both an exact literal match and a minimatch-based (escaped) glob pattern correctly allow module paths with special characters.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_0km6Ir45NgVD5uhK

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

@codacy-production
Copy link

codacy-production bot commented May 27, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.00% (target: -1.00%) 100.00%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (12a7cda) 3626 3474 95.81%
Head commit (674a717) 3626 (+0) 3474 (+0) 95.81% (+0.00%)

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 (#352) 1 1 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

@codecov
Copy link

codecov bot commented May 27, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.27%. Comparing base (12a7cda) to head (674a717).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #352   +/-   ##
=======================================
  Coverage   96.27%   96.27%           
=======================================
  Files          92       92           
  Lines        4836     4836           
  Branches     1825     1823    -2     
=======================================
  Hits         4656     4656           
  Misses        179      179           
  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 May 27, 2025

Open in StackBlitz

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

commit: 674a717

@JounQin JounQin force-pushed the fix/no-unassigned-import branch from 65a6616 to 674a717 Compare May 27, 2025 00:29
@JounQin JounQin changed the title fix: support literal path instead of glob for rule no-unassigned-import's allow option fix: enable nocomment glob option for rule no-unassigned-import's allow option May 27, 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 674a717 in 36 seconds. Click for details.
  • Reviewed 54 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 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/wise-tables-stop.md:1
  • Draft comment:
    Changeset message is clear and concise; the patch version and commit message correctly indicate the change to support literal path matching.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
2. src/rules/no-unassigned-import.ts:24
  • Draft comment:
    Good update: both minimatch calls now include the nocomment: true option to correctly handle literal '#' paths (issue #347).
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
3. test/rules/no-unassigned-import.spec.ts:77
  • Draft comment:
    New test cases for literal '#' import paths and glob matching effectively validate the fix.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_TJjBtyrFAc7mO9AH

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

@JounQin JounQin merged commit 3bc48fc into master May 27, 2025
88 checks passed
@JounQin JounQin deleted the fix/no-unassigned-import branch May 27, 2025 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

#local imports paths being recognized in no-unassigned-import config

2 participants