import-js / eslint-plugin-import

ESLint plugin with rules that help validate proper imports.
MIT License
5.58k stars 1.57k forks source link

[import/extensions] Suffixes in file names #2147

Closed anton-johansson closed 2 years ago

anton-johansson commented 3 years ago

Hey! I've embraced a file name convention where the files are suffixed depending on the kind of export. For example:

I'm having a hard time tweaking import/extensions to support this. I want to exclude .ts from my imports, so naturally that means "ts": "never". However, I do get errors like:

Unexpected use of file extension "class" for "~/shared/my-package/my-service.class" (import/extensions).

Adding "class": "always" doesn't help either.

Any ideas? I assume the ideal scenario would be to treat the "extension" as only the last part of the file name, but I also assume that it's hard for the plugin to see that (I guess it only looks on the actual import statement?).

ljharb commented 3 years ago

Since there’s only one extension, and your odd convention isn’t an extension, I’d expect it to have no impact on the rule.

In other words, this is a bug with the rule that should be fixed (altho I’d suggest dropping this convention).

A PR or link to a branch with failing test cases would be most helpful.

olsonpm commented 2 years ago

I thought I ran into this however found my path to the dependency was incorrect. Here was the resulting behavior

file structure

/run.js
/some.dep.js
// run.js
require('./some.dep')
require('./some.dep2')
$ npx eslint run.js

/path/to/run.js
  2:9  error  Unexpected use of file extension "dep2" for "./some.dep2"  import/extensions

✖ 1 problem (1 error, 0 warnings)

this error is further clarified with "import/no-unresolved", so I don't know it's a bug in the plugin.

ljharb commented 2 years ago

Yeah, after reviewing this again, I think this isn't a bug, this is just the consequence of using dots in a file while also omitting extensions in the specifier.

This isn't a good convention - filenames should only have zero or one dots, and the extension is whatever's after the dot.

marvinruder commented 2 months ago

This isn't a good convention - filenames should only have zero or one dots, and the extension is whatever's after the dot.

I am having the same issue with lots of messages like

  13:25  warning  Unexpected use of file extension "service" for "../src/user/user.service"            import/extensions

in a NestJS project after upgrading to ESLint 9. NestJS, which is a rather popular framework (3.5M weekly downloads, 67k GitHub stars) uses file name structures like app.controller.ts, app.controller.spec.ts, app.module.ts, app.service.ts etc. in a default project setup.

@ljharb Would you consider reopening this issue, given that the default file naming convention in NestJS requires multiple dots in a filename?

ljharb commented 2 months ago

@marvinruder That's very unfortunate. Can NestJS maintainers weigh in on why this is a reasonable choice for them to make?

marvinruder commented 2 months ago

@marvinruder That's very unfortunate. Can NestJS maintainers weigh in on why this is a reasonable choice for them to make?

@ljharb Neither am I a NestJS developer nor do I know one, but in this comment it is hinted that NestJS’s naming convention is derived from Angular’s, which is documented in detail here. There is not much reasoning to find on the choice of the dot as a separator though.

michaelfaith commented 2 months ago

Having worked in Angular for a long time in my last position, I'm intimately familiar with that pattern. It was in part born out of the fact that up until recently, Angular components consisted of several collaborating files. So the guidance the Angular team gave was to have a folder named after the component (e.g my-component), and then each part of the component would be named as my-component.<purpose>.<extension>.

and it wasn't just guidance, but their tooling actually enforced this pattern. If you generate a component skeleton with the angular cli (ng generate component MyComponent), that's the structure it builds out for you.

In recent years, they've been shifting more towards single file components, but that's after almost a decade of having it this other way. I will say, it's not just the Angular ecosystem that use this convention. My current team sometimes uses an intermediate suffix to denote purpose. E.g. Config.types.ts. In a really large monorepo codebase, it can be helpful for quickly identifying what file you're looking for in the IDE.

olsonpm commented 2 months ago

@michaelfaith the question isn't why angular structures their code the way they do - but why they use a period as a separator for that information. Personally I don't understand why this is such a sticking point - I see multiple dots used in filenames commonly. But it's whatever - if people feel strongly about this it's easy enough to fork

michaelfaith commented 2 months ago

I'm not really trying to explain or justify the decision to use a dot as that separator, but just that they've had this convention (backed by their tooling) for many years. And so, this would negatively impact the entire Angular development community pretty directly.

ljharb commented 1 month ago

I'm not really sure how I'd modify the rule to handle this anyways.

Both files could exist: foo.bar.js and foo.bar, and the .bar in the second example thus has to be considered as an extension.

michaelfaith commented 1 month ago

That brings up an interesting point. Looking at the condition where this error would fire:

https://github.com/import-js/eslint-plugin-import/blob/67cc79841fc823ad4af2532af2dc6704e4b3b03a/src/rules/extensions.js#L187-L192

It seems that it would only throw the error if either what it detected as an extension (class in the OPs example) is expressly forbidden or if all extensions are forbidden (the default config) and the path can be resolved without that portion of the path (e.g. without .class). In the case of the Angular component naming convention that last condition wouldn't generally hold true. You wouldn't have a my-component.component.ts in the same folder as a my-component.ts or a nested folder named my-component with a barrel file. So it wouldn't be resolvable without that second meta segment.

@anton-johansson in your example, do you have both ~/shared/my-package/my-service.class.ts and ~/shared/my-package/my-service.ts (or some other file type) next to each other?

michaelfaith commented 1 month ago

Here's another counter example, though, that I believe would fail that second condition, from the MUI (React) library: https://github.com/mui/material-ui/tree/master/packages/mui-base/src/Button

Image

It's definitely not uncommon, from my experience, to use that kind of intermediate suffix for meta info.

ljharb commented 1 month ago

Even if it were exceedingly uncommon to have that scenario, it's possible, so the rule would need to account for it.

In other words, while "Nuxt and Angular use this convention" may be an argument for supporting it, even if it's an unwise convention, but without a feasible plan for supporting it, we wouldn't be able to do anything anyways.