What about undeclared dynamic dependencies?

[alexeagle]

npm has the semantics that all programs see the entire node_modules tree. This allows libraries to have the dubious behavior "I don't declare any kind of dependency on foolib in my package.json, but if you just install foolib into your project, I'll have different behavior". That's because a program can just try { require('foolib') } and have conditional behavior based on whether it was found.

It seems to me that any formulation of individual npm packages under bazel, where a package's dependencies are expected to be declared statically, is going to have the flaw that some program behavior no longer works because the dynamic lookup of foolib fails when it's not in the declared action inputs.

Have you thought about this at all?

[alexeagle]

Ping @pcj in case you don't check new issues in this repo very often.

[pcj]

I see your point.

My personal opinion is that implicit/undeclared dependencies in npm modules should be regarded as bugs. As an npm library developer I SHOULD declare all my dependencies. For bazel, it would seem that end-users / application develops will need to be aware of these undeclared dependencies and explicitly declare them.

Granted I'm sure this is an over-simplification. WDYT?

[alexeagle]

Yeah, I agree with that, but it's unfortunate that program behavior changes when run under Bazel. Will make it harder to get wide-scale adoption. I'm glad you didn't encounter that problem in rules_node yet, maybe it's very uncommon to rely on this. I think for now we'll add optionalDependency in packages we control that have this problem, and maybe have some way in our new design for rules_nodejs that lets you specify these invisible deps in the `yarn_install` rule.

…

On Fri, Aug 24, 2018 at 8:35 AM Paul Cody Johnston ***@***.***> wrote: I see your point. My personal opinion is that these implicit/undeclared dependencies in npm modules is a bug. As an npm library developer I SHOULD declare all my dependencies. For bazel, it would seem that end-users / application develops will need to be aware of these dependencies and explicitly declare them. Granted I'm sure this is an over-simplification. WDYT? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#73 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAC5I5uu24_kbfWEq9jCRRp3YVwSjvtgks5uUB0zgaJpZM4WKwfm> .

[pcj]

Do you have a concrete example? Could be a foundation to educate community better.

[alexeagle]

We just made a change to Angular, such that the closure compiler compat layer (tsickle) is not required for @angular/compiler-cli. We attempt to require it, and if found, then we output code that has closure compiler JSDoc annotations.

[gregmagolan]

I don't think using optionalDependencies approach will work for the tsickle case. npm and yarn always attempt to install optionalDependencies and if the install fails then it is ignored.

Packages are meant to handle a missing optional dependency gracefully at runtime. For example, chokidar has an optional dependency on fsevents, which installs fine on osx/linux but fails to install on Windows. chokidar would have to handle gracefully at runtime if fsevents is missing.

For the tsickle case, we'd need bazel specific attribute in a package.json that would mean check if this dependency exists in node_modules and add it as a transitive dep in the filegroup if it does. npm & yarn wouldn't install and deps listed in that attribute.

Thoughts?

[alexeagle]

I'm bumping into this again in bazelbuild/rules_typescript#398

[alexeagle]

I think we are settling on bazelDynamicDependencies
We would like to use dynamicDependencies but maybe npm will decide that has a meaning in the future?

[sgammon]

@alexeagle did bazelDynamicDependencies ever get implemented?