fix: Relative file path imports now supports extensions

[Francesco-Lanciana]

Previously if a relative import was used then the plugin would make no attempt to resolve the file using the extensions provided. This was an issue if you were using an extension that NodeJs didn't automatically check for you, for example the jsx extension. Now you can import using a relative file and we will try and resolve it against all provided extensions.

This step is done after attempting to resolve the import using the provided aliases but before using the root config. I thought that relative paths take precedence over those that are resolved relative to the root of the project.

Let me know if I was a bit off in my implementation or if the tests are lacking. Happy to discuss and fix that up ASAP.

[Francesco-Lanciana]

I somehow managed to completely glance over pull request #259 which addresses the same issue, sorry about that.
However I did notice that the implementation is slightly different in that I have resolving against aliases taking precedence over relative imports. I also didn’t remove the initial isrelative check in the resolvePath method, I only modified it to also check that the import already includes a file extension.
I believe those two things should minimise any possible subtle breaking changes. Having said that I’m happy if you want to remove this request if you feel that it’s functionally equivalent to the first.

On that note though, why did the first pull request stall? Also as a general question, wouldn’t it make more sense not to strip out the extensions of imports by default. By stripping them out we are essentially having NodeJs do the work of resolving the extension all over again. It also introduces behaviour that deviates from what users expect when they have custom extensions. So far I can’t see any upside of even having a stripextensions option.

[tleunen]

@fatfisz - Would you like to jump on this? :)

@Francesco-Lanciana First of all, thank you for the PR to try to add this functionality. To answer your question about why the first PR never got merged, it's because we were not sure about the implementation, and the possible issues that would emerge from it. This is not an easy feature, and this could lead to many complications if we miss something.

To be clear, I see the benefit of doing it, but since the plugin will run against ALL paths with this kind of change instead of only the "absolute" paths, we can definitely break a lot of projects if we don't pay enough attention.

Also as a general question, wouldn’t it make more sense not to strip out the extensions of imports by default.

This was done to keep the output as close as what someone would write their code as well. Nobody writes imports with a .js extension :) And I'm pretty sure that node would still test every possibility to find the file, since you could have multiple dots in the filename.

So far I can’t see any upside of even having a stripextensions option.

Hmm... Again, as I said, this was done to keep it as close as what you would do originally without using the plugin. But also, from what I recall, to let the React Native bundler use the right Android or IOS file when you decide to to import a file that has either a .ios.js or .android.js extension.
But to be honest, I agree that this specific option is very specific to the nodejs extension and the react native ones. Otherwise, I'm pretty sure it would fail. Unless you're in a webpack environment and also add the custom extensions I guess.

[Francesco-Lanciana]

No worries, was a fun little challenge :) I can see why you would be nervous about making the change. What checks would you recommend if we were to try and get this feature merged?

In regards to the stripExtensions option, from what I've read about the node module resolution algorithm it seems that after checking if it is a core module (e.g. fs, http, ect) it checks if it is a relative path. If so it checks if there is a file at that exact path. If there is it stops. If there isn't it will append extensions checking each time to see if it is now a file. It doesn't keep attempting to resolve the file after it has a match. In this case resolving the file to then strip off the extension seems like repeated work.

I also don't fully understand why you want to keep the outputted code as close to what you would get if you didn't use the plugin. If a developer is using the plugin they are opting into the behaviour of the plugin, it just feels like this choice is limiting what the plugin can do. I've been trying to move away from using webpack for everything (in this case the resolve config option) in order to not have to repeat config when I don't use webpack (as is the case when writing tests). However I have been finding small deviations between webpacks algorithm and this plugin, with webpack often seeming to make more sense. For example when using an alias I noticed that this plugin won't resolve the extension for me, for example:
If I have the alias: "Components": path_to_components_folder
and the extensions: [".js", ".jsx"]
and I use the import: import A from "Components/A"
where A is a jsx file it won't work. It will substitute the alias but it won't add the extension, therefore node will be unable to resolve the file. This is counterintuitive since I provided the extensions as an option.

Would you be open to a pull request to add this behaviour in? Sorry for poking and prodding, I'm sure all these choices were made for a reason however I've just been finding that I keep hitting walls (small ones) after switching from webpacks resolve algorithm.