Skip to content

Support loading from NODE_PATH, fixes tkellen/node-liftoff#28 #44

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

Merged
merged 2 commits into from
Feb 24, 2015

Conversation

Meroje
Copy link
Contributor

@Meroje Meroje commented Feb 23, 2015

This will pass NODE_PATH to resolve.sync when looking for the module to require, or empty array if undefined (default for most people) which behaves the same.

@heikki
Copy link
Contributor

heikki commented Feb 23, 2015

Fails on windows because of this: .split(':')

Example path: C:\Users\IEUser\Desktop\GitHub\node-liftoff\explicit\cwd

@tkellen
Copy link

tkellen commented Feb 24, 2015

I'm conflicted about including this. It really belongs in resolve, but @substack refused a PR adding support for this here: browserify/resolve#47

Any thoughts @phated?

@phated
Copy link
Member

phated commented Feb 24, 2015

If we have to use substack modules, then we should handle the case in our lib. Just make sure it works cross platform

@heikki
Copy link
Contributor

heikki commented Feb 24, 2015

http://nodejs.org/api/modules.html#modules_loading_from_the_global_folders

Note: On Windows, NODE_PATH is delimited by semicolons instead of colons

@tkellen
Copy link

tkellen commented Feb 24, 2015

Thanks @Meroje!

tkellen pushed a commit that referenced this pull request Feb 24, 2015
Support loading from NODE_PATH, fixes #28
@tkellen tkellen merged commit 7eed039 into gulpjs:master Feb 24, 2015
@tkellen
Copy link

tkellen commented Feb 24, 2015

released as 2.0.2

@Meroje
Copy link
Contributor Author

Meroje commented Feb 24, 2015

Thanks for your quick answers on this

tkellen pushed a commit that referenced this pull request Jul 25, 2016
Support loading from NODE_PATH, fixes #28
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.

4 participants