Support for optionalDependencies

[Profpatsch]

Looks like npm finally added a field optionalDependencies.

So we should support it somehow.

I have no idea what the semantics are.

[sternenseemann]

Maybe a switch to enable including optionalDependencies in the node_modules directory? Such a switch would be a last resort if a build fails otherwise, since theoretically you'd expect an optional dependency being in the dependencies field somewhere in the dependency tree.

[Profpatsch]

since theoretically you'd expect an optional dependency being in the dependencies field somewhere in the dependency tree.

That wouldn’t make the require() for package Y work however, because we just symlink the tree; let’s see X has dep D it as a normal dependency and Y has it as optionalDependency:

X/node_modules/D  (symlink to /nix/store/…)
Y/node_modules/… (no reference to D because it’s an optionalDependency, require() returns `null`)

[Profpatsch]

tbh I have no idea how to handle this situation except for making it easy to add optional dependencies to certain packages in the lockfile. We already have at least partial override support, so maybe this is just a matter of documentation (after finding a use-case where it makes sense to enable an optional dependency that is).

[Profpatsch]

Another step would be to figure out the following: does the yarn.lock file contain optionalDependencies (and peerDependencies) by default, or do you have to tell yarn explicitly to include them?

[sternenseemann]

That wouldn’t make the require() for package Y work however, because we just symlink the tree; let’s see X has dep D it as a normal dependency and Y has it as optionalDependency:

This problem could be circumvented by wrapping output executables and collecting all node_modules directories into NODE_PATH (now that I write it — sounds horrible). The big problem here is that apparently support for NODE_PATH in the node community seems to be rather poor, some even call it deprecated. For example browserify/resolve which claims to implement the nodejs require.resolve() algorithm diverts from nodejs by not supporting NODE_PATH (see this dismissed PR and this issue).