Use yarn nodeLinker: node-modules to make tests faster

[geigerzaehler]

This change almost halves the execution time of main.test.ts (from 57s to 35s on my machine using NOCK_ENV=replay). I expect similar results on CI.

The end-to-end tests run corepack in a new node process about 150 times. With the pnp linker, node is asked to require .pnp.cjs on every startup which significantly contributes to test time.

As an alternative, I tried removing --require=.pnp.cjs from NODE_OPTIONS in runCli. After all, we’re running the bundled version of corepack. But this did not work because tests/recordRequests.js is also required which in turn needs better-sqlite3.

[MikeMcC399]

• This PR would also work around the Yarn PnP limitation described in issue Contributor Windows build on non-HOMEDRIVE 'Could not resolve "clipanion"' #598 and it would mean that documentation PR docs: add contributor windows os non-homedrive usage #606 could be reverted.

[geigerzaehler]

I expect similar results on CI.

Yeah, for the Node 22 test on Ubuntu total job time drops by 40s to 1min. And on Windows it drops by 3:40 to 8min.

[arcanis]

I don't mind, it's true that we're in kind of a special case here since we're spawning a very large amount of subprocesses in our tests.

[arcanis]

Why those changes?

[geigerzaehler]

With the original patch I get the following error:

Error: Directory import '.../corepack/node_modules/clipanion/lib/platform' is not supported resolving ES modules imported from ...corepack/node_modules/clipanion/lib/advanced/Cli.mjs

In fact, Cli.mjs uses import ... from './platform' but Node requires relative import specifiers to always point to files. I guess this worked before because yarn’s resolver is implemented differently and supports relative package imports.

[MikeMcC399]

@aduh95

Will you merge this? It seems like a good improvement.

[arcanis]

I would prefer to keep PnP, if only to have a Node.js project dogfooding the default yarn install strategy. The perf impact is unfortunate, but 30% doesn't feel too dramatic an increase that this is a must-do.

[MikeMcC399]

Testing locally I observed the following timings and savings with Node.js 22.14.0 LTS running yarn test:

Duration	Windows 11 24H2	Ubuntu 22.04.2 LTS
No change (pnp)	338.35s	163.50s
Change to node-modules	179.53s	95.36s
Savings	47%	42%

Depending on needs and priorities I suggest to either merge the PR or to reject and close it.

My personal preference would be to have it merged, as it not only improves performance, it also works around issue #598.

[arcanis]

I'll close it then; as a package manager author I find it somewhat reassuring to have at least one Node.js project using PnP (or loaders in general). If Corepack moves out of the Node.js org it can be revisited.