Fix incorrectly hoisted JS dependencies

[ravicious]

Yarn v1 has a bug related to hoisting dependencies when using workspaces. Instead of hoisting dependencies as defined in package.json of our workspaces, the hoisting algorithm seems to be impacted by transitive dependencies as well.

For example, if web/packages/build/package.json depends on @babel/core ^7.23.2 and @storybook/builder-webpack5 ^6.5.16 which in turn depends on @babel/core ^7.12.10, Yarn will hoist the @babel/core version resolved from that storybook dependency rather than the direct @babel/core dep in package.json. What's more, it doesn't even seem to dedupe those deps correctly despite overlapping version ranges for @babel/core.

Solutions

The Yarn team has stated that this won't be fixed in v1, so if we want to address this, we have to either upgrade to v2 or move to another package manager.

From my own attemtps, upgrading to v2 doesn't make me feel optimistic as the official migration guide does not account for such a simple thing as informing you that you should empty all node_modules dirs in the project (or it could just do that for you). Ryan also had mixed feelings from his recent attempt to upgrade one of his projects to v2.

Alas, when it comes to other package managers pnpm doesn't seem to work properly with electron-builder due to how electron-builder approaches bundling deps. bun has the issue with a binary lockfile format, meaning that on every conflict we'd have to throw away the lockfile and start fresh – this is not something you want to do on a git conflict as it unceremoniously updates all packages. See the discussion in bun's repo for more details.

npm would be another candidate to investigate. IIRC, npm has a rather unorthodox approach to caching which involves some additional setup if you want to, for example, cache your Docker layers properly. npm clean-install is the only method to install packages while respecting the lockfile. But it has the quirk where it'll first remove node_modules completely. So you not only have to cache node_modules but also npm's cache folder. Also, while it does seem to support workspaces, it's unknown how well it does so.

See also Grzegorz's report from trying to update Yarn to v2 and changing how we approach hoisting deps within workspaces.

Alternatives

An alternative would be to dedupe overlaping version ranges by running yarn-deduplicate manually and then following up with another invocation of yarn install to clean up any orphaned packages. I remembered that @jentfoo used yarn-deduplicate a while back. Calling it without any args introduces a big change though. Fortunately, it also supports flags to minimize the amount of changes.

In #33355, I updated and deduplicated babel deps by running npx yarn-deduplicate yarn.lock --scopes @babel. The tool also supports the --packages flag for individual packages. This is going to work as long as both Storybook and our workspaces have overlapping version ranges. I don't know how Yarn is going to behave if those ranges don't overlap.

[ravicious]

As a stop gap, we could add yarn-deduplicate to our build deps, add a script in the root package.json called deduplicate that's just yarn-deduplicate && yarn install and then add a bash script similar to yarn-install-frozen-lockfile.sh that runs during CI and rejects PRs which add dependencies without deduping the lockfile.

[gzdunek]

Sounds good to me.

[ravicious]

corepack can automatically install the correct version of a specific package manager. This seems like a super good feature, as people wouldn't have to manually manage the version of whatever manager that we pick.

https://x.com/mattpocockuk/status/1796616720280375393