tleunen / babel-plugin-module-resolver

Custom module resolver plugin for Babel
MIT License
3.46k stars 205 forks source link

Using a windows path in the alias replace string breaks #242

Open vjpr opened 6 years ago

vjpr commented 6 years ago

When wallaby.projectCacheDir is C:\x\y\.z\system\wallaby\projects\6efd40f7185030b1...

require('packages/foo') tries to look for C:\x\y\.z\system\wallaby\projectsefd40f7185030b1\instrumented/packages/foo.

The windows path backslashes are escaped, and then it matches \\6 after projects.

module.exports = function moduleResolverPlugin(wallaby) {
  return [
    'module-resolver',
    {
      alias: {
        '^@test/(.+)': wallaby.projectCacheDir + path.sep + 'packages/\\1',
        '^packages/(.+)': wallaby.projectCacheDir + path.sep + '',
      },
    },
  ]
}

You can reference the n-th matched group with '\\n' ('\\0' refers to the whole matched path).

To use the backslash character (\) just escape it like so: '\\\\' (double escape is needed because of JSON already using \ for escaping).

Using \\n for match groups has the potential for really weird issues. Can we change it to something else?

fatfisz commented 6 years ago

Hmm we definitely couldn't change that in a short time - this is a breaking change. It's also impossible to infer what is the meaning of \6 from your example without some funny heuristics in the plugin.

I'm not convinced that complicating the RegExp alias config is a good trade-off for handling backslashes from Windows paths. As someone who works on Windows myself, I care about properly handling paths in different systems a lot, but in this case we're in the realm of JS regular expressions. Any solution in the plugin other than the current would be hackish with respect to the regular expressions themselves.

I'd suggest replacing backslashes with slashes here: as a user of the plugin you have much more control over what to do with the data that is passed. I feel your pain, but I'm skeptical we'd find a better solution here. Unless you could present some ideas?

vjpr commented 6 years ago

Hmm we definitely couldn't change that in a short time - this is a breaking change

What about allowing the following (would not be breaking):


module.exports = function moduleResolverPlugin(wallaby) {
  return [
    'module-resolver',
    {
      alias: {
        '^@test/(.+)': {replace: 'foo\\654\\\1'},
        '^packages/(.+)': {replace: 'foo\\654\\\1'},
      },
    },
  ]
}
``` @
tleunen commented 6 years ago

Hmm.. Yeah.. this sucks for windows :/ And it would definitely be great to support Wallaby. Maybe we could use a custom syntax instead of the usual \\n to prevent any issue using a path delimiter.

fatfisz commented 6 years ago

Here's an idea: since I see that you're using JS to configure Babel, would allowing a function as a value of the alias be acceptable for you? E.g.

module.exports = function moduleResolverPlugin(wallaby) {
  return [
    'module-resolver',
    {
      alias: {
        '^@test/(.+)': ([, name]) => path.join(wallaby.projectCacheDir, 'packages', name),
      },
    },
  ]
}

I think this looks clean enough and should solve your problem.

fatfisz commented 6 years ago

The argument would be the result of calling exec on a matched path (so it would never be null). This means that the argument is always an array with the full match and the matched groups, which can be used as desired.

vjpr commented 6 years ago

@fatfisz Yep, this is fine.

tleunen commented 6 years ago

@fatfisz Another solution would also be to replace the \\n from the regexp into $n. That's what jest uses with their moduleNameMapper option for example.

fatfisz commented 6 years ago

Hmm, this would require a special treatment of the native \n groups... not convinced it's the best idea (though I'm not ruling it out, either).