Skip to content

Commit

Permalink
mkYarnModules: only run yarn install inside a workspace-structure i…
Browse files Browse the repository at this point in the history
…f needed

It's known that using workspaces usually produces a slightly different
`node_modules/`[1]. While this used to be no big deal for us, we actually
triggered a case where "dependency hoisting" — the process of moving
dependencies of workspaces up in the final `node_modules/`-tree —
becomes a problem, basically issue 5705[2].

The problem was triggered by the following - simplified - dependency
tree:

    <root>
    |- commander (6.1)
    |- mocha (3)
    |  `- commander (2.9)
    `- sql-generate (1.5)
       `- commander (2.9)

`yarn2nix` created a workspace of the package `root` with
`node_modules/` containing `mocha`/`sql-generate`. In the `yarn
install` operation these are all placed into a single `node_modules/` in
`$out`, however `commander` from 2.9 is preferred over 6.1:

    $ yarn why commander
    [...]
    => Found "[email protected]"
    info Has been hoisted to "commander"
    info Reasons this module exists
       - "workspace-aggregator-dc20c757-df45-41d7-bdba-927d267212a3" depends on it
       - Hoisted from "_project_#root#mocha#commander"
       - Hoisted from "_project_#root#sql-generate#commander"
    => Found "root#[email protected]"
    info This module exists because "_project_#root" depends on it.

Even specifying `nohoist` for both dependencies inside the temporary
`package.json` that's built by `yarn2nix` doesn't solve the problem
since `yarn` still doesn't put the newer `[email protected]` into
`$out/node_modules/`, I don't really know why, could be a `yarn`-bug
though.

Anyways, we're not even using the workspace feature and since it's known
to produce different results, it's IMHO easier to just disable the
entire feature if no `workspaces` are defined in `package.json`.

[1] https://classic.yarnpkg.com/lang/en/docs/workspaces/#toc-limitations-caveats
[2] yarnpkg/yarn#5705
  • Loading branch information
Ma27 committed Jan 20, 2022
1 parent 2ec29a2 commit 4497a22
Showing 1 changed file with 13 additions and 6 deletions.
19 changes: 13 additions & 6 deletions default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ in rec {
let
offlineCache = importOfflineCache yarnNix;

requiresWorkspaceSupport = (builtins.fromJSON (builtins.readFile packageJSON))?workspaces;

mkBuildExtra = name: { buildInputs ? [], postInstall ? "", ... }: with lib;
let
inherit (callPackage yarnNix {}) packages;
Expand Down Expand Up @@ -157,11 +159,16 @@ in rec {
buildPhase = ''
runHook preBuild
mkdir -p "deps/${pname}"
cp ${packageJSON} "deps/${pname}/package.json"
cp ${workspaceJSON} ./package.json
cp ${yarnLock} ./yarn.lock
chmod +w ./yarn.lock
${if requiresWorkspaceSupport then ''
mkdir -p "deps/${pname}"
cp ${packageJSON} "deps/${pname}/package.json"
cp ${workspaceJSON} ./package.json
cp ${yarnLock} ./yarn.lock
'' else ''
cp ${packageJSON} package.json
cp ${yarnLock} yarn.lock
''}
chmod +w yarn.lock
yarn config --offline set yarn-offline-mirror ${offlineCache}
Expand All @@ -180,7 +187,7 @@ in rec {
mkdir $out
mv node_modules $out/
mv deps $out/
${lib.optionalString requiresWorkspaceSupport "mv deps $out/"}
patchShebangs $out
runHook postBuild
Expand Down

0 comments on commit 4497a22

Please sign in to comment.