Regarding _requiredBy

[iarna]

You all are using the _requiredBy field in installed modules:

https://github.com/Microsoft/nodejstools/blob/master/Nodejs/Product/Npm/SPI/NodeModules.cs#L62

This concerns me because the contents of keys beginning with _ are not a part of npm's semver contract, so we can and will change these in the future. Further, it's not the sort of thing we even call out in changelogs as it's entirely internal to npm.

I'd love to work with you all to find a solution that can be a part of our semver contract and is thus more supportable. To do that I would find it helpful if you all could help me understand what questions you're looking for answers to about modules. From the code linked to, it looks like the question is just "is this a top level dependency?" – is that correct?

As you are using this, you should be aware of some further caveats:

1. This is essentially a write-only field for npm. It uses this to store what it was thinking at install time, but it recomputes these after reading them off disk. This means none of npm's logic actually depends what's in the file.
2. If an installed module was bundled and you installed it with npm < 3.5.0, its package.json won't have any of this metadata in it. npm prior to 3.5.0 didn't rewrite bundle module's package.json files at all. Similarly, if any of the modules were originally installed with npm less than 3, then they won't have this metadata.
3. It is made up of either #USER (which means the user installed it but didn't save it to their node_modules folder), #EXISTING (which means we found it here and it previously didn't have a _requiredBy key, typically folders w/ npm@2 modules), a module-centric path to a thing that requires it (eg /moduleA/moduleB), or #DEV: followed by a module-centric path to a thing that requires it (eg #DEV:/moduleC/moduleD). #DEV: type entries are (unsurprisingly) created when the module is required by a development dependency.

[mousetraps]

@iarna As mentioned earlier, you rock 😃

Thanks for the detailed explanation. It would be awesome to find a way to get at this information without reading from the _requiredBy property. And yes, the question is indeed "is this a top level dependency?" We used to be able to discern this info more easily from the directory tree created by npm v2, but now we're forced to reason about it ourselves. Our one constraint is that we cannot take a dependency on executing node / npm and need to be able to understand it from the filesystem itself. See the following comment for more reasoning on this decision:

Initially, we considered using npm ls --json to understand the tree. This makes sense for the solution explorer integration, which already takes a dependency on npm and where race conditions are not an issue. However, it takes a while (several seconds), spikes the cpu, and is going to cause all sorts of performance issues and race conditions if we attempt to use it during analysis which is far more complicated, and triggered far more frequently.
#233 (comment)

Additionally, it would hurt IntelliSense perf in cases that npm is not installed or a different version of npm is installed.

TLDR: Basically, if we can find a way to answer "is this a top level dependency?" via the filesystem in some efficient manner, then we're happy. (Especially since we are also considering using this logic for some other efforts beyond Node.js Tools going forward, so it would be great to land on something more dependable here.)

Regarding the caveats...

This is essentially a write-only field for npm. It uses this to store what it was thinking at install time, but it recomputes these after reading them off disk. This means none of npm's logic actually depends what's in the file.

Indeed, we explictly chose to make some tradeoffs on correctness in favor of something that got us most of the way there without compromising on performance. E.g. we're okay with assuming npm built the tree, so write-only properties are not an issue.

If an installed module was bundled and you installed it with npm < 3.5.0, its package.json won't have any of this metadata in it. npm prior to 3.5.0 didn't rewrite bundle module's package.json files at all. Similarly, if any of the modules were originally installed with npm less than 3, then they won't have this metadata.

We address the npm < 3 case by falling back to separate logic in the event that _requiredBy does not exist: https://github.com/Microsoft/nodejstools/blob/master/Nodejs/Product/Npm/SPI/NodeModules.cs#L70

Thanks for the heads up on npm < 3.5.0 + bundled dependencies. Luckily we haven't gotten any user reports on this yet, but hadn't realized that!

It is made up of either #USER (which means the user installed it but didn't save it to their node_modules folder), #EXISTING (which means we found it here and it previously didn't have a _requiredBy key, typically folders w/ npm@2 modules), a module-centric path to a thing that requires it (eg /moduleA/moduleB), or #DEV: followed by a module-centric path to a thing that requires it (eg #DEV:/moduleC/moduleD). #DEV: type entries are (unsurprisingly) created when the module is required by a development dependency.

Awesome, thanks. Looks like we'll need to update the logic a bit here if we want to compensate for installing a package and it's dev dependencies as a dependency. Luckily we have yet to see reports on this and I don't expect this to be a common use case for our users. (top-level dev-dependencies are most important for our purposes)

[paulvanbrenk]

Going through old issues, since I own them now.

@iarna I think I understand your concern. (Don't use internal fields. we'll break you!) Do you have (the beginning of) an answer to a reliable way to figure out if a package is a top level dependency?

[paulvanbrenk]

I think we can implement the behavior here using the npm list --json command

[iarna]

Hey! So obviously npm list --json will always get you an answer, though I gather you'd avoided it previously due to speed issues and while that's improved some, it's still a hard latency hit.

I would recommend you all use the package-lock.json these days. While it only directly encodes the relationship in one direction (requires), the reverse can be deduced, and we provide a module for doing just that: https://www.npmjs.com/package/npm-logical-tree

(We've maintained support for _requiredBy for y'all, and don't have any immediate plans to retire it, but I'd still be happy to see you move beyond that.)

[paulvanbrenk]

Thanks @iarna. I think we'll do our own parsing of the package-lock.json, since starting node to run some JavaScript will probably be as slow as invoking npm.

[amcasey]

@uniqueiniquity Is there any easy way we can move off this internal dependency?