overrides doesn't work as expected

[RoB8080]

pnpm version: 6.30.0

Code to reproduce the issue:

Just a package.json like this:

{
  "name": "test-pnpm",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "pnpm": {
    "overrides": {
      "lz-string": "github:pieroxy/lz-string#2f749bc9fe3cc889fa9e32d2769a930d977596e6"
    }
  }
}

Then run pnpm add lz-string

Expected behavior:

Updated package.json like this:

{
  // ...
  "dependencies": {
    "lz-string": "github:pieroxy/lz-string#2f749bc9fe3cc889fa9e32d2769a930d977596e6"
  }
  // ...
}

Actual behavior:

Overrides don't have effect when add a new direct dependency, and the package.json become:

{
  // ...
  "dependencies": {
    "lz-string": "^1.4.4" // latest version
  }
  // ...
}

After running pnpm add lz-string again, then it will be corrected.

Additional information:

• node -v prints: 14.17.2
• Windows, macOS, or Linux?: macOS

[RoB8080]

Seems the problem is overrides are handled in readPackage hook.

When adding a new package, the manifest doesn't contain it and it will be treated as normal ('latest' in above case).

Is this logic as expected? @zkochan

cc @wre232114

[zkochan]

I don't think the overrides should override direct dependencies at all. If you need to change a direct dependency, why do you need an override? Just change the direct dependency. Overrides are needed for subdependencies, where you can directly access the package.json files where they are declared.

[wre232114]

I don't think the overrides should override direct dependencies at all. If you need to change a direct dependency, why do you need an override? Just change the direct dependency. Overrides are needed for subdependencies, where you can directly access the package.json files where they are declared.

But I think this brings inconsistence, from the user's perspective, the overrides should unify all the version of the same package, no matter it is a direct dependencies or not.

And in this issue, the first time we add a package it is latest(the overrides doesn't work), but if we do pnpm add again, the overrides works. I think we should make the first time pnpm add works with overrides too, which maybe the most user expect

[RoB8080]

I don't think the overrides should override direct dependencies at all. If you need to change a direct dependency, why do you need an override? Just change the direct dependency. Overrides are needed for subdependencies, where you can directly access the package.json files where they are declared.

Firstly, I understand you point. But according to the purpose of this field:

This field allows you to instruct pnpm to override any dependency in the dependency graph. This is useful to enforce all your packages to use a single version of a dependency, backport a fix, or replace a dependency with a fork.

I think it would be better to override the resolution for direct dependency.

Also, currently the behavior doesn't strictly follow your point as i tested.

• when a lockfile is present and having overrides in it, adding dependency in package.json & running pnpm install will result in overridden one in pnpm-lock.yaml.
• adding a new package twice will result in overridden resolution in both package.json and pnpm-lock.yaml.

Anyway, I'm glad to contribute code.

[zkochan]

The inconsistent behavior should be fixed for sure. pnpm add and pnpm install should work the same with overrides.

I checked how this works with npm CLI. When you use an override for a direct dependency, an error is thrown

I think this makes sense.

[zkochan]

Yarn resolutions seem to work as you would like it to work. They override direct dependencies as well.

So we can do what npm does. Or we can do what Yarn does.

Vote with +1 for Yarn (overrides change direct dependencies)
Vote with -1 for npm (overrides don't change direct dependencies)

cc @pnpm/collaborators

[zkochan]

Looks like most support the Yarn way. I don't have objections in that case to make it work the same way as in Yarn. So to apply the overrides to direct dependencies as well.

[shernshiou]

I have updated pnpm to v6.32 but it still is not overriding direct dependency. Maybe I am understanding wrong.

❯ pnpm audit
┌─────────────────────┬───────────────────────────────────────────────────┐
│ high                │ Authorization bypass in url-parse                 │
├─────────────────────┼───────────────────────────────────────────────────┤
│ Package             │ url-parse                                         │
├─────────────────────┼───────────────────────────────────────────────────┤
│ Vulnerable versions │ <1.5.6                                            │
├─────────────────────┼───────────────────────────────────────────────────┤
│ Patched versions    │ >=1.5.6                                           │
├─────────────────────┼───────────────────────────────────────────────────┤
│ More info           │ https://github.com/advisories/GHSA-rqff-837h-mm52 │
└─────────────────────┴───────────────────────────────────────────────────┘
1 vulnerabilities found
Severity: 1 high
❯ pnpm audit --fix
1 overrides were added to package.json to fix vulnerabilities.
Run "pnpm install" to apply the fixes.

The added overrides:
{
  "url-parse@<1.5.6": ">=1.5.6"
}
❯ pnpm install

❯ pnpm --version
6.32.0
❯ pnpm audit
┌─────────────────────┬───────────────────────────────────────────────────┐
│ high                │ Authorization bypass in url-parse                 │
├─────────────────────┼───────────────────────────────────────────────────┤
│ Package             │ url-parse                                         │
├─────────────────────┼───────────────────────────────────────────────────┤
│ Vulnerable versions │ <1.5.6                                            │
├─────────────────────┼───────────────────────────────────────────────────┤
│ Patched versions    │ >=1.5.6                                           │
├─────────────────────┼───────────────────────────────────────────────────┤
│ More info           │ https://github.com/advisories/GHSA-rqff-837h-mm52 │
└─────────────────────┴───────────────────────────────────────────────────┘
1 vulnerabilities found
Severity: 1 high

[zkochan]

Make a repro repository because it works in my case.

[shernshiou]

Make a repro repository because it works in my case.

Thanks for the confirmation. I have tested. The override is successful when

"pnpm": {
    "overrides": {
      "url-parse": ">=1.5.6"
    }
}

but not

"pnpm": {
    "overrides": {
      "url-parse@<1.5.6": ">=1.5.6"
    }
}

as suggested by pnpm audit --fix

[briangonzalez]

Can confirm this happens to me as well.

[zkochan]

If you have issues related to overrides, open a new issue with steps to reproduce. A repository that reproduces the issue would be highly appreciated.