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

NPM same logic as Node? #396

Closed
4 tasks done
martinheidegger opened this issue Dec 8, 2016 · 9 comments
Closed
4 tasks done

NPM same logic as Node? #396

martinheidegger opened this issue Dec 8, 2016 · 9 comments

Comments

@martinheidegger
Copy link

martinheidegger commented Dec 8, 2016

I just read through the NPM code and stumbled over: find_prefix.js#L46 which basically looks-up the file-system tree to find where the node_modules or package.json folder that should be used as root for the operation.

This looks like this also needs to be done by Node.js in order to identify from where to load the packages.

About that regard I have a few questions / problems:

  1. I wasn't able to find the location of the Node.js logic. Could you point me to it? 😅
  2. What could be a reason for both Node and NPM to implement this logic separately ❓
  3. Which implementation is faster and why? 🚤
  4. Shouldn't there be just one source of truth? 💡
@richardlau
Copy link
Member

Node.js' module loading is described in the Modules API docs and the implementation in lib/module.js. Note that to find modules Node.js starts in the directory of the module that invoked require() -- It does not need to find the 'root' so there is no analogous code in the Node.js source tree (which is why you couldn't find it).

@martinheidegger
Copy link
Author

@richardlau thank you for pointing out lib/module.js. Actually it seems that on _findPath the logic actually loops through the folders by trying to look into the package.json. And iterating differently through the package structure on L358.

So check on 1). with 2) I think the reason is that NPM uses a subset of the Node functionality. Node does check for more than just the node_modules or package.json folder. 3) and 4) are still open though.

@bnoordhuis
Copy link
Member

Which implementation is faster and why?

The one in core, most likely. It takes shortcuts where it can, e.g., internalModuleStat() and internalModuleReadFile().

Shouldn't there be just one source of truth?

That's by definition the one in core.

@martinheidegger
Copy link
Author

martinheidegger commented Dec 9, 2016

Shouldn't there be just one source of truth?

That's by definition the one in core.

So NPM's reimplementation should follow core in any case, right?

The one in core, most likely. It takes shortcuts where it can, e.g., internalModuleStat() and internalModuleReadFile().

If that is the case: Can Node.js expose the logic used to identify the prefix somehow like: module.prefix in order for NPM to make use of that "one source of truth"?

@bnoordhuis
Copy link
Member

Then it becomes official API and can't be changed anymore. That was expressly not my intent when I added internalModuleStat() and internalModuleReadFile().

@martinheidegger
Copy link
Author

@bnoordhuis I am not sure if this was clear but do not mean to say that internalModuleStat and internalModuleReadFile should become API's. only the prefix. The default prefix to be used for module location. (a simple path-String)

@bnoordhuis
Copy link
Member

What do you mean by prefix? The string 'node_modules'? That doesn't seem worthwhile, that's so set in stone it's never ever going to change.

@martinheidegger
Copy link
Author

I am sorry: I meant the default output that you also get when calling npm config get prefix. Unless the prefix is set it will fallback on the folder that node also uses to check for packages.

@bnoordhuis
Copy link
Member

Ah sorry, I see what you mean now. It kind of exists but it's scattered across lib/module.js. If npm wanted to use that, they should file an issue or pull request to make it public API.

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

No branches or pull requests

3 participants