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

src: fix module search path for preload modules #1812

Closed
wants to merge 1 commit into from

Conversation

ofrobots
Copy link
Contributor

When the preload module is not a abs/relative path, we should use
the standard search mechanism of looking into the node_modules folders
outwards. The current working directory is deemed to be the 'requiring
module', i.e. parent. The search path starts from cwd outwards.

Fixes: #1803

@mscdex mscdex added the module Issues and PRs related to the module subsystem. label May 27, 2015
assert.ok(module.parent);
var expectedPaths = require('module')._nodeModulePaths(process.cwd());
assert.ok(module.parent.paths.length === expectedPaths.length &&
module.parent.paths.every(function(e,i) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Space after the ,. I think module should line up with module on the previous line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed by replacing with assert.deepEquals.

@Fishrock123
Copy link
Contributor

When the preload module is not a abs/relative path, we should use
the standard search mechanism of looking into the node_modules folders
outwards. The current working directory is deemed to be the 'requiring
module', i.e. parent. The search path starts from cwd outwards.

Fixes: nodejs#1803
@cjihrig
Copy link
Contributor

cjihrig commented May 27, 2015

FWIW, I pulled this down locally and was able to -r npm modules, so 👍

@wavded
Copy link
Contributor

wavded commented May 28, 2015

Thx for tackling this @ofrobots, LGTM.

Fishrock123 pushed a commit that referenced this pull request May 30, 2015
When the preload module is not a abs/relative path, we should use
the standard search mechanism of looking into the node_modules folders
outwards. The current working directory is deemed to be the 'requiring
module', i.e. parent. The search path starts from cwd outwards.

Fixes: #1803
PR-URL: #1812
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@Fishrock123
Copy link
Contributor

Thanks, landed in 5759722 :)

@rvagg
Copy link
Member

rvagg commented May 31, 2015

marking this as semver-minor, let me know if you disagree

@rvagg rvagg added the semver-minor PRs that contain new features and should be released in the next minor version. label May 31, 2015
rvagg added a commit to rvagg/io.js that referenced this pull request May 31, 2015
Notable Changes:

* node: Speed-up require() by replacing usage of fs.statSync() and
  fs.readFileSync() with internal variants that are faster for this use-case
  and do not create as many objects for the garbage collector to clean up.
  The primary two benefits are: significant increase in application start-up
  time on typical applications and better start-up time for the debugger by
  eliminating almost all of the thousands of exception events.
  (Ben Noordhuis) nodejs#1801.
* node: Resolution of pre-load modules (-r or --require) now follows the
  standard require() rules rather than just resolving paths, so you can now
  pre-load modules in node_modules. (Ali Ijaz Sheikh) nodejs#1812.
* npm: Upgraded npm to v2.11.0. New hooks for preversion, version, and
  postversion lifecycle events, some SPDX-related license changes and license
  file inclusions. See the release notes for full details.
rvagg added a commit to rvagg/io.js that referenced this pull request May 31, 2015
PR-URL: nodejs#1808

Notable Changes:

* node: Speed-up require() by replacing usage of fs.statSync() and
  fs.readFileSync() with internal variants that are faster for this use-case
  and do not create as many objects for the garbage collector to clean up.
  The primary two benefits are: significant increase in application start-up
  time on typical applications and better start-up time for the debugger by
  eliminating almost all of the thousands of exception events.
  (Ben Noordhuis) nodejs#1801.
* node: Resolution of pre-load modules (-r or --require) now follows the
  standard require() rules rather than just resolving paths, so you can now
  pre-load modules in node_modules. (Ali Ijaz Sheikh) nodejs#1812.
* npm: Upgraded npm to v2.11.0. New hooks for preversion, version, and
  postversion lifecycle events, some SPDX-related license changes and license
  file inclusions. See the release notes for full details.
andrewdeandrade pushed a commit to andrewdeandrade/node that referenced this pull request Jun 3, 2015
When the preload module is not a abs/relative path, we should use
the standard search mechanism of looking into the node_modules folders
outwards. The current working directory is deemed to be the 'requiring
module', i.e. parent. The search path starts from cwd outwards.

Fixes: nodejs/node#1803
PR-URL: nodejs/node#1812
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@ofrobots ofrobots deleted the fix-1803 branch August 24, 2015 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module Issues and PRs related to the module subsystem. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

-r/--require flag for npm modules
6 participants