Loading from multiple node_modules with version > 4.4.1

[duarten]

🐞 bug report

Affected Rule

nodejs_binary / nodejs_test

Is this a regression?

Yes, 4.4.1 works fine, but 4.4.2 and 4.4.3 are broken. This is likely related to the linker refactor.

Description

When a package is symlinked under node_modules due to, for example, the packge_name attribute of js_library, then node modules will be loaded from a different directory.

I noticed this with react, which complains when multiple instances are loaded.

We can see the problem from the following (edited) stack trace:

      <snip>
      at useMemo (node_modules/repro/node_modules/react/cjs/react.development.js:1531:20)
      at Component (../component/component.tsx:4:12)
      <snip>
      at act (node_modules/react-dom/cjs/react-dom-test-utils.development.js:1042:14)
      <snip>
      at runCLI (node_modules/@jest/core/build/cli/index.js:173:3)

Notice how we first load react from node_modules/react-dom, and later from node_modules/repro/node_modules.

The component package contains the following rule:

js_library(
    name = "component",
    package_name = "repro/component",
    visibility = ["//visibility:public"],
    deps = ["component-ts"],
)

Omitting the workspace name (repro) from the package_name seems to solve the problem (i.e., changing it to "component" or "blah/component").

🔬 Minimal Reproduction

I committed a reproduction here.

🔥 Exception or Error

In this case, react complains and the test fails.

🌍 Your Environment

Operating System:

  
  macOS
  

Output of bazel version:

  
  4.2.1
  

Rules_nodejs version:

  
  4.4.3
  

[duarten]

This is still happening with version 4.6.0.

@alexeagle, @gregmagolan any tips on how to fix this would be greatly appreciated :)

[alexeagle]

hi sorry, we've been 100% heads-down on the 5.x breaking changes and getting that release out. I doubt this is fixed there but we should at least verify it's still reproducible on 5.0.0-rc.0 which I'm going to cut todayish.

[duarten]

It still reproduces with 5.0.0-rc.1. I've updated the reproducer here.

[m0ar]

Could this possibly be related to my issue here? 🤔 #3340

[duarten]

It could be! If you are on 5.x.x it may also be related to #3264, so you should try to set exports_directories_only to False in your yarn_install or npm_install rule.

[m0ar]

@duarten Thanks for the clue! 🙏 Indeed solved the problem. Spend most of Friday trying to hunt this one out.

If you understand the mechanics behind the problem: will it be solved when #3264 is taken care of? I have a hard time figuring out if this is a bug or a side-effect of an optimisation. Anyway if it's the latter, it shouldn't be the default behaviour at least... :)

[duarten]

I think it's a bug and will be fixed in #3264, but I don't understand what exactly is causing the issue, I just had recently spent some time trying to figure a problem that was caused by it :)

[thesayyn]

Fixed by #3380