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

Find relative path for process module #31

Closed
wants to merge 2 commits into from

Conversation

jkimbo
Copy link

@jkimbo jkimbo commented Apr 23, 2014

Include process\browser module as a relative path instead of an absolute one to avoid having full system paths in bundles.

@jkimbo
Copy link
Author

jkimbo commented Apr 23, 2014

Potential fix for browserify/browserify#675 and browserify/browserify#657

@jkimbo
Copy link
Author

jkimbo commented Apr 23, 2014

And browserify/browserify#706

@ghost
Copy link

ghost commented Apr 23, 2014

Thanks for the patch. I'm hesitant to merge something with so much custom path string arithmetic. Could this be rewritten to use path.resolve() or path.relative()? It seems like it would be a single-line change.

@jkimbo
Copy link
Author

jkimbo commented Apr 24, 2014

@substack the reason for all the path string manipulation is that basedir is not always the actual base directory of the project. Sometimes it is the full file path of the parent file. Not quite sure why this is but thats why I needed to find the common directory which I can then use as the basedir. Any idea on why that is?

@jkimbo
Copy link
Author

jkimbo commented Apr 30, 2014

@substack any thoughts?

@calvinmetcalf
Copy link

@jkimbo you might want to use path.sep instead of hardcoding seperators

@jkimbo
Copy link
Author

jkimbo commented May 7, 2014

I think #33 is a better solution so I'm closing this.

@jkimbo jkimbo closed this May 7, 2014
@ghost
Copy link

ghost commented May 10, 2014

fixed upstream in 3.46.1 by passing in the hash-mapped path into insert-module-globals manually

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.

2 participants