Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix external relative paths on Windows #1704

Closed
wants to merge 8 commits into from
Closed

Fix external relative paths on Windows #1704

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
eceac40
Fix external relative paths on Windows
Shingyx Mar 20, 2017
775e5b4
Replace all \ with / in relative paths for OS-independent behavior
Shingyx Apr 1, 2017
1f3cb40
Add simple .gitignore
Shingyx Apr 1, 2017
d292dce
Formatting
Shingyx Apr 1, 2017
51c7e8e
Make relativePath function a bit clearer
Shingyx Apr 1, 2017
4a558e3
Merge https://github.com/browserify/browserify
Shingyx Oct 10, 2017
64cb665
Merge branch 'master' into master
Shingyx Feb 20, 2018
a689437
Merge branch 'master' into master
Shingyx Mar 3, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
package-lock.json
node_modules/
12 changes: 7 additions & 5 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ inherits(Browserify, EventEmitter);

var fs = require('fs');
var path = require('path');
var relativePath = require('cached-path-relative')
var cachedPathRelative = require('cached-path-relative');

var paths = {
empty: path.join(__dirname, 'lib/_empty.js')
};
Expand Down Expand Up @@ -136,14 +137,12 @@ Browserify.prototype.require = function (file, opts) {
var expose = opts.expose;
if (file === expose && /^[\.]/.test(expose)) {
expose = '/' + relativePath(basedir, expose);
expose = expose.replace(/\\/g, '/');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't you use your helper function from below here instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean. My changes basically extended the existing relativePath method to replace backslashes with forward slashes, which means that adding the .replace(/\\/g, '/') after a relativePath call is no longer necessary.

I don't quite remember, but I think the problem was that .replace(/\\/g, '/') was missing after some of the other relativePath calls and so the path behaviour was inconsistent on Windows.

}
if (expose === undefined && this._options.exposeAll) {
expose = true;
}
if (expose === true) {
expose = '/' + relativePath(basedir, file);
expose = expose.replace(/\\/g, '/');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

}

if (isStream(file)) {
Expand Down Expand Up @@ -789,8 +788,7 @@ Browserify.prototype._debug = function (opts) {
return through.obj(function (row, enc, next) {
if (opts.debug) {
row.sourceRoot = 'file://localhost';
row.sourceFile = relativePath(basedir, row.file)
.replace(/\\/g, '/');
row.sourceFile = relativePath(basedir, row.file);
}
this.push(row);
next();
Expand Down Expand Up @@ -855,3 +853,7 @@ function isExternalModule (file) {
/^[\/.]/;
return !regexp.test(file);
}
function relativePath (from, to) {
// Replace \ with / for OS-independent behavior
return cachedPathRelative(from, to).replace(/\\/g, '/');
}