Simplify path replacing

[fatfisz]

This branch consists of a few changes, so maybe it's better to split it into a few smaller ones.

The changes are:

• indentation fixes - although the linter doesn't complain, some places look like an artifact of some other indentation (most notably misaligned inline comment - was the indentation 4 spaces wide previously?), so I went ahead and reindented the code manually. I hope this is not going overboard 😛 This is the 31e7a6d commit.
• the transformers were doing unnecessary work replacing the whole call expressions, where only the arguments needed to be replaced. By doing that the need to keep info about the node being visited was removed. This is the 00c2ed7 commit.
• the rest of the commits deal with the problem described here: Alias resolves to wrong alias when used inside aliased code #96 (comment). Basically the module resolver plugin was running twice for the imports when paired with the module-to-commonjs transformer. The solution seems to be visiting the nodes on program exit. The order of the plugins doesn't matter in this case: module-to-commonjs also runs on program exit.

The last part changes the behavior of the plugin, so at least a minor version bump would be needed.

[codecov]

Codecov Report

Merging #136 into master will not change coverage.
The diff coverage is 100%.

Impacted Files	Coverage Δ
src/getRealPath.js	100% <ø> (ø)	⬆️
src/utils.js	100% <ø> (ø)	⬆️
src/index.js	100% <100%> (ø)	⬆️
src/transformers/systemImport.js	100% <100%> (ø)	⬆️
src/transformers/require.js	100% <100%> (ø)	⬆️
src/transformers/jest.js	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c4dd26f...e7c54a2. Read the comment docs.

[tleunen]

Thanks @fatfisz!

[tleunen]

How good/bad this is?

Is it the same in term of performance as what was done before? I remember reading that if we can do something without traversing, it's usually best

[fatfisz]

I thought of other solutions, too, but each one of them fell short:

• caching the paths won't help with all the edge cases: let's say that path A -> path B, so path B is marked as untouchable - what about path B appearing in some other place? This could result in the loss of determinism (result would depend on the order of the paresd files and would be bad anyway)
• "marking" the paths in any way is not viable - I tried wrapping the path in the String object, which didn't play with the other plugin; also the node may get replaced by other plugins which will result in loss of the metadata (babel-plugin-transform-es2015-modules-commonjs is taking value and putting it into a new StringLiteral node)

FWIW babel-plugin-transform-es2015-modules-commonjs is also doing something similar to traverse on Program exit (it doesn't have to do the full traverse, because import is only allowed at the top level).

[tleunen]

Same here, we can use moduleArg instead of nodePath.get('arguments.0')

[tleunen]

Great! We can even do better by replacing nodePath.get('arguments.0') with moduleArg :)

[fatfisz]

Oh, I didn't know that elements of the arguments path were also paths. Will fix that.

[tleunen]

Same for nodePath.get('arguments.0')

[fatfisz]

@tleunen Ping ;)

Btw. is there a chance you'll be releasing a new version with the fixes anytime soon?

[tleunen]

I'm good with these changes, no worries :)
I'll try to make a new release later today after testing all these new things in an existing project. No guarantees. I just moved in a new city and I still have a lot of things to do ;)