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.

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

use relative path for process/browser.js #33

Closed
wants to merge 1 commit into from

Conversation

Trott
Copy link

@Trott Trott commented May 4, 2014

Trying (again) to solve the same minor issue as #31 and #32.

  • No more whitespace removal artifacts in this PR.
  • The test here is simplistic. Will try to emulate existing test procedures more if this is too crude or dissimilar to the way the existing tests are done.

add test

tidy test, although I still suspect I'm Doing It Wrong for certain values of "It"

restore white space from master to de-spammify

simplify tests

rm whitespace addition
@ghost
Copy link

ghost commented May 4, 2014

I'm still seeing the global paths in browserify output with this patch, this case from /tmp:

$ echo 'process.nextTick(function () { console.log("beep") })' | browserify | tail -n3

}).call(this,require("../home/substack/projects/insert-module-globals/node_modules/process/browser.js"))
},{"../home/substack/projects/insert-module-globals/node_modules/process/browser.js":1}]},{},[2])

I'm not sure insert-module-globals is the best place to solve this issue actually.

@Trott
Copy link
Author

Trott commented May 4, 2014

Ah, yes, that's a bummer.

If process/browser.js is in your home directory and you are including it from /tmp, what would be the solution? Absolute path will show your home dir, as will relative path.

Only obvious things I can think of would be copying process/browser.js to /tmp/node_modules or something like that, or else symlinking. Both of those ideas seem prone to issues.

Is there another option?

Since just using the relative path solves the issue for some (most, perhaps?) typical use cases, might it make sense just to go with it? Or perhaps go with it until a better, more thorough solution is arrived at?

@ghost
Copy link

ghost commented May 10, 2014

I fixed this by passing in the process implementation through opts.vars from browserify and then hashing the path in browserify using its bundle._hash() machinery. Closing since this is no longer an issue in browserify 3.46.1 and I added a test to cover this case.

@ghost ghost closed this May 10, 2014
@Trott
Copy link
Author

Trott commented May 10, 2014

Sweet. Thanks for this and everything else you do.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant