[Bug] Transitive dependencies must follow the top level project's nodeLinker setting

[molst]

Expected behavior

When using the portal protocol, all transitive dependencies should resolve using their local nodeLinker setting.

Current behavior

When the top level dependency uses node-modules, linking fails at the top level when a referenced library uses the default PnP resolution.

Notes

• In my particular case, I unfortunately have to use the nodeLinker: node-modules setting in my top level project due to this issue. This means that all my libraries also have to fall back to using node-modules.
• If a sub project uses the default PnP resolution the parent project will fail with ERR_MODULE_NOT_FOUND when referencing the sub project.

[larixer]

@molst Thank you for opening this issue. If you are using pnp library from node_modules, you can make this pnp library interoperable for calls from node_modules by having separate entry point which just requires .pnp.js first, your libraries don't have to fall back to using node-modules, they just need have small additional code to be callable from node_modules.

[larixer]

When using the portal protocol, all transitive dependencies should resolve using their local nodeLinker setting.

This is certainly not an expected behaviour and hence this issue is not a bug. Yarn must not add/modify files outside of <projectRoot> folder, except in temporary folders. The bug is if Yarn adds files into folder pointed by portal, it must not.

[molst]

@molst Thank you for opening this issue. If you are using pnp library from node_modules, you can make this pnp library interoperable for calls from node_modules by having separate entry point which just requires .pnp.js first, your libraries don't have to fall back to using node-modules, they just need have small additional code to be callable from node_modules.

Alright, I think I understand what you mean, but to make sure, do you have some example of how to accomplish this? I tried what I suppose should be the simplest thing following your guidance which was to to add this at the top of the file I'm trying to load, which is vectors.mjs (see below). Unfortunately without luck so far.. (I have to load require because I'm using ES modules):

import { createRequire } from "module";
const require = createRequire(import.meta.url);
var pnp = require('../.pnp.js');

I'm using this kind of exports declaration in package.json

  "exports": {
    "./util": "./src/util.mjs",
    "./vectors": "./src/vectors.mjs"
  }

[larixer]

To be fair I have hard time understanding what your setup is, as this issue didn't mention MJS initially. My recommendation was about requiring commonjs pnp modules from commonjs node modules.

[arcanis]

@molst Please put exact reproduction steps in your issues. Anything harder than cloning a repository and running two commands increases the number of back and forth required 🙂

The issue template even mentions a tool called Sherlock that we strongly recommend for issues opened against this repository, precisely for this reason.

[molst]

That's totally understandable! Thanks for your fast response.
While setting up the demo repo I realized that to reproduce the probem I need to run a server. It uses nodejs 12 with --experimental-modules activated, which had me realize that this issue quite likely is a consequence of 1150.
So, maybe we should close this one. I can live with node-modules until all the ES modules stuff is sorted out.