eslint-plugin-import-alias
eslint-plugin-import-alias copied to clipboard
Add options to enable aliases for siblings and subpaths
Hello,
This is a continuation of #130 (which has been closed by its author) and closes #133.
This adds 2 options forSubpaths and forSiblings, to enable aliases for these specific cases. I separated them because I wanted only siblings (import from './foo') and not subpaths (import from ./sub/foo) to be converted to aliases.
I took the liberty to refactor a bit because the previous main conditional branches (if (sourcePath |> isParentImport) and if (!(importWithoutAlias |> isParentImport) && hasAlias)) were making things too convoluted. Feel free to improve/refactor as you wish.
And lastly, I had to create a importType variable to be able to make the tests pass (having Unexpected parent import in the eslint message). I would suggest to remove this word for the sake of simplicity, but that would imply updating the tests and I didn't want to take this decision myself.
@dword-design Hey Sebastian. Could we have this merged? <3
@simplenotezy Sry for the delay. It generally lgtm. Can we do the following changes:
- Use 2 separate calls instead of the array destructuring? Or does the destructuring have a specific purpose?
- Refactor the import type into a function so that we don't have the
letvar - Add a doc to the readme
Then I'm up to merge 🙂.
Hi, no problem for the delay.
I did the requested changes but then I tested on a real project and realized a bug and a regression introduced by my modifications, those are fixed now.
After that, I realized that the two new options forSiblings and forSubpaths were not sufficient for what I wanted to do. Indeed, they didn't make some distinctions that I wanted to have, which are :
- forcing alias for siblings only if the current file (in which the import is written) is located at the root of the alias,
- forcing alias for subpaths only if the current file is located outside of the alias.
That's why I improved those two options to also accept objects instead of booleans. So now, forSiblings accepts an object in the shape of { ofMaxNestingLevel: number } to limit the maximum nesting level for which the alias will be enforced, and forSubpaths accepts an object in the shape of { fromInside: boolean, fromOuside: boolean } to be able to enforce aliases only for subpaths from inside or from outside the alias.
I hope those changes are not too much, I think it's ok since the boolean way sill work and the fine-grained configuration is opt-in.
Didn't have the time to update the readme yet but I'll do it (with good examples so everything is well understandable) :wink:
@dword-design docs are here, feel free to merge if you think it's ok.
Just one last little thing: in my project I have some parent imports inside of an alias, which I would prefer to have in the relative path format instead of the aliased format. I'm wondering if we could add a forParents option, similar to forSubpaths which have the fromInside and fromOutside sub-options. This option would defaults to true (= enforcing aliases for all parent imports, as it is now), but setting it to { fromOutside: true } would only enforce aliases when the import is done from the outside (and enforce relative paths when it's done from the inside).
What do you think ? It could be the object of another MR though.
@chapa Ok I have some time now to work on this. My basic question generally is what the use cases are of having these fine-granular options.
The idea of the plugin is to have aliases instead of having these ../../src/utils/foo.js imports. That's also why the sibling and subpath imports are enforced by default because the alias import would be longer. However, I get the use case to have aliases for everything, but this would basically be a single forSubpaths: true, nothing else 😋. Can you maybe elaborate on your use cases in general, why you need the options?
Apart from that, thanks already for the effort you put into the PR 🙂.
Sure, in my project I have some root folders for general (non-specific) stuff and I want their imports to be written with the alias everywhere in the code, but I still want relative path for imports that are "local to that stuff".
Here is a simplified example to show what's problematic with the actual implementation, I have these files:
./
├── components/ <-- Aliased as `_/components`
│ └── MyComponent/
│ ├── components/
│ │ └── SubComponent.tsx
│ ├── MyComponent.tsx
│ └── index.ts
├── hooks/ <-- Aliased as `_/hooks`
│ ├── useHookOne.ts
│ └── useHookTwo.ts
├── pages/
│ ├── PageOne.tsx
│ └── PageTwo.tsx
└── App.tsx
In the actual implementation:
- if I want to import
components/*orhooks/*fromApp.tsxit would enfore relative path (subpath) and I would prefer aliases,- but I still want to enforce relative paths when importing
components/MyComponent/components/SubComponentfromcomponents/MyComponent/MyComponent.tsx(also subpath, but "from inside"), hence thefromInsideandfromOutsidesub-options forforSubpathsto differentiate those two cases,
- but I still want to enforce relative paths when importing
- if I want to import
useHookOnefromuseHookTwoit would enforce relative path (sibling) and I would prefer aliases,- but I still want to enforce relative paths when importing
components/MyComponent/MyComponentfromcomponents/MyComponent/index.ts(also sibling, but with a deeper nesting level), hence theofMaxNestingLevelsub-option forforSiblingsto differentiate those two cases*.
- but I still want to enforce relative paths when importing
Does it make more sense ?
*: actually I first created a forRootSiblings option that answered my use case, but then I though it was too specific and conflicting with forSiblings, so I came up with the idea of a ofMaxNestingLevel sub-option.
@chapa If I got it right, these components and hooks folders are some kind of "contexts" that want to be treated in isolation and if the import goes across the boundaries, it should be an alias import.
So if there is an import from App.tsx into components, it's an alias import because it goes from outside components inside it. But if I import something inside components from a file which is already in there, it's a relative import (if it's a sibling or subpath).
I don't understand why you want an alias when importing between the two hooks but not an alias if you import between components. What's the reasoning behind it?
Or, do you always want to import top-level files from components and hooks as aliases so it's clear from the import that they are "components" and "hooks"?
Yes you're right with the last sentence, I always want to import top-level files from components and hooks as aliases so it's clear from the import that they are "components" and "hooks".
I didn't give the example of importing between two (top-level) components because they all have their proper folder (unlike hooks that are all together in the hooks folder), so imports are "parent" ones so already enforced as aliases.
As I re-read you're answer, I'm thinking maybe you misunderstood the example with components/MyComponent/MyComponent.tsx importing components/MyComponent/components/SubComponent and why I want a relative path here.
I don't know if you're familiar with React, but if not you just have to know that when a component becomes too large you can extract some parts of it into other sub-components (like you would refactor a large function into multiple smaller ones), for clarity and maintainability).
Everything could very well be in the same file (e.g. components/MyComponent/MyComponent.tsx), but sometimes there is just too much to put in a single file so it's better to put components in their own files. For this kind of import (so, non top-level components), I prefer relative paths.
Is that better ? :)
@chapa Ok yeah, so summing that up again:
- There are multiple aliases that are subpaths of the root path of the project. So e.g.
components,hooks,utilsetc., similar to the directory structure of Nuxt.js (I think Next.js is similar, I'm coming from the Vue world). - Every of these aliased paths defines some stuff that is exposed to the outside and can be used elsewhere. This exposed stuff should always be imported via an alias.
- There is also encapsulated stuff inside the folders that is consumed by the exposed stuff, but isn't exposed itself, to distribute logic. This should always be imported using relative import (also if there is a superpath in it).
The issue now is: What is exposed and how is it defined what is exposed? The obvious answer is to take the direct subfiles of the aliased paths (components/MyComponent.tsx), but it's more complicated:
components/MyComponent/index.tsx, so a folder per componentcomponents/b/Button.tsx(having it in some subfolder and this subfolder is then prefixing the component (see e.g. Nuxt components folder)
So, the nesting level inside the aliased path doesn't necessarily say if it should be exposed. If I got it right, you tried to define that logic using the options you proposed (max nesting level, from inside, from outside, etc.).
Currently the proposal still feels a bit not thought till the end and it feels to me like a very clear use case that "should" only have a single boolean option to change the plugin to this use case.
Another idea I had was maybe to have a function/regex/matcher to decide if something should be definitely aliased. This doesn't include then where the import comes from (but maybe also doable with some from: to array), but in your case it looks a bit like you always want to use relative paths except for these specific top-level files.
fyi i shipped the option to always use aliases, also for subpaths.
Hi, sorry for the delay.
Yeah you summed it up well :+1:
Actually I didn't know that the .eslintrc.json (and equivalents) was deprecated in favor of eslint.config.js (and equivalents), so having a function in the plugin options didn't came to my mind, but I think it would be perfect because it's both simple, clear and powerful.
For example we could have a shouldAlias(params): boolean function with the following params:
source: string: path of the importing file,sourceAlias: string | undefined: name of the detected alias for the importing file (if any),sourceAliasPath: string | undefined: path of the detected alias for the importing file (if any),destination: string: path of the imported file,destinationAlias: string: name of the detected alias for the imported file,destinationAliasPath: string: path of the detected alias for the imported file.
That would give all the possible flexibility to the user to decide whether the import should be aliased or not, and should answer every use cases. What do you think ?
@chapa Sounds great! Feel free to provide a PR!