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

-r/--require flag for npm modules #1803

Closed
wavded opened this issue May 26, 2015 · 9 comments
Closed

-r/--require flag for npm modules #1803

wavded opened this issue May 26, 2015 · 9 comments
Labels
confirmed-bug Issues with confirmed bugs. module Issues and PRs related to the module subsystem.

Comments

@wavded
Copy link
Contributor

wavded commented May 26, 2015

Given the following steps:

npm install heapdump
node -r heapdump file.js

I get the following error:

Error: Cannot find module 'heapdump'
    at Function.Module._resolveFilename (module.js:337:15)
    at Function.Module._load (module.js:287:25)
    at node.js:858:16
    at Array.forEach (native)
    at Function.startup.preloadModules (node.js:857:32)
    at startup (node.js:95:17)
    at node.js:963:3

My expectation was the module would be resolved the same way as if I were to do:

require('heapdump')
@ofrobots
Copy link
Contributor

I'm investigating.

@ofrobots
Copy link
Contributor

The problem is related to the fact that preload modules do not have a parent. Not even the main module. As such they don't use the .paths property from the parent which gives us a starting point for searching node_module directories.

@ofrobots
Copy link
Contributor

Here's a potential fix I am evaluating:

Module.preloadModules = function(requests) {
  if (requests) {
    // Preloaded modules have a dummy parent module which is deemed to exist
    // in the current working directory. This seeds the search path for
    // preloaded modules.
    var parent = new Module('internal/preload', null);
    parent.paths = Module._nodeModulePaths(process.cwd());
    requests.forEach(function(request) {
      Module._load(request, parent, false);
    });
  }
}

Note the slight semantic difference between -r foo and require(foo). The former search from the working directory of the process upwards whereas the latter starts from the directory of the requiring module.

@mscdex mscdex added the module Issues and PRs related to the module subsystem. label May 27, 2015
@i5ting
Copy link

i5ting commented May 27, 2015

the same issue

@i5ting
Copy link

i5ting commented May 27, 2015

i want in cli i can use require 'xxx'

@Fishrock123
Copy link
Contributor

Note the slight semantic difference between -r foo and require(foo). The former search from the working directory of the process upwards whereas the latter starts from the directory of the requiring module.

Right, but that seems to make sense, the requiring "module" in this case is effectively the working directory.

@ofrobots
Copy link
Contributor

@Fishrock123 That's my intuition too. I wanted to point it out to make sure it is not surprising to others.

ofrobots added a commit to ofrobots/node that referenced this issue May 27, 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#1803
ofrobots added a commit to ofrobots/node that referenced this issue May 27, 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#1803
@Fishrock123 Fishrock123 added the confirmed-bug Issues with confirmed bugs. label May 30, 2015
Fishrock123 pushed a commit that referenced this issue 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 <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
@Fishrock123
Copy link
Contributor

Fixed in 5759722!

andrewdeandrade pushed a commit to andrewdeandrade/node that referenced this issue 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 <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
@bmeck
Copy link
Member

bmeck commented Aug 12, 2015

Using process.cwd() makes using preloaded modules with absolute paths blow up when you are in a ENOENT directory. investigating fix

Fishrock123 pushed a commit that referenced this issue Aug 24, 2015
Fixes a regression from 5759722
that prevented modules from being preloaded if the cwd does not exist.
Absolute and builtin modules now preload correctly again.

Refs: #1803
PR-URL: #2353
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Fishrock123 pushed a commit that referenced this issue Aug 24, 2015
Fixes a regression from 5759722
that prevented modules from being preloaded if the cwd does not exist.
Absolute and builtin modules now preload correctly again.

Refs: #1803
PR-URL: #2353
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. module Issues and PRs related to the module subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants