jmm / pathmodify

In a nutshell, rewrite (AKA alias, map) `require()` IDs / paths / directories to different values in browserify.
MIT License
27 stars 1 forks source link

Does not work on Windows #1

Closed davidtheclark closed 9 years ago

davidtheclark commented 9 years ago

Running the same build on a Windows machine that I've been running on Linux and Mac does not work. The path modifications are not being picked up, so the modules are not being found.

Unfortunately I do not have the time to help diagnose and fix this problem right now --- I have to switch to something else to get some things done immediately -- but I thought it would be worth logging the issue to bring it to your attention.

jmm commented 9 years ago

@davidtheclark

but I thought it would be worth logging the issue to bring it to your attention.

Absolutely, thank you! If you're able to provide a minimal repo that will allow me to reproduce the problem, that'd be great. Or since you're pressed for time, even a snippet of code would be helpful for the time being.

jmm commented 9 years ago

@davidtheclark I think I may have identified the issue you ran into. If you can post some reproduce code that'll help me confirm.

In your project do all of the require() strings use forward slash /?

davidtheclark commented 9 years ago

Yes, I am using forward slashes in my paths. I'll try to make something real quick to reproduce.

davidtheclark commented 9 years ago

Does this make sense? https://gist.github.com/davidtheclark/0e7a289f30fb4c9c71b7 It's causing the same error that I had seen in my more complicated build.

jmm commented 9 years ago

@davidtheclark

Does this make sense?

Absolutely. As I suspected, it was an issue with the path seprator. Before, this tested the beginning of the require() string for 'foo' + path.sep (the platform specific path seprator). So it was testing for foo\bar instead of foo/bar. I changed it so that it checks the require() string for presence of /|\ and uses the separator it detects to perform the test. If for some reason a string contained both it would give precedence to /. If this proves not to be robust enough I'll come up with something else.

The updated version has been published as v0.4.0. If you have the chance to try it and post an update here that would be great.

BTW, I noticed that you're using a relative path in your replacement:

pathmodify.mod.dir('foo', './test/foo')

You certainly can do that, and it wasn't the source of the problem you had, but you may find it preferable to use an absolute path as the replacement (although that may take a bit of extra work to normalize the path separators for cross-platform use, e.g. [__dirname.replace(/\\/g, '/'), 'test', 'foo'].join('/'). At any rate, if you use a relative replacement just bear in mind that it'll be relative to the parent module (the one the require() call is in), not cwd or basedir. Not saying you don't know that, just want to make sure it's obvious.

Sorry about the problem and thanks very much for reporting it and providing the repro code.

davidtheclark commented 9 years ago

At any rate, if you use a relative replacement just bear in mind that it'll be relative to the parent module (the one the require() call is in), not cwd or basedir. Not saying you don't know that, just want to make sure it's obvious.

Good to know, thanks. I think that in my real code I was already using absolute paths --- I will check and make sure.

I'll give this fix a try when I get back on that stupid Windows machine later today :) Thanks for the quick response! I was just about to start looking for quick alternatives; now I probably don't have to.

davidtheclark commented 9 years ago

Fixed. Thanks again!

jmm commented 9 years ago

@davidtheclark Great, glad to hear that did the trick. Thanks for your interest in the project and your help with this!