Possible issue on handling TypeScript path aliases

[starkfire]

This issue is slightly related to https://github.com/thepassle/eslint-plugin-barrel-files/issues/9. Please feel free to close this issue should you find it out of scope. I'm also struggling to fully reproduce the issue at the moment, but I'm open to further suggestions on how I could help with it if necessary.

I realized that the plugin "quietly" struggles when dealing with TypeScript path aliases. For instance, I have a TSX file which imports types from a barrel file:

import type { Template } from "@/shared/types";

This import is done several times along the way, and it would take ESLint a while before it finally responds with a Rust thread panic.

thread '<unnamed>' panicked at src\lib.rs:147:10:
called `Result::unwrap()` on an `Err` value: NotFound("@/shared/types")
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
 ELIFECYCLE  Command failed with exit code 3221226505.

The only rule which triggers this issue is the avoid-importing-barrel-files rule, primarily the call to count_module_graph_size from eslint-barrel-file-utils.

What I mean by "quietly" struggling is that once I pass the debug: true option to the eslint-plugin-barrel-files rule, ESLint would then reveal that it's failing to resolve several local modules imported using TS path aliases (40+ times until the thread panic happens). For example:

Error: Failed to resolve importer: "D:\Users\kmccallister\Desktop\GitHub\sandbox\ts-testbed\apps\vessel\src\features\formtemplates-display\ui\atoms\FormTemplateCard.tsx", importee: "@/shared/ui/Card"
    at resolve (D:\Users\kmccallister\Desktop\GitHub\sandbox\ts-testbed\node_modules\.pnpm\eslint-barrel-file-utils@0.0.10\node_modules\eslint-barrel-file-utils\index.cjs:23:9)
    at ImportDeclaration (D:\Users\kmccallister\Desktop\GitHub\sandbox\ts-testbed\node_modules\.pnpm\eslint-plugin-barrel-files@2.0.7_eslint@8.57.0\node_modules\eslint-plugin-barrel-files\lib\rules\avoid-importing-barrel-files.js:133:26)
    at ruleErrorHandler (D:\Users\kmccallister\Desktop\GitHub\sandbox\ts-testbed\node_modules\.pnpm\eslint@8.57.0\node_modules\eslint\lib\linter\linter.js:1076:28)
    at D:\Users\kmccallister\Desktop\GitHub\sandbox\ts-testbed\node_modules\.pnpm\eslint@8.57.0\node_modules\eslint\lib\linter\safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit (D:\Users\kmccallister\Desktop\GitHub\sandbox\ts-testbed\node_modules\.pnpm\eslint@8.57.0\node_modules\eslint\lib\linter\safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (D:\Users\kmccallister\Desktop\GitHub\sandbox\ts-testbed\node_modules\.pnpm\eslint@8.57.0\node_modules\eslint\lib\linter\node-event-generator.js:297:26)
    at NodeEventGenerator.applySelectors (D:\Users\kmccallister\Desktop\GitHub\sandbox\ts-testbed\node_modules\.pnpm\eslint@8.57.0\node_modules\eslint\lib\linter\node-event-generator.js:326:22)
    at NodeEventGenerator.enterNode (D:\Users\kmccallister\Desktop\GitHub\sandbox\ts-testbed\node_modules\.pnpm\eslint@8.57.0\node_modules\eslint\lib\linter\node-event-generator.js:340:14)
    at CodePathAnalyzer.enterNode (D:\Users\kmccallister\Desktop\GitHub\sandbox\ts-testbed\node_modules\.pnpm\eslint@8.57.0\node_modules\eslint\lib\linter\code-path-analysis\code-path-analyzer.js:803:23)

For additional context:

• node version: v20.15.0
• pnpm version: 9.4.0

[thepassle]

If you want a quick fix, you can use package imports instead of aliases

A more structural fix would be the following: We should switch out the resolve function for oxc-resolver, (the current implementation uses the rust oxc_resolver crate internally but doesnt pass all the resolution options to it, also oxc-resolver wasnt published as npm package when I built this). Then we should make the resolution options configurable in the eslint plugin configuration, so you can pass your projects aliasFields

Would you be willing to create a PR for this? 🙂

[thepassle]

Hm actually, type imports should be ignored by the plugin 🤔 https://github.com/thepassle/eslint-plugin-barrel-files/blob/main/lib/rules/avoid-importing-barrel-files.js#L123 Could you try to create a minimal reproduction? I wonder why its reaching the resolve part for that import

[starkfire]

At the moment, this is the only best reproduction I could make: https://github.com/starkfire/barrel-vite

Tried to reproduce the path alias issue in the Vite example, but to no avail. I still have to come back with it and come up with a good reproduction of the issue without taking the entire project as an example. Might also examine if configs have something to do with it.

This will slightly deviate from my original subject (i.e. path aliases) but in the Vite example, I realized that a minimum Vite + React-Typescript template alone is enough to cause the same panic I had mentioned. In this case, the fastest hack is to ignore vite.config.ts:

thread '<unnamed>' panicked at src\lib.rs:147:10:
called `Result::unwrap()` on an `Err` value: NotFound("lightningcss")
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

With regards to the oxc-resolver PR, I'll be working on it and see what I can do.

[thepassle]

Looks like this is the line that it crashes on in your reproduction:

It tries to resolve lightningcss, which is in the module graph of vite (by analyzing vite.config.ts), but lightningcss is not installed and not present in node_modules, so its not able to resolve it:

Im not sure why, but I feel like this is a bug on the side of vite. Ill create an issue there

[starkfire]

Based on the Vite docs (https://vitejs.dev/guide/features.html#lightning-css), LightningCSS is an optional dependency, so it could be the reason why they would dynamically import it. Maybe there's a need for a strategy to detect dynamic imports? Or am I missing something? I'll be looking on to it.

I made another branch on the example repo: https://github.com/starkfire/barrel-vite/tree/path-alias-repro. On this one, I finally managed to reproduce the path alias problem. The resolution fails quietly when a component tries to import another component via path aliases:

Error: Failed to resolve importer: "D:\Users\kmccallister\Desktop\GitHub\sandbox\barrel-vite\src\components\example\Example.tsx", importee: "@/shared/ui/AnotherExample"

I think the plugin ignores type imports though, since in the previous example I provided, I forgot to mention that the importer and importee were just exporting React components:

Error: Failed to resolve importer: "D:\Users\kmccallister\Desktop\GitHub\sandbox\ts-testbed\apps\vessel\src\features\formtemplates-display\ui\atoms\FormTemplateCard.tsx", importee: "@/shared/ui/Card"

Also did this on the example I provided, and the module never encountered issues on type imports. Although if avoid-importing-barrel-files does ignore types, I just wonder why did it panic over @/shared/types on a larger project we're working on (even if it's just a barrel file made up of type exports):

thread '<unnamed>' panicked at src\lib.rs:147:10:
called `Result::unwrap()` on an `Err` value: NotFound("@/shared/types")
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
 ELIFECYCLE  Command failed with exit code 3221226505.

It would also take several "Failed to resolve importer" errors before the module also starts to panic this exact same way. I still have to reproduce this one without resorting to a larger example, but either way, I just went for the quick fix for the time being as I try to dig deeper on to this.

I never spent that much time on Rust as well (skill issues lmfao), so I think this seems like a great time to try working on oxc-resolver while the Node binding is still apparently a WIP. Thanks though!

[thepassle]

Hm, yeah thats a sticky situation. I think the solution for the aliases is to move to oxc-resolver and make the resolution options configurable in the eslint plugin config, so that you can pass custom aliases to the plugin. We shouldn't need to write any Rust to make those changes :) If you're happy to make a PR for that, I'd be happy to review it, if not, i'll try to take a look at this sometime next week.

And additionally, in general (also for any people who may be reading this in the future), I recommend switching to package imports instead of aliases.

I also think it's probably fine to just exclude the vite.config.ts for this plugin, I think you can configure that in your eslint config as well