refactor: make no-unused-modules no-op on ESLint 10 or later#457
refactor: make no-unused-modules no-op on ESLint 10 or later#457SukkaW merged 6 commits intoun-ts:masterfrom
no-unused-modules no-op on ESLint 10 or later#457Conversation
🦋 Changeset detectedLatest commit: 8c91a7b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDefers access to ESLint's FileEnumerator to runtime, makes the no-unused-modules rule a no-op when that API is unavailable (ESLint 10+), emits a warning unless suppressed, and adds the Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant DevRule as "Rule (no-unused-modules)"
participant ESLint as "ESLint Runtime"
participant FileEnum as "FileEnumerator API"
participant Logger as "Console/Logger"
DevRule->>ESLint: rule created / config parsed
ESLint->>DevRule: invoke rule factory
DevRule->>FileEnum: attempt lazy access (guarded)
alt FileEnumerator available
FileEnum-->>DevRule: returns file list
DevRule->>ESLint: perform unused-module checks
else FileEnumerator missing (ESLint 10+)
DevRule->>Logger: emit one-time warning (unless suppressed)
DevRule-->>ESLint: return no-op rule (skip file enumeration)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
|
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. |
commit: |
|
What is it that causes the crash? I think I'm missing the context here 👀 |
|
1ae11a6 to
4b25628
Compare
|
ok that makes sense. in that case, i agree it should ultimately be removed but could temporarily be no-op until then |
4b25628 to
c815920
Compare
Technically, we can iterate over all the files ourselves, but I prefer not to do that. ESLint is really not great for cross-file linting for now. In the meantime let's just no-op it. |
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to c815920 in 10 seconds. Click for details.
- Reviewed
87lines of code in2files - Skipped
0files when reviewing. - Skipped posting
0draft 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.
Workflow ID: wflow_FKF3xjB5ccFnWqj0
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/rules/no-unused-modules.ts (1)
29-32: Nit: comment grammar.Line 30: "avoid access the removed APIs" → "avoid accessing the removed APIs".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rules/no-unused-modules.ts` around lines 29 - 32, Update the inline comment that currently reads "This allows us to avoid access the removed APIs as long as the function is not invoked." to correct the grammar by changing "avoid access" to "avoid accessing" (the surrounding comment references shouldUseFlatConfig and FileEnumerator and should remain otherwise unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/clever-meals-fly.md:
- Line 5: Fix the stray extra quote in the config example: update the snippet
`['error', { suppressMissingFileEnumeratorAPIWarning: true }']` to `['error', {
suppressMissingFileEnumeratorAPIWarning: true }]` (or include the missing
closing quote only around the whole string as intended) wherever it appears —
specifically in the .changeset/clever-meals-fly.md text and in the console.warn
message in src/rules/no-unused-modules.ts (the warn at/around the place
referenced, previously line 559). Also search for any other occurrences of the
malformed snippet in the repo and correct them so the example is syntactically
valid when copy-pasted.
In `@src/rules/no-unused-modules.ts`:
- Around line 557-560: The warning string contains a stray quote in the embedded
config snippet (`import-x/no-unused-modules: ['error', {
suppressMissingFileEnumeratorAPIWarning: true }']`) — update that literal so the
snippet ends with `}]` (i.e., `import-x/no-unused-modules: ['error', {
suppressMissingFileEnumeratorAPIWarning: true }]`) so the shown config is
syntactically correct; locate the console.warn call that prints this message
(the string containing "suppressMissingFileEnumeratorAPIWarning") and fix the
trailing quote.
- Around line 554-563: The warning is printed once per file because create()
runs for each file; add a module-level boolean (e.g.,
warnedMissingFileEnumerator) near the top of the module (around the Options
interface or other module state) and change the FileEnumerator check in the
create() implementation to only call console.warn when the flag is false, then
set the flag to true after logging so subsequent create() calls do not emit the
same message; keep the existing suppressMissingFileEnumeratorAPIWarning option
check as-is to allow explicit suppression.
---
Nitpick comments:
In `@src/rules/no-unused-modules.ts`:
- Around line 29-32: Update the inline comment that currently reads "This allows
us to avoid access the removed APIs as long as the function is not invoked." to
correct the grammar by changing "avoid access" to "avoid accessing" (the
surrounding comment references shouldUseFlatConfig and FileEnumerator and should
remain otherwise unchanged).
There was a problem hiding this comment.
Pull request overview
This PR refactors the no-unused-modules rule to gracefully handle ESLint 10+, where the FileEnumerator API has been removed. The rule becomes a no-op on ESLint 10+ and displays an optional warning to users about this behavior.
Changes:
- Moved
FileEnumeratorandshouldUseFlatConfigaccess intolistFilesUsingFileEnumerator()function to avoid accessing removed APIs at module load time - Added
suppressMissingFileEnumeratorAPIWarningoption to allow users to suppress the informational warning - Added ESLint 10 detection logic that returns early with an empty rule implementation when
FileEnumeratoris unavailable
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/rules/no-unused-modules.ts | Refactored to lazily access FileEnumerator API and added ESLint 10 compatibility check with optional warning |
| .changeset/clever-meals-fly.md | Added changeset documenting the patch-level change for ESLint 10 compatibility |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/rules/no-unused-modules.ts (1)
554-562:⚠️ Potential issue | 🟠 MajorMissing-API warning should be emitted once, not once per linted file.
Because
create()runs per file, this prints the same warning repeatedly in large projects. Please gate the warning with a module-level flag.🔧 Suggested fix
+let warnedAboutMissingFileEnumeratorAPI = false + export interface Options { src?: string[] ignoreExports?: string[] missingExports?: true @@ - if (!('FileEnumerator' in eslintUnsupportedApi)) { - if (!suppressMissingFileEnumeratorAPIWarning) { + if (!('FileEnumerator' in eslintUnsupportedApi)) { + if ( + !suppressMissingFileEnumeratorAPIWarning && + !warnedAboutMissingFileEnumeratorAPI + ) { + warnedAboutMissingFileEnumeratorAPI = true console.warn(` ESLint removes the FileEnumerator API since ESLint 10, which is required by the "no-unused-modules" rule. Therefore, the "no-unused-modules" rule is no-op for now before we can implement an alternative. In the meantime, if you want to keep this rule enabled, you can suppress this warning with the "suppressMissingFileEnumeratorAPIWarning" rule option: \`import-x/no-unused-modules: ['error', { suppressMissingFileEnumeratorAPIWarning: true }]\` `) } return {} }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rules/no-unused-modules.ts` around lines 554 - 562, The warning inside the create() path is emitted per-file because create() is called for each linted file; add a module-level flag (e.g., let missingFileEnumeratorWarningEmitted = false) at top of src/rules/no-unused-modules.ts and wrap the console.warn block with if (!missingFileEnumeratorWarningEmitted && !suppressMissingFileEnumeratorAPIWarning) { ...; missingFileEnumeratorWarningEmitted = true; } so the message (controlled by eslintUnsupportedApi and suppressMissingFileEnumeratorAPIWarning) is logged only once across the process.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/rules/no-unused-modules.ts`:
- Line 559: Update the example config text for the "import-x/no-unused-modules"
rule so it satisfies the rule's options schema: include either "unusedExports:
true" or "missingExports: true" alongside
"suppressMissingFileEnumeratorAPIWarning". Locate the example that mentions
suppressMissingFileEnumeratorAPIWarning and change the suggested object to
include the required flag (e.g. add unusedExports: true) so the snippet becomes
a valid options object for the rule.
---
Duplicate comments:
In `@src/rules/no-unused-modules.ts`:
- Around line 554-562: The warning inside the create() path is emitted per-file
because create() is called for each linted file; add a module-level flag (e.g.,
let missingFileEnumeratorWarningEmitted = false) at top of
src/rules/no-unused-modules.ts and wrap the console.warn block with if
(!missingFileEnumeratorWarningEmitted &&
!suppressMissingFileEnumeratorAPIWarning) { ...;
missingFileEnumeratorWarningEmitted = true; } so the message (controlled by
eslintUnsupportedApi and suppressMissingFileEnumeratorAPIWarning) is logged only
once across the process.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: JounQin <admin@1stg.me>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: JounQin <admin@1stg.me>
f4c609b to
dcd459f
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/rules/no-unused-modules.ts (1)
561-564:⚠️ Potential issue | 🟡 MinorThe warning's config example doesn't satisfy this rule's schema.
The example shown in the warning message only includes
suppressMissingFileEnumeratorAPIWarning: true, but the rule's schema (lines 512-537) requires eitherunusedExports: trueormissingExports: truevia theanyOfconstraint. Users copying this example would get a schema validation error.✏️ Suggested fix
-ESLint removes the FileEnumerator API since ESLint 10, which is required by the "no-unused-modules" rule. Therefore, the "no-unused-modules" rule is no-op for now before we can implement an alternative. -In the meantime, if you want to keep this rule enabled, you can suppress this warning with the "suppressMissingFileEnumeratorAPIWarning" rule option: \`import-x/no-unused-modules: ['error', { suppressMissingFileEnumeratorAPIWarning: true }]\` +ESLint removes the FileEnumerator API since ESLint 10, which is required by the "no-unused-modules" rule. Therefore, the "no-unused-modules" rule is no-op for now before we can implement an alternative. +In the meantime, you can suppress this warning by adding "suppressMissingFileEnumeratorAPIWarning: true" to your existing rule config (e.g., \`import-x/no-unused-modules: ['error', { unusedExports: true, suppressMissingFileEnumeratorAPIWarning: true }]\`).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rules/no-unused-modules.ts` around lines 561 - 564, The warning message for the "no-unused-modules" rule shows a config example that violates the rule schema by omitting the required anyOf option; update the example in the console.warn block in src/rules/no-unused-modules.ts to include one of the required options (either "unusedExports" or "missingExports") alongside "suppressMissingFileEnumeratorAPIWarning" (for example: { unusedExports: true, suppressMissingFileEnumeratorAPIWarning: true }) so a pasted config validates against the rule schema; ensure the message string references the exact option names "suppressMissingFileEnumeratorAPIWarning", "unusedExports", and "missingExports".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/rules/no-unused-modules.ts`:
- Around line 561-564: The warning message for the "no-unused-modules" rule
shows a config example that violates the rule schema by omitting the required
anyOf option; update the example in the console.warn block in
src/rules/no-unused-modules.ts to include one of the required options (either
"unusedExports" or "missingExports") alongside
"suppressMissingFileEnumeratorAPIWarning" (for example: { unusedExports: true,
suppressMissingFileEnumeratorAPIWarning: true }) so a pasted config validates
against the rule schema; ensure the message string references the exact option
names "suppressMissingFileEnumeratorAPIWarning", "unusedExports", and
"missingExports".
Another step toward ESLint 10 support (#438).
Important
Refactor
no-unused-modulesrule to be a no-op on ESLint 10 or later, adding a suppressible warning for the removedFileEnumeratorAPI.no-unused-modulesrule is now a no-op on ESLint 10 or later due to removal ofFileEnumeratorAPI.suppressMissingFileEnumeratorAPIWarningoption.FileEnumeratorandshouldUseFlatConfigaccess insidelistFilesUsingFileEnumerator()to avoid accessing removed APIs unless invoked.suppressMissingFileEnumeratorAPIWarningtoOptionsinno-unused-modules.ts.create()function ifFileEnumeratoris not present and suppression option is not set.This description was created by
for c815920. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation