[Bug]: Yarn is sensitive to the order in which resolutions are specified

isker commented 2 years ago

Self-service

[ ] I'd be willing to implement a fix

Describe the bug

Yarn is sensitive to the order of resolutions provided in package.json, which is unexpected since they are the keys of the JSON object and thus their order should not change anything.

To reproduce

The issue reproduces with all linkers, the sample package.json to demonstrate it:

{
  "resolutions": {
    "is-odd/is-number": "2.0.0",
    "is-number": "4.0.0"
  },
  "dependencies": {
    "is-odd": "^1.0.0"
  }
}

Try to change order inside resolutions and execute yarn why is-number.

Sherlock repro:

await packageJsonAndInstall({
  resolutions: {
    "is-odd/is-number": "2.0.0",
    "is-number": "4.0.0"
  },
  dependencies: {
    "is-odd": "^1.0.0"
  }
});

const whyFirst = JSON.parse(await yarn('why', 'is-number', '--json'));

await packageJsonAndInstall({
  resolutions: {
    "is-number": "4.0.0",
    "is-odd/is-number": "2.0.0"
  },
  dependencies: {
    "is-odd": "^1.0.0"
  }
});

const whySecond = JSON.parse(await yarn('why', 'is-number', '--json'));
expect(whySecond).toEqual(whyFirst);

Original issue

Describe the bug

When using the node-modules linker with multiple per-package resolutions, the generated node_modules tree is sensitive to the order in which the resolutions are specified.

As described at https://yarnpkg.com/configuration/manifest#resolutions, you can specify not only a global resolution, but also resolutions for immediate dependencies of specific packages.

When using the node-modules linker, the behavior in this situation is actually sensitive to the order in which the resolutions are specified. By changing the order in which resolutions are specified, you can get different linking outcomes.

To reproduce

Follow the commit history of this repo https://github.com/isker/resolutions-ordering.

We have a direct dependency on latest ajv (8.x), but also use eslint which has both a direct dependency on ajv 6.x, and an indirect dependency on ajv 6.x, via eslint -> @eslint/eslintrc -> ajv.

In https://github.com/isker/resolutions-ordering/commit/c8005757b88589c17dad2236b922c6a40ce4848e we add resolutions affecting all three of these dependency paths, but as the commit message describes, the the eslint/ajv resolution does not do anything; eslint gets the global resolution.

In the next commit https://github.com/isker/resolutions-ordering/commit/6840032771840c2a3d7e1013273ce11a143af268 we only change the ordering of the resolutions, and get the expected outcome. We can thus say that the behavior of the node-modules linker is sensitive to the ordering of resolutions.

The behavior should not be sensitive to the ordering of resolutions.

Environment

# Note that I do not think this bug is sensitive to environment.
λ yarn dlx -q envinfo --preset jest

  System:
    OS: macOS 12.3.1
    CPU: (12) x64 Intel(R) Core(TM) i7-8850H CPU @ 2.60GHz
  Binaries:
    Node: 16.10.0 - /private/var/folders/r1/270_r5sj2fq58rrx4cr9rqzm0000gq/T/xfs-b340cd10/node
    Yarn: 3.2.0 - /private/var/folders/r1/270_r5sj2fq58rrx4cr9rqzm0000gq/T/xfs-b340cd10/yarn
    npm: 7.24.0 - ~/.local/share/nvm/v16.10.0/bin/npm

Additional context

No response

larixer commented 2 years ago

I can confirm that resolution order matters in this case, but it is not scoped to node-modules linker, it does reproduce with pnp as well, you don't need to use jq inside node_modules to see this, yarn why is enough. @arcanis is this intended?

yarnbot commented 2 years ago

This issue reproduces on master:

Error: expect(received).toEqual(expected) // deep equality

- Expected
+ Received

  Object {
    "children": Object {
-     "is-number@npm:2.0.0": Object {
-       "descriptor": "is-number@npm:2.0.0",
-       "locator": "is-number@npm:2.0.0",
+     "is-number@npm:4.0.0": Object {
+       "descriptor": "is-number@npm:4.0.0",
+       "locator": "is-number@npm:4.0.0",
      },
    },
    "value": "is-odd@npm:1.0.0",
  }
    at module.exports (evalmachine.<anonymous>:25:19)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async /github/workspace/.yarn/cache/@arcanis-sherlock-npm-2.0.3-558f52b79f-286d94b96d.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:57:13
    at async executeInTempDirectory (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-2.0.3-558f52b79f-286d94b96d.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:18:16)
    at async executeRepro (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-2.0.3-558f52b79f-286d94b96d.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:25:12)
    at async ExecCommand.execute (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-2.0.3-558f52b79f-286d94b96d.zip/node_modules/@arcanis/sherlock/lib/commands/exec.js:26:38)
    at async ExecCommand.validateAndExecute (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-4061026d74.zip/node_modules/clipanion/lib/advanced/Command.js:161:26)
    at async Cli.run (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-4061026d74.zip/node_modules/clipanion/lib/advanced/Cli.js:74:24)
    at async Cli.runExit (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-4061026d74.zip/node_modules/clipanion/lib/advanced/Cli.js:83:28)

isker commented 2 years ago

Hah, sorry about all that, in every regard. I tried my best.

yarnpkg / berry