-
-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add options to enable aliases for siblings and subpaths #175
base: master
Are you sure you want to change the base?
Conversation
@dword-design Hey Sebastian. Could we have this merged? <3 |
@simplenotezy Sry for the delay. It generally lgtm. Can we do the following changes:
Then I'm up to merge 🙂. |
ba58977
to
49584c9
Compare
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
That's why I improved those two options to also accept objects instead of booleans. So now, 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) 😉 |
@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 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 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:
In the actual implementation:
Does it make more sense ? *: actually I first created a |
@chapa If I got it right, these So if there is an import from 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 |
Yes you're right with the last sentence, I always want to import top-level files from 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 As I re-read you're answer, I'm thinking maybe you misunderstood the example with Is that better ? :) |
@chapa Ok yeah, so summing that up again:
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 (
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 |
fyi i shipped the option to always use aliases, also for subpaths. |
Hi, sorry for the delay. Yeah you summed it up well 👍 Actually I didn't know that the For example we could have a
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! |
Hello,
This is a continuation of #130 (which has been closed by its author) and closes #133.
This adds 2 options
forSubpaths
andforSiblings
, 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)
andif (!(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.