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: 49077b5 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 |
📝 WalkthroughWalkthroughThis PR adds four new ESLint rules for detecting and preventing barrel file anti-patterns (avoid-barrel-files, avoid-importing-barrel-files, avoid-namespace-import, avoid-re-export-all) along with comprehensive documentation, test coverage, and configuration updates. Includes a new dependency on eslint-barrel-file-utils. Changes
Sequence DiagramsequenceDiagram
participant ESLint as ESLint Engine
participant ImportVisitor as Import Declaration Visitor
participant Resolver as Module Resolver
participant FileSystem as File System
participant BarrelUtils as Barrel File Utils
participant GraphCalc as Graph Size Calculator
ESLint->>ImportVisitor: Traverse ImportDeclaration nodes
ImportVisitor->>Resolver: Resolve import source path
Resolver-->>ImportVisitor: Return resolved file path
ImportVisitor->>FileSystem: Check if is_bare_module or local
alt Bare Module Specifier (Cached)
FileSystem-->>ImportVisitor: Return cached result
else Uncached or Local Module
ImportVisitor->>FileSystem: Read target file contents
FileSystem-->>BarrelUtils: Provide file AST
BarrelUtils-->>ImportVisitor: Determine if barrel file
alt Is Barrel File
ImportVisitor->>GraphCalc: Calculate module graph size
GraphCalc-->>ImportVisitor: Return graph size
ImportVisitor->>ImportVisitor: Compare against threshold
ImportVisitor-->>ESLint: Report violation (if exceeds limit)
else Not Barrel File
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.02% -1.09%
==========================================
Files 96 100 +4
Lines 4952 5062 +110
Branches 1860 1881 +21
==========================================
+ Hits 4759 4810 +51
- Misses 192 251 +59
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.
| "import-x/avoid-barrel-files": ["error", { | ||
| "amountOfExportsToConsiderModuleAsBarrel": 5 | ||
| }] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the default value of amountOfExportsToConsiderModuleAsBarrel
rg -n 'amountOfExportsToConsiderModuleAsBarrel' --type=ts -C3Repository: un-ts/eslint-plugin-import-x
Length of output: 8024
Correct the default value for amountOfExportsToConsiderModuleAsBarrel in the documentation.
The documentation shows 5, but the actual default in the implementation (src/rules/avoid-barrel-files.ts, line 10) is 3. Update the example or explanation to reflect the correct default.
🤖 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 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.
There was a problem hiding this comment.
@43081j Yeah, probably update docs with the default value.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| "@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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
curl -s https://registry.npmjs.org/eslint-barrel-file-utils | jq -c '{name: .name, versions: .["dist-tags"], latest_version: .["dist-tags"].latest, published_date: .time.modified, version_0_0_15: .versions["0.0.15"]}'Repository: un-ts/eslint-plugin-import-x
Length of output: 3678
Verify eslint-barrel-file-utils package maturity.
This is a 0.0.x pre-release dependency with only a single published version (0.0.15). The package is a native Rust addon compiled via NAPI, which adds maintenance complexity. Since ^0.0.15 pins to exactly 0.0.15, any future version bump will require an explicit upgrade. Confirm the package's stability and API is suitable for a production dependency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at 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.
| | Name | Description | 💼 | ⚠️ | 🚫 | 🔧 | 💡 | ❌ | | ||
| | :------------------------------------------------------------------------- | :----------------------------- | :-- | :-- | :-- | :-- | :-- | :-- | | ||
| | [avoid-importing-barrel-files](docs/rules/avoid-importing-barrel-files.md) | Forbid importing barrel files. | | | | | | | | ||
|
|
There was a problem hiding this comment.
avoid-importing-barrel-files appears misplaced — should be in the Performance section with the other barrel rules.
This rule is rendered in its own unnamed table between the auto-generated rules header and "Helpful warnings", while the other three barrel-file rules (avoid-barrel-files, avoid-namespace-import, avoid-re-export-all) are grouped under the new "Performance" subsection at lines 284–291. Move this rule into the Performance table for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 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.
| ### Performance | ||
|
|
||
| | Name | Description | 💼 | ⚠️ | 🚫 | 🔧 | 💡 | ❌ | | ||
| | :------------------------------------------------------------- | :------------------------------------ | :-- | :-- | :-- | :-- | :-- | :-- | | ||
| | [avoid-barrel-files](docs/rules/avoid-barrel-files.md) | Forbid authoring of barrel files. | | | | | | | | ||
| | [avoid-namespace-import](docs/rules/avoid-namespace-import.md) | Forbid namespace imports. | | | | | | | | ||
| | [avoid-re-export-all](docs/rules/avoid-re-export-all.md) | Forbid re-exporting \* from a module. | | | | | | | | ||
|
|
There was a problem hiding this comment.
TOC is missing the new "Performance" subsection.
The Table of Contents (lines 36–39) lists "Helpful warnings", "Module systems", "Static analysis", and "Style guide" under Rules, but does not include the newly added "Performance" subsection. Add a - [Performance](#performance) entry to the TOC.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 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.
| if (n.type === 'ExportDefaultDeclaration') { | ||
| if ( | ||
| n.declaration.type === 'FunctionDeclaration' || | ||
| n.declaration.type === 'CallExpression' | ||
| ) { | ||
| declarations += 1 | ||
| } else if (n.declaration.type === 'ObjectExpression') { | ||
| exports += n.declaration.properties.length | ||
| } else { | ||
| exports += 1 | ||
| } | ||
| } |
There was a problem hiding this comment.
export default class is counted as an export rather than a declaration.
FunctionDeclaration inside ExportDefaultDeclaration increments declarations (line 79), but ClassDeclaration falls through to the else branch and increments exports (line 83). This inconsistency means a file with export default class Foo { ... } is slightly more likely to be flagged as a barrel file than export default function foo() { ... }, even though both are real declarations.
Suggested fix
if (n.type === 'ExportDefaultDeclaration') {
if (
n.declaration.type === 'FunctionDeclaration' ||
+ n.declaration.type === 'ClassDeclaration' ||
n.declaration.type === 'CallExpression'
) {
declarations += 1🤖 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 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).
| const cache: Record< | ||
| string, | ||
| { isBarrelFile: boolean; moduleGraphSize: number } | ||
| > = {} |
There was a problem hiding this comment.
Module-level cache is never invalidated and leaks across lint runs.
The cache object lives at module scope, so in long-lived processes (IDE integrations, eslint --cache, or eslint_d) it persists indefinitely. If a dependency is updated, the stale cached result will continue to be served until the process restarts.
Consider either:
- Scoping the cache inside
create()(trades performance for correctness), - Using a
WeakRef-based or time-bounded cache, or - At minimum, keying on
resolvedPath.pathrather than just the specifier string so different resolution contexts don't collide.
🤖 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 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.
| if (!resolvedPath.path) { | ||
| throw new ResolveError('NotFound', resolvedPath.error) | ||
| } |
There was a problem hiding this comment.
Unhandled throw outside try-catch will crash the linter.
If resolver.sync succeeds without setting resolvedPath.error but returns a falsy path, the throw new ResolveError(...) on line 226 is outside the try-catch block and will propagate as an uncaught exception, crashing the ESLint process.
This should return gracefully, consistent with the catch block above:
🐛 Proposed fix
- if (!resolvedPath.path) {
- throw new ResolveError('NotFound', resolvedPath.error)
- }
+ if (!resolvedPath.path) {
+ return
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!resolvedPath.path) { | |
| throw new ResolveError('NotFound', resolvedPath.error) | |
| } | |
| if (!resolvedPath.path) { | |
| return | |
| } |
🤖 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 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.
| throw new ResolveError('NotFound', resolvedPath.error) | ||
| } | ||
|
|
||
| const fileContent = readFileSync(resolvedPath.path, 'utf8') |
There was a problem hiding this comment.
readFileSync can throw if the file is unreadable — unhandled exception will crash the linter.
The resolved path may point to a file that is deleted between resolution and read (TOCTOU), inaccessible due to permissions, or a broken symlink. Wrap this in a try-catch for robustness, consistent with the resolution error handling above.
🛡️ Proposed fix
- const fileContent = readFileSync(resolvedPath.path, 'utf8')
+ let fileContent: string
+ try {
+ fileContent = readFileSync(resolvedPath.path, 'utf8')
+ } catch {
+ return
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const fileContent = readFileSync(resolvedPath.path, 'utf8') | |
| let fileContent: string | |
| try { | |
| fileContent = readFileSync(resolvedPath.path, 'utf8') | |
| } catch { | |
| return | |
| } |
🤖 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` at 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.
| const resolver = new ResolverFactory({ | ||
| tsconfig, | ||
| alias, | ||
| conditionNames: exportConditions, | ||
| mainFields, | ||
| extensions, | ||
| }) |
There was a problem hiding this comment.
Please re-use the eslint-plugin-import-x built-in resolve utility instead of creating a new resolver instance.
Please check other rules for their implementations.
| if (options?.allowList?.includes(moduleSpecifier)) { | ||
| return | ||
| } |
There was a problem hiding this comment.
This includes call may be called many times, and allowList may have many items. Let's create a Set first, then use Set#has here.
| if (node?.importKind === 'type') { | ||
| return | ||
| } |
There was a problem hiding this comment.
Maybe we can early return type-only import before allowList check, which could improve performance.
| if (builtinModules.includes(moduleSpecifier.replace('node:', ''))) { | ||
| return | ||
| } |
There was a problem hiding this comment.
Last time I checked, eslint-plugin-import-x's built-in resolver does support built-in module detection. Re-use that when possible.
| throw new ResolveError('NotFound', resolvedPath.error) | ||
| } | ||
|
|
||
| const fileContent = readFileSync(resolvedPath.path, 'utf8') |
There was a problem hiding this comment.
We should read files on demand here.
If isBareModuleSpecifier(moduleSpecifier) is true and the cache hit, we don't really need fileContent anymore, then we can reduce I/O access here.
https://github.com/un-ts/eslint-plugin-import-x/blob/master/src/utils/lazy-value.ts
We have a lazy utility that does the trick:
const fileContent = lazy(() => readFileSync(resolvedPath.path, 'utf8'));
is_barrel_file(fileContent());2 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