Conversation
* feat: port eslint-plugin-barrel-files This ports the rules from `eslint-plugin-barrel-files` into the package. * chore: pr fixes * chore: remove debug flag * chore: leftover debug flags * docs: update example to flat config * chore: re-use default options * chore: upgrade barrel utils * docs: update docs for barrel rules
🦋 Changeset detectedLatest commit: c51b016 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:
📝 WalkthroughWalkthroughAdds four new ESLint rules to detect and prevent barrel-file patterns (authoring, importing, namespace imports, re-export-all), with docs, tests, rule registration, a dependency ( Changes
Sequence Diagram(s)sequenceDiagram
participant ESLint as ESLint Engine
participant ImportVisitor as Import Declaration Visitor
participant Resolver as Module Resolver
participant FS as File System
participant BarrelUtils as Barrel Analyzer
participant GraphCalc as Module Graph Calculator
ESLint->>ImportVisitor: Traverse ImportDeclaration nodes
ImportVisitor->>Resolver: Resolve import source
alt unresolved / builtin / allowlisted
Resolver-->>ImportVisitor: skip
else resolved
Resolver-->>ImportVisitor: resolved path
ImportVisitor->>FS: read target file / check cache
FS-->>ImportVisitor: file contents / cached analysis
ImportVisitor->>BarrelUtils: analyze AST for barrel characteristics
BarrelUtils-->>ImportVisitor: isBarrel (true/false)
alt isBarrel
ImportVisitor->>GraphCalc: compute module graph size
GraphCalc-->>ImportVisitor: size
ImportVisitor->>ESLint: report if size > threshold
else not barrel
ImportVisitor-->>ESLint: no report
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to e908191 in 14 seconds. Click for details.
- Reviewed
1059lines of code in16files - Skipped
1files 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_cKvKRGtARZgxlftj
You can customize 
|
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: |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesFootnotes
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #456 +/- ##
==========================================
- Coverage 96.10% 95.05% -1.06%
==========================================
Files 96 100 +4
Lines 4952 5072 +120
Branches 1860 1895 +35
==========================================
+ Hits 4759 4821 +62
- Misses 192 250 +58
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (9)
src/rules/avoid-barrel-files.ts (2)
42-43: Same redundant|| defaultOptionsfallback as inavoid-namespace-import.ts.
createRulealready mergesdefaultOptionsintocontext.options. Harmless but unnecessary.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rules/avoid-barrel-files.ts` around lines 42 - 43, The create(context) function in avoid-barrel-files.ts redundantly does "const options = context.options[0] || defaultOptions"; remove the unnecessary "|| defaultOptions" fallback since the createRule helper already merges defaultOptions into context.options—just assign options from context.options[0] (or use the same safe access pattern used in avoid-namespace-import.ts) so the rule relies on the merged defaults provided by createRule.
66-68:ExportNamedDeclarationwith adeclaration(e.g.export const x = 1) is silently uncounted.When
ExportNamedDeclarationhas adeclarationproperty (inline exports likeexport const x = 1),specifiersis empty, so neitherexportsnordeclarationsis incremented. This works as a heuristic (inline exports are both a declaration and an export, so they cancel out), but it's worth noting thatexport function foo() {}is also uncounted — this could lead to unexpected results in edge cases where a file mixes inline exports with re-exports.A more explicit approach would be to count the declaration within
ExportNamedDeclarationwhen present:if (n.type === 'ExportNamedDeclaration') { exports += n.specifiers.length if (n.declaration) { declarations += 1 // or count inner declarations exports += 1 } }This makes the "cancels out" logic explicit rather than relying on the omission.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rules/avoid-barrel-files.ts` around lines 66 - 68, The ExportNamedDeclaration branch currently only adds n.specifiers.length to exports, which silently ignores inline declarations (e.g. export const x = 1 or export function foo() {}); update the logic in the export counting code to detect n.declaration and explicitly increment declarations and exports (or count inner declarations) when present so inline exports are accounted for—modify the block handling ExportNamedDeclaration (the variables exports, declarations and the AST node n) to add declarations += 1 and exports += 1 (or a suitable inner-declaration count) whenever n.declaration exists.src/rules/avoid-namespace-import.ts (1)
44-46: Redundant fallback —createRulealready mergesdefaultOptions.Since
createRule(via@typescript-eslint/utilsRuleCreator) mergesdefaultOptionsintocontext.options, the|| defaultOptionsfallback on line 46 is unnecessary. Not harmful, but it obscures the fact that defaults are already applied.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rules/avoid-namespace-import.ts` around lines 44 - 46, Remove the redundant fallback in the rule's create function: the line that sets options using "const options = context.options[0] || defaultOptions" should instead read only from context.options[0] because createRule/RuleCreator already merges defaultOptions; update the create arrow function in avoid-namespace-import (the create function referenced in this diff) to rely on context.options[0] without the "|| defaultOptions" fallback.src/rules/avoid-re-export-all.ts (1)
24-24: Unnecessary optional chaining on AST visitornodeparameter.
nodeis always defined when theExportAllDeclarationvisitor fires.node.exportKindis sufficient. Same pattern appears atavoid-barrel-files.tsline 70.Suggested fix
- if (node?.exportKind !== 'type') { + if (node.exportKind !== 'type') {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rules/avoid-re-export-all.ts` at line 24, Remove the unnecessary optional chaining on the AST visitor parameter: replace node?.exportKind with node.exportKind in the ExportAllDeclaration visitor inside avoid-re-export-all.ts; do the same for the analogous visitor in avoid-barrel-files.ts (replace any node?.exportKind uses with node.exportKind) so the code assumes the guaranteed defined node parameter when the visitor runs.test/rules/avoid-re-export-all.spec.ts (1)
9-17: Minor inconsistency: valid cases use plain strings instead oftValid()wrappers.The other new test files (
avoid-namespace-import,avoid-barrel-files) wrap valid cases withtValid(). Here,tValidis not even destructured (line 9). While plain strings are functionally valid forRuleTester, usingtValid()consistently would allow merging default options if they're added later.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/rules/avoid-re-export-all.spec.ts` around lines 9 - 17, Destructure tValid from createRuleTestCaseFunctions<typeof rule>() alongside tInvalid and replace the plain string entries in the valid array passed to ruleTester.run('avoid-re-export-all', rule, ...) with tValid(...) wrappers (e.g., tValid('export { foo } from "foo";')), so valid test cases match the pattern used in other test files and will accept merged default options if added later; keep the same test strings and only wrap them with tValid.docs/rules/avoid-importing-barrel-files.md (1)
9-14: Consider using a plainimportstatement as the "incorrect" example.The example shows
export { foo } from 'foo', which is a re-export. Since this rule targets importing barrel files, a more intuitive example would be a plain import:-export { foo } from 'foo' +import { foo } from 'foo'This better illustrates the rule's purpose to the reader. The re-export form works too (it implicitly imports), but can be confused with the
avoid-re-export-allrule's domain.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/rules/avoid-importing-barrel-files.md` around lines 9 - 14, Replace the re-export example `export { foo } from 'foo'` with a plain import form to make the rule clearer: use `import { foo } from 'foo'` as the "incorrect" example so readers see importing a barrel file (symbol: foo) directly, avoiding confusion with re-export rules like `avoid-re-export-all`; update the surrounding explanatory text if needed to mention this is an import-based example.src/rules/avoid-importing-barrel-files.ts (3)
50-55: UnnecessaryreplaceAll("'", '')—node.source.valueis already unquoted.
node.source.valuereturns the raw string value of the import specifier without surrounding quotes. ThereplaceAll("'", '')call is dead code that adds confusion. Additionally, the function can be simplified.♻️ Simplified implementation
const isBareModuleSpecifier = (specifier: string): boolean => { - if (specifier && specifier.length > 0) { - return /[`@a-zA-Z`]/.test(specifier.replaceAll("'", '')[0]) - } - return false + return specifier.length > 0 && /^[`@a-zA-Z`]/.test(specifier) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rules/avoid-importing-barrel-files.ts` around lines 50 - 55, The isBareModuleSpecifier function contains an unnecessary replaceAll("'", '') since the import specifier (e.g., node.source.value) is already unquoted; remove that dead code and simplify the logic: ensure specifier is non-empty and test only its first character against a concise regex (e.g., check if specifier[0] matches /[A-Za-z@]/ or use a single regex like /^[A-Za-z@]/.test(specifier)) inside isBareModuleSpecifier to return a boolean, keeping the function small and clear.
160-162: UsecreateRule'sdefaultOptionsmechanism instead of manual fallback.
defaultOptions: []bypasses the deep-merge thatcreateRule(from@typescript-eslint/utils'sRuleCreator) provides. The manual|| defaultOptionson line 162 works only when the user supplies no options at all — when partial options are provided, individual defaults come from the JSON Schema'sdefaultkeywords instead, which could drift out of sync withdefaultOptions.Pass the defaults through
createRuleso they are deep-merged automatically:♻️ Proposed fix
- defaultOptions: [], + defaultOptions: [defaultOptions], create(context) { - const options = context.options[0] || defaultOptions + const options = context.options[0]!🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rules/avoid-importing-barrel-files.ts` around lines 160 - 162, The rule currently defines defaultOptions and then does a manual fallback in create(context) using context.options[0] || defaultOptions; instead, supply defaultOptions to createRule so the rule creator will deep-merge defaults and JSON Schema "default" values automatically; remove the manual fallback and read the already-merged options directly (replace the use of context.options[0] || defaultOptions inside create with the options parameter provided by createRule), keeping the same option shape and preserving the JSON Schema defaults.
232-306: Extract duplicated report logic into a helper.The
context.report(...)call with identicalmessageId,node, anddatashape appears three times (lines 252–262, 269–281, 295–304). A small helper would reduce duplication and ensure consistency.♻️ Sketch
+ const reportBarrel = (moduleGraphSize: number) => { + if (moduleGraphSize > maxModuleGraphSizeAllowed) { + context.report({ + node: node.source, + messageId: 'avoidImport', + data: { + amount: moduleGraphSize, + specifier: moduleSpecifier, + maxModuleGraphSizeAllowed, + }, + }) + } + }Then each branch simply calls
reportBarrel(moduleGraphSize).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rules/avoid-importing-barrel-files.ts` around lines 232 - 306, There is duplicated context.report(...) logic in avoid-importing-barrel-files.ts across the three spots that check moduleGraphSize; extract a small helper function (e.g., reportBarrel) near the top of the rule file that accepts (node, specifier, moduleGraphSize) or just (moduleGraphSize, moduleSpecifier, node) and calls context.report with messageId 'avoidImport' and the consistent data shape { amount: moduleGraphSize, specifier: moduleSpecifier, maxModuleGraphSizeAllowed }; then replace the three inline context.report blocks (the ones inside the cache[moduleSpecifier] branch, the cached branch, and the local-module branch) with calls to reportBarrel(moduleGraphSize, moduleSpecifier, node.source) and remove the duplicated object literals to keep behavior identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/rules/avoid-barrel-files.md`:
- Around line 22-24: The docs show the default for
amountOfExportsToConsiderModuleAsBarrel as 5 but the implementation (rule
import-x/avoid-barrel-files, symbol amountOfExportsToConsiderModuleAsBarrel in
src/rules/avoid-barrel-files.ts) uses 3; update the example block in
docs/rules/avoid-barrel-files.md to show 3 (or explicitly note the implemented
default of 3) so the documentation matches the actual default value used by the
rule.
In `@package.json`:
- Line 82: The dependency "eslint-barrel-file-utils" is a 0.0.x native Rust
N-API addon with only one published version; verify its maturity by checking its
repository, issue/activity history, release cadence, binary compatibility
guarantees, and CI/release artifacts for all target platforms, then decide
whether to keep, replace, or vendor it. If keeping, document upgrade and rebuild
expectations and lock the dependency explicitly (or add a postinstall build
check) and add a fallback/alternative (e.g., a pure-JS implementation or
different package) in case native builds fail; if replacing, update package.json
to the chosen alternative and remove "eslint-barrel-file-utils" and update any
references to its API. Ensure references to the package name
"eslint-barrel-file-utils" and any usages in the codebase are updated
accordingly and include a short note in the PR about the chosen strategy for
native addon maintenance.
In `@README.md`:
- Around line 256-259: The README currently renders the
avoid-importing-barrel-files rule row in a stray table; move the table row for
avoid-importing-barrel-files into the "Performance" subsection table where
avoid-barrel-files, avoid-namespace-import, and avoid-re-export-all are listed
so all barrel-related rules are grouped together, keeping the same columns and
icons for consistency and removing the orphan table entry; ensure the
Performance table ordering matches the other barrel rules and that there are no
duplicate rows left behind.
- Around line 284-291: Add the missing "Performance" subsection to the README
Table of Contents by inserting a `- [Performance](`#performance`)` entry in the
Rules TOC near the existing entries "Helpful warnings", "Module systems",
"Static analysis", and "Style guide" so the newly added "Performance" section
(header "### Performance") appears in the navigation; update the TOC block where
those four entries are listed to include this new list item.
In `@src/rules/avoid-barrel-files.ts`:
- Around line 74-85: The ExportDefaultDeclaration branch in
avoid-barrel-files.ts treats export default class as an export rather than a
declaration; update the conditional that checks n.declaration.type (currently
testing 'FunctionDeclaration' and 'CallExpression') to also treat
'ClassDeclaration' as a declaration so that export default class increments
declarations (the declarations variable) instead of exports; adjust the branches
handling in the ExportDefaultDeclaration case accordingly (refer to
ExportDefaultDeclaration and the declarations/exports variables).
In `@src/rules/avoid-importing-barrel-files.ts`:
- Around line 41-44: The module-level cache variable `cache` leaks across lint
runs and should be made safe: move the `cache: Record<string, { isBarrelFile:
boolean; moduleGraphSize: number }>` into the `create()` factory so it is
re-created per linter invocation (or alternatively implement a
time-bounded/WeakRef cache), and when keeping a shared cache at least key
entries by `resolvedPath.path` combined with the specifier to avoid
resolution-context collisions; update all references to `cache` in functions
inside this module (e.g., where `isBarrelFile` and `moduleGraphSize` are
read/written) to use the new scoped cache or new composite key.
- Line 229: The call to readFileSync (reading resolvedPath.path) can throw and
should be wrapped in a try-catch like the earlier resolution error handling;
update the code in avoid-importing-barrel-files (around the readFileSync usage)
to catch errors from readFileSync(resolvedPath.path, 'utf8'), handle/log the
error consistently (e.g., use the same warning/error path used for resolution
failures), and skip processing that file when an exception occurs so the linter
does not crash.
- Around line 225-227: The throw of ResolveError when resolvedPath.path is falsy
is outside the surrounding try-catch and can crash the linter; instead of
throwing in the if (!resolvedPath.path) branch, handle it the same way as the
catch block by returning gracefully (e.g., return undefined or early exit) and
optionally propagate the resolver error into the existing error handling path;
update the check around resolver.sync/resolvedPath (the resolvedPath variable
and the if (!resolvedPath.path) branch) to avoid throwing and mirror the
catch-path behavior so the rule does not raise an uncaught exception.
---
Nitpick comments:
In `@docs/rules/avoid-importing-barrel-files.md`:
- Around line 9-14: Replace the re-export example `export { foo } from 'foo'`
with a plain import form to make the rule clearer: use `import { foo } from
'foo'` as the "incorrect" example so readers see importing a barrel file
(symbol: foo) directly, avoiding confusion with re-export rules like
`avoid-re-export-all`; update the surrounding explanatory text if needed to
mention this is an import-based example.
In `@src/rules/avoid-barrel-files.ts`:
- Around line 42-43: The create(context) function in avoid-barrel-files.ts
redundantly does "const options = context.options[0] || defaultOptions"; remove
the unnecessary "|| defaultOptions" fallback since the createRule helper already
merges defaultOptions into context.options—just assign options from
context.options[0] (or use the same safe access pattern used in
avoid-namespace-import.ts) so the rule relies on the merged defaults provided by
createRule.
- Around line 66-68: The ExportNamedDeclaration branch currently only adds
n.specifiers.length to exports, which silently ignores inline declarations (e.g.
export const x = 1 or export function foo() {}); update the logic in the export
counting code to detect n.declaration and explicitly increment declarations and
exports (or count inner declarations) when present so inline exports are
accounted for—modify the block handling ExportNamedDeclaration (the variables
exports, declarations and the AST node n) to add declarations += 1 and exports
+= 1 (or a suitable inner-declaration count) whenever n.declaration exists.
In `@src/rules/avoid-importing-barrel-files.ts`:
- Around line 50-55: The isBareModuleSpecifier function contains an unnecessary
replaceAll("'", '') since the import specifier (e.g., node.source.value) is
already unquoted; remove that dead code and simplify the logic: ensure specifier
is non-empty and test only its first character against a concise regex (e.g.,
check if specifier[0] matches /[A-Za-z@]/ or use a single regex like
/^[A-Za-z@]/.test(specifier)) inside isBareModuleSpecifier to return a boolean,
keeping the function small and clear.
- Around line 160-162: The rule currently defines defaultOptions and then does a
manual fallback in create(context) using context.options[0] || defaultOptions;
instead, supply defaultOptions to createRule so the rule creator will deep-merge
defaults and JSON Schema "default" values automatically; remove the manual
fallback and read the already-merged options directly (replace the use of
context.options[0] || defaultOptions inside create with the options parameter
provided by createRule), keeping the same option shape and preserving the JSON
Schema defaults.
- Around line 232-306: There is duplicated context.report(...) logic in
avoid-importing-barrel-files.ts across the three spots that check
moduleGraphSize; extract a small helper function (e.g., reportBarrel) near the
top of the rule file that accepts (node, specifier, moduleGraphSize) or just
(moduleGraphSize, moduleSpecifier, node) and calls context.report with messageId
'avoidImport' and the consistent data shape { amount: moduleGraphSize,
specifier: moduleSpecifier, maxModuleGraphSizeAllowed }; then replace the three
inline context.report blocks (the ones inside the cache[moduleSpecifier] branch,
the cached branch, and the local-module branch) with calls to
reportBarrel(moduleGraphSize, moduleSpecifier, node.source) and remove the
duplicated object literals to keep behavior identical.
In `@src/rules/avoid-namespace-import.ts`:
- Around line 44-46: Remove the redundant fallback in the rule's create
function: the line that sets options using "const options = context.options[0]
|| defaultOptions" should instead read only from context.options[0] because
createRule/RuleCreator already merges defaultOptions; update the create arrow
function in avoid-namespace-import (the create function referenced in this diff)
to rely on context.options[0] without the "|| defaultOptions" fallback.
In `@src/rules/avoid-re-export-all.ts`:
- Line 24: Remove the unnecessary optional chaining on the AST visitor
parameter: replace node?.exportKind with node.exportKind in the
ExportAllDeclaration visitor inside avoid-re-export-all.ts; do the same for the
analogous visitor in avoid-barrel-files.ts (replace any node?.exportKind uses
with node.exportKind) so the code assumes the guaranteed defined node parameter
when the visitor runs.
In `@test/rules/avoid-re-export-all.spec.ts`:
- Around line 9-17: Destructure tValid from createRuleTestCaseFunctions<typeof
rule>() alongside tInvalid and replace the plain string entries in the valid
array passed to ruleTester.run('avoid-re-export-all', rule, ...) with
tValid(...) wrappers (e.g., tValid('export { foo } from "foo";')), so valid test
cases match the pattern used in other test files and will accept merged default
options if added later; keep the same test strings and only wrap them with
tValid.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/rules/avoid-barrel-files.md`:
- Around line 19-25: Update the ambiguous wording in the docs for the
"amountOfExportsToConsiderModuleAsBarrel" option: replace "Anything over will
trigger the rule." with an explicit phrase that matches the actual
implementation (e.g., "Anything at or over
<amountOfExportsToConsiderModuleAsBarrel> will trigger the rule." if the check
is >=, or change the example/wording to "Anything over ..." and adjust the
example exports if the check is >); ensure the text around the option name
"amountOfExportsToConsiderModuleAsBarrel" and any example export counts reflect
the chosen operator so docs and implementation are consistent.
- Around line 9-15: The "incorrect" example shows only three exports but the
rule checks exports > amountOfExportsToConsiderModuleAsBarrel (default 3), so
update the example in docs/rules/avoid-barrel-files.md to include a fourth
export so it demonstrates 4+ exports; specifically add an additional export line
(e.g., a fourth named export) to the existing block so the example will trigger
the rule given the default threshold and clarify that the threshold is 3
(amountOfExportsToConsiderModuleAsBarrel).
| Examples of **incorrect** code for this rule: | ||
|
|
||
| ```js | ||
| export { foo } from 'foo' | ||
| export { bar } from 'bar' | ||
| export { baz } from 'baz' | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'amountOfExportsToConsiderModuleAsBarrel|exportCount|reExportCount' --type=ts -C5 src/rules/avoid-barrel-files.ts | head -100Repository: un-ts/eslint-plugin-import-x
Length of output: 1413
Update the example to show 4 or more exports to match the default threshold behavior.
The rule implementation uses exports > amountOfExportsToConsiderModuleAsBarrel (strictly greater than). With the default threshold of 3, the rule only triggers for 4+ exports. The current example shows exactly 3 exports, which would not trigger the rule—making it invalid as an "incorrect code" example. Add a 4th export to the example:
Updated example
export { foo } from 'foo'
export { bar } from 'bar'
export { baz } from 'baz'
export { qux } from 'qux'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/rules/avoid-barrel-files.md` around lines 9 - 15, The "incorrect"
example shows only three exports but the rule checks exports >
amountOfExportsToConsiderModuleAsBarrel (default 3), so update the example in
docs/rules/avoid-barrel-files.md to include a fourth export so it demonstrates
4+ exports; specifically add an additional export line (e.g., a fourth named
export) to the existing block so the example will trigger the rule given the
default threshold and clarify that the threshold is 3
(amountOfExportsToConsiderModuleAsBarrel).
| This rule has the following options, with these defaults: | ||
|
|
||
| ```js | ||
| "import-x/avoid-barrel-files": ["error", { | ||
| "amountOfExportsToConsiderModuleAsBarrel": 3 | ||
| }] | ||
| ``` |
There was a problem hiding this comment.
Consider adding a note clarifying "over" vs "at or over" for the threshold.
The option description on line 29 says "Anything over will trigger the rule." This is ambiguous — if the check is >=, then "at or over" is more accurate; if it is >, then the example needs an additional export. Either way, clarify the semantics.
✏️ Suggested wording if the check is `>=`
-This option sets the number of exports that will be considered a barrel file. Anything over will trigger the rule.
+This option sets the number of exports that will be considered a barrel file. At or above this number of exports, the rule will trigger.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/rules/avoid-barrel-files.md` around lines 19 - 25, Update the ambiguous
wording in the docs for the "amountOfExportsToConsiderModuleAsBarrel" option:
replace "Anything over will trigger the rule." with an explicit phrase that
matches the actual implementation (e.g., "Anything at or over
<amountOfExportsToConsiderModuleAsBarrel> will trigger the rule." if the check
is >=, or change the example/wording to "Anything over ..." and adjust the
example exports if the check is >); ensure the text around the option name
"amountOfExportsToConsiderModuleAsBarrel" and any example export counts reflect
the chosen operator so docs and implementation are consistent.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
240-333:⚠️ Potential issue | 🔴 CriticalAdd
category: 'Performance'toavoid-importing-barrel-files.tsmetadata.Three of the four rules listed in the Performance section have
category: 'Performance'declared in their metadata (avoid-barrel-files.ts, avoid-namespace-import.ts, avoid-re-export-all.ts), butavoid-importing-barrel-files.tsis missing this declaration. The generation script usesruleListSplit: 'meta.docs.category'in.eslint-doc-generatorrc.jsto organize rules by category, so the rule lacking this metadata will not appear in the Performance section if the README is regenerated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 240 - 333, The rule avoid-importing-barrel-files.ts is missing meta.docs.category; open that file, locate the exported metadata object (meta) and add docs.category: 'Performance' (e.g., ensure meta.docs.category = 'Performance' alongside existing docs fields) so the generator (ruleListSplit: 'meta.docs.category') places it under the Performance section.
🧹 Nitpick comments (5)
README.md (1)
287-287:avoid-namespace-importand the existingno-namespacerule (Style guide) appear to overlap in purpose.
no-namespace(line 327) already "Forbid namespace (a.k.a. 'wildcard'*) imports" with an auto-fix. Consider adding a brief disambiguation to each rule's documentation clarifying their distinction (e.g., performance-oriented detection vs. style-oriented enforcement with auto-fix) to avoid user confusion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 287, The two rules avoid-namespace-import and no-namespace overlap in purpose; update the README table entry for each rule and the rule docs for avoid-namespace-import and no-namespace so they include a one-line disambiguation describing their difference (e.g., "avoid-namespace-import: performance/semantic detection" vs "no-namespace: style enforcement with automatic fix"), add a short note in each doc pointing to the other rule for clarity, and ensure the README row for avoid-namespace-import is updated to reflect this distinction.src/rules/avoid-importing-barrel-files.ts (4)
193-195: Optional chaining onoptionsis unnecessary.
optionsis always assigned on line 162 (falling back todefaultOptions), sooptions?.allowList?.includes(...)can safely beoptions.allowList.includes(...). The extra?.obscures thatoptionsis guaranteed non-nullish.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rules/avoid-importing-barrel-files.ts` around lines 193 - 195, The optional chaining on options is unnecessary because options is always assigned (falls back to defaultOptions); replace the expression options?.allowList?.includes(moduleSpecifier) with a direct access options.allowList.includes(moduleSpecifier) to make the guaranteed non-nullability explicit—update the check in the same block where moduleSpecifier is compared and keep the early return behavior unchanged.
232-308: Triplicatedcontext.reportcalls — extract a helper to reduce duplication.The
context.report(...)block is repeated nearly identically at lines 255–263, 274–282, and 298–306. Additionally,let isBarrelFiledeclared at line 232 is only assigned in theifbranch (line 241); theelsebranch re-declares its ownconst isBarrelFile(line 290), leaving the outerletunused there.Extracting the analysis + report logic into a local helper would collapse the three branches and eliminate the dangling variable:
♻️ Sketch of extracted helper
+ const analyzeAndReport = ( + filePath: string, + content: string, + specifier: string, + ) => { + const barrel = is_barrel_file( + content, + amountOfExportsToConsiderModuleAsBarrel, + ) + const graphSize = barrel + ? count_module_graph_size(filePath, resolutionOptions) + : -1 + if (graphSize > maxModuleGraphSizeAllowed) { + context.report({ + node: node.source, + messageId: 'avoidImport', + data: { + amount: graphSize, + specifier, + maxModuleGraphSizeAllowed, + }, + }) + } + return { isBarrelFile: barrel, moduleGraphSize: graphSize } + }Then the three branches collapse to calls of
analyzeAndReport, with the bare-specifier branch storing the result incache.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rules/avoid-importing-barrel-files.ts` around lines 232 - 308, Extract the repeated analysis + reporting into a small local helper (e.g., analyzeAndReport) that takes the moduleSpecifier, resolvedPath.path, fileContent, resolutionOptions and returns/handles moduleGraphSize and calls context.report when moduleGraphSize > maxModuleGraphSizeAllowed; use is_barrel_file and count_module_graph_size inside that helper and refer to context.report there to remove the three duplicated context.report blocks; in the bare-module branch compute and store the result in cache[moduleSpecifier] and call the helper to perform reporting, and in the local-file branch call the same helper (remove the outer dangling let isBarrelFile and the inner redeclaration) so both branches reuse analyzeAndReport and avoid duplicate logic.
160-162: Partial user options will lose defaults not covered by the JSON schemadefaultkeyword.
context.options[0] || defaultOptionsis all-or-nothing: if the user supplies any options object,defaultOptionsis entirely bypassed. While ESLint/ajv fills indefaultvalues declared in the schema,tsconfigandaliashave no schema-leveldefault. This works today because they default toundefined, but if new required fields with non-trivial defaults are added later this pattern will silently drop them.Consider spreading defaults to be defensive:
♻️ Suggested improvement
- const options = context.options[0] || defaultOptions + const options = { ...defaultOptions, ...context.options[0] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rules/avoid-importing-barrel-files.ts` around lines 160 - 162, The current options assignment in create(context) uses an all-or-nothing pattern (const options = context.options[0] || defaultOptions) which bypasses defaultOptions if any user object exists; change it to merge defaults with the user-provided object so missing defaults are preserved (e.g., compute options by shallow-spreading defaultOptions and then spreading context.options[0] or an empty object), updating the code that references defaultOptions and the local options variable accordingly.
50-55:replaceAll("'", '')is unnecessary —node.source.valueis already unquoted.ESLint's
node.source.valueprovides the raw string value without surrounding quotes, so thereplaceAllis dead code. Also, Node.js#subpathimports (package.jsonimportsfield) start with#and won't be recognized as bare specifiers — they'll be re-analyzed on every import rather than cached. This may be intentional but is worth noting.♻️ Suggested simplification
const isBareModuleSpecifier = (specifier: string): boolean => { - if (specifier && specifier.length > 0) { - return /[`@a-zA-Z`]/.test(specifier.replaceAll("'", '')[0]) - } - return false + return specifier.length > 0 && /^[`@a-zA-Z`]/.test(specifier) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rules/avoid-importing-barrel-files.ts` around lines 50 - 55, The helper isBareModuleSpecifier unnecessarily calls specifier.replaceAll("'", '') even though node.source.value is already unquoted; update isBareModuleSpecifier to read the first character directly (e.g., specifier[0]) and test that char with the regexp instead of replacing quotes — keep the function name isBareModuleSpecifier and the parameter specifier intact; optionally, if you want imports starting with '#' to be treated as bare (instead of re-analyzed), include '#' in the character class (e.g., /[`@a-zA-Z`#]/) when testing the first char.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@README.md`:
- Around line 240-333: The rule avoid-importing-barrel-files.ts is missing
meta.docs.category; open that file, locate the exported metadata object (meta)
and add docs.category: 'Performance' (e.g., ensure meta.docs.category =
'Performance' alongside existing docs fields) so the generator (ruleListSplit:
'meta.docs.category') places it under the Performance section.
---
Duplicate comments:
In `@src/rules/avoid-importing-barrel-files.ts`:
- Around line 41-44: The module-level cache variable `cache` persists across
lint runs and can produce stale results in long-lived processes; move its
declaration inside the `create()` factory (or replace it with an
invalidatable/bounded cache strategy tied to the rule instance) so it is reset
per lint invocation. Locate the `cache` symbol in this file and either relocate
its initialization into the `create()` function or implement a cache class with
clear/size limits and ensure `isBarrelFile`/`moduleGraphSize` entries are
invalidated when file dependencies change. Update all references to `cache`
within functions like the rule visitor to use the new scoped or invalidatable
cache.
- Around line 240-252: The cache currently keys only by moduleSpecifier which
can collide across different resolution options; change the caching key to
include resolution-relevant data (for example combine moduleSpecifier with
resolvedPath.path and/or a serialized form of resolutionOptions such as
exportConditions and extensions) before accessing cache[moduleSpecifier]; update
usages around cache, is_barrel_file, count_module_graph_size, and
resolvedPath.path so cached entries are stored and retrieved by this composite
key to avoid stale results across different resolver configs.
---
Nitpick comments:
In `@README.md`:
- Line 287: The two rules avoid-namespace-import and no-namespace overlap in
purpose; update the README table entry for each rule and the rule docs for
avoid-namespace-import and no-namespace so they include a one-line
disambiguation describing their difference (e.g., "avoid-namespace-import:
performance/semantic detection" vs "no-namespace: style enforcement with
automatic fix"), add a short note in each doc pointing to the other rule for
clarity, and ensure the README row for avoid-namespace-import is updated to
reflect this distinction.
In `@src/rules/avoid-importing-barrel-files.ts`:
- Around line 193-195: The optional chaining on options is unnecessary because
options is always assigned (falls back to defaultOptions); replace the
expression options?.allowList?.includes(moduleSpecifier) with a direct access
options.allowList.includes(moduleSpecifier) to make the guaranteed
non-nullability explicit—update the check in the same block where
moduleSpecifier is compared and keep the early return behavior unchanged.
- Around line 232-308: Extract the repeated analysis + reporting into a small
local helper (e.g., analyzeAndReport) that takes the moduleSpecifier,
resolvedPath.path, fileContent, resolutionOptions and returns/handles
moduleGraphSize and calls context.report when moduleGraphSize >
maxModuleGraphSizeAllowed; use is_barrel_file and count_module_graph_size inside
that helper and refer to context.report there to remove the three duplicated
context.report blocks; in the bare-module branch compute and store the result in
cache[moduleSpecifier] and call the helper to perform reporting, and in the
local-file branch call the same helper (remove the outer dangling let
isBarrelFile and the inner redeclaration) so both branches reuse
analyzeAndReport and avoid duplicate logic.
- Around line 160-162: The current options assignment in create(context) uses an
all-or-nothing pattern (const options = context.options[0] || defaultOptions)
which bypasses defaultOptions if any user object exists; change it to merge
defaults with the user-provided object so missing defaults are preserved (e.g.,
compute options by shallow-spreading defaultOptions and then spreading
context.options[0] or an empty object), updating the code that references
defaultOptions and the local options variable accordingly.
- Around line 50-55: The helper isBareModuleSpecifier unnecessarily calls
specifier.replaceAll("'", '') even though node.source.value is already unquoted;
update isBareModuleSpecifier to read the first character directly (e.g.,
specifier[0]) and test that char with the regexp instead of replacing quotes —
keep the function name isBareModuleSpecifier and the parameter specifier intact;
optionally, if you want imports starting with '#' to be treated as bare (instead
of re-analyzed), include '#' in the character class (e.g., /[`@a-zA-Z`#]/) when
testing the first char.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/rules/avoid-importing-barrel-files.ts (2)
192-268: Triple-duplicatedcontext.reportblock — extract a helper.The same report call (with identical
messageId,datashape, and threshold check) appears at lines 214-224, 231-243, and 257-267. A small local helper eliminates the repetition and the risk of the three copies drifting apart.Also, the
let isBarrelFileon line 192 is only assigned inside the firstifbranch and shadowed by aconstin theelsebranch (line 250) — it can be scoped more tightly.♻️ Sketch of a helper to deduplicate
+ const reportIfBarrel = ( + specifier: string, + filePath: string, + fileContent: string, + ) => { + const barrel = is_barrel_file( + fileContent, + amountOfExportsToConsiderModuleAsBarrel, + ) + if (!barrel) return + const moduleGraphSize = count_module_graph_size( + filePath, + resolutionOptions, + ) + if (moduleGraphSize > maxModuleGraphSizeAllowed) { + context.report({ + node: node.source, + messageId: 'avoidImport', + data: { + amount: moduleGraphSize, + specifier, + maxModuleGraphSizeAllowed, + }, + }) + } + }The bare-module cache path would call this helper on a cache miss and store the result; the cache-hit path would re-check the stored
moduleGraphSizewithout re-computing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rules/avoid-importing-barrel-files.ts` around lines 192 - 268, The code duplicates the context.report call and the threshold check across three places and also unnecessarily declares let isBarrelFile at top-level then shadows it; extract a small helper function (e.g., reportIfTooLarge(specifier: string, node: Node, size: number)) that performs the moduleGraphSize > maxModuleGraphSizeAllowed check and calls context.report with messageId 'avoidImport' and the same data shape, then replace the three duplicated blocks with calls to that helper, and tighten isBarrelFile's scope by removing the top-level let and declaring it where needed around calls to is_barrel_file(...) before computing moduleGraphSize via count_module_graph_size(...).
46-51: UnnecessaryreplaceAll("'", '')— AST.valuenever contains quotes.
node.source.valueis already the unquoted string, so thereplaceAllis a no-op. The truthiness check also makes thelength > 0guard redundant.♻️ Simplification
-const isBareModuleSpecifier = (specifier: string): boolean => { - if (specifier && specifier.length > 0) { - return /[`@a-zA-Z`]/.test(specifier.replaceAll("'", '')[0]) - } - return false -} +const isBareModuleSpecifier = (specifier: string): boolean => + specifier.length > 0 && /^[`@a-zA-Z`]/.test(specifier)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rules/avoid-importing-barrel-files.ts` around lines 46 - 51, The isBareModuleSpecifier function currently calls specifier.replaceAll("'", '') and checks both truthiness and length; change it to directly inspect the unquoted AST value: remove the replaceAll call and the redundant length > 0 check, simply return /[`@a-zA-Z`]/.test(specifier[0]) after ensuring specifier is a non-empty string (e.g., if (!specifier) return false), updating the function name references (isBareModuleSpecifier) accordingly.
🤖 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/avoid-importing-barrel-files.ts`:
- Around line 144-149: Change the rule's exported defaultOptions from an empty
array to an array wrapping the defaultOptions object so the framework merges
user options with defaults; i.e., replace defaultOptions: [] with
defaultOptions: [defaultOptions] and keep the existing options assignment (const
options = context.options[0] || defaultOptions) so functions like
count_module_graph_size and is_barrel_file always receive a fully populated
config.
---
Duplicate comments:
In `@src/rules/avoid-importing-barrel-files.ts`:
- Around line 37-40: The module-level const cache persists across lint runs and
should be removed; move caching inside the rule's runtime so it is scoped per
lint invocation (e.g., create a local Map inside the rule's create(context)
closure) or provide an explicit reset mechanism and call it at the start of each
run; replace the top-level cache variable with a per-run Map created in the
create function (or export/resetCache paired with the rule's create) and update
uses of cache to reference that scoped Map so results cannot become stale in
long-lived processes.
---
Nitpick comments:
In `@src/rules/avoid-importing-barrel-files.ts`:
- Around line 192-268: The code duplicates the context.report call and the
threshold check across three places and also unnecessarily declares let
isBarrelFile at top-level then shadows it; extract a small helper function
(e.g., reportIfTooLarge(specifier: string, node: Node, size: number)) that
performs the moduleGraphSize > maxModuleGraphSizeAllowed check and calls
context.report with messageId 'avoidImport' and the same data shape, then
replace the three duplicated blocks with calls to that helper, and tighten
isBarrelFile's scope by removing the top-level let and declaring it where needed
around calls to is_barrel_file(...) before computing moduleGraphSize via
count_module_graph_size(...).
- Around line 46-51: The isBareModuleSpecifier function currently calls
specifier.replaceAll("'", '') and checks both truthiness and length; change it
to directly inspect the unquoted AST value: remove the replaceAll call and the
redundant length > 0 check, simply return /[`@a-zA-Z`]/.test(specifier[0]) after
ensuring specifier is a non-empty string (e.g., if (!specifier) return false),
updating the function name references (isBareModuleSpecifier) accordingly.
| const cache: Record< | ||
| string, | ||
| { isBarrelFile: boolean; moduleGraphSize: number } | ||
| > = {} |
There was a problem hiding this comment.
Probably should use a Map here?
Also, though the bare modules are most likely dependencies, they might still change (e.g., via npm i). Maybe mark this as TODO and we can revisit this later.
There was a problem hiding this comment.
they might still change (e.g., via
npm i).
I am talking about in IDE linting here.
SukkaW
left a comment
SukkaW
left a comment
There was a problem hiding this comment.
LGTM overall, just small nitpicks.
JounQin
left a comment
JounQin
left a comment
There was a problem hiding this comment.
Sorry I don't quite have enough time on OSS these days, and thanks for all of your hard works! Most LGTM, except adding eslint-barrel-file-utils as new dependency.
| "@typescript-eslint/types": "^8.35.0", | ||
| "comment-parser": "^1.4.1", | ||
| "debug": "^4.4.1", | ||
| "eslint-barrel-file-utils": "^0.0.15", |
There was a problem hiding this comment.
This napi project is actually a tiny wrapper of oxc-resolver (https://github.com/thepassle/eslint-barrel-file-utils/blob/main/src/lib.rs) including oxc_parser, etc. crates, which makes duplicate binaries, so I think we'd better adapt it into js version instead.
I noticed that only rule avoid-importing-barrel-files is using it, so maybe inlining some js helpers should just work.
|
|
||
| Amount of exports to consider a module as a barrel file. | ||
|
|
||
| ### `exportConditions` |
There was a problem hiding this comment.
Those resolution details options should be unified by internal resolver and parser instead like other rules?
3 participants
We never merged this into main it seems 👀
Fixes #453
Important
Port eslint-plugin-barrel-files, adding rules to handle barrel files, namespace imports, and re-exports, with documentation and tests.
avoid-barrel-filesrule inavoid-barrel-files.tsto forbid authoring barrel files.avoid-importing-barrel-filesrule inavoid-importing-barrel-files.tsto forbid importing barrel files.avoid-namespace-importrule inavoid-namespace-import.tsto forbid namespace imports.avoid-re-export-allrule inavoid-re-export-all.tsto forbid re-exporting*from a module.docs/rules/avoid-barrel-files.md,docs/rules/avoid-importing-barrel-files.md,docs/rules/avoid-namespace-import.md, anddocs/rules/avoid-re-export-all.md.avoid-barrel-filesinavoid-barrel-files.spec.tsandavoid-barrel-files-ts.spec.ts.avoid-namespace-importinavoid-namespace-import.spec.ts.avoid-re-export-allinavoid-re-export-all.spec.ts.package.jsonto includeeslint-barrel-file-utilsdependency.README.mdto include new rules in the rules list.src/index.ts.This description was created by
for e908191. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores