eslint-plugin-module-resolver icon indicating copy to clipboard operation
eslint-plugin-module-resolver copied to clipboard

Prefer relative import instead of alias if the import is a subpath

Open dword-design opened this issue 3 years ago • 3 comments

Summary

There is an inconsistent case when dealing with relative subpath imports.

When importing ./foo/bar, the import is kept and not replaced, which is good. When importing alias/bar however, the import is not replaced back to ./foo/bar, leading to non-deterministic behavior because the convention depends on the current code state. It should be replaced back to ./foo/bar. Because then always holds:

  • If the imported file is in a subpath of the importing file, use a relative import
  • If the imported file is not in a subpath of the importing file, use the alias import

This has a few implications though. The current implementation is kinda 'specialized' on a single fixer, which is why the fixer is decoupled from the error detection. This will lead to a lot of duplicated code because the error detection has to be replicated in the fixer.

Actually I found this plugin recently and implemented my own one before, so I thought the best idea is to put both together. This PR includes some refactoring and the fixes I already had in my project.

  • Instantiate the fixer right in the traversal function
  • Use the actual babel-plugin-module-resolver logic to resolve the aliases
  • Make it windows-compatible. The current state will generate \ separators in the fixed import paths

Would be great to have this merged! If there are questions or open things to be done, feel free to ask.

dword-design avatar Apr 14 '21 19:04 dword-design

@HeroProtagonist Let me know if there is anything left to be done.

dword-design avatar Apr 14 '21 20:04 dword-design

Hi @dword-design,

Thanks for the information and PR. I like the idea of using the actually library to resolve aliases.

The rule is written to not do anything with imports at the same level (./) The functionality you describe seems like it would be good to add as an option, rather than default behavior.

#161 has a windows fix that will get merged in

HeroProtagonist avatar May 19 '21 18:05 HeroProtagonist

Hey @HeroProtagonist, the PR should also keep imports at the same level at ./. If it's not like this, I can adjust it again. The PR should keep the behavior like it is and only make alias imports relative in case they are on the same level or in a subpath. So it's supposed to be a bugfix originally :P.

Regarding the windows, alright good to see there is another PR for it. I'd say wait for it until we have a state here that we can merge and then we can see how to deal with the windows paths (merge from default branch or do something else).

Thanks for replying, let me know what you think.

dword-design avatar May 22 '21 09:05 dword-design