Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Updating a dependency in a workspace with overrides duplicates the package #6979

Closed
2 tasks done
Tracked by #920
thislooksfun opened this issue Nov 9, 2023 · 10 comments
Closed
2 tasks done
Tracked by #920
Labels
Bug thing that needs fixing Priority 2 secondary priority issue Release 10.x

Comments

@thislooksfun
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

This issue exists in the latest npm version

  • I am using the latest npm

Current Behavior

Updating a dependency should update the installed package in-place, not cause duplication.

Expected Behavior

The package is duplicated, and the old version is still present.

Steps To Reproduce

  1. Make a new project
  2. Enable workspaces, and make a workspace package (I'll refer to it as foo)
  3. Add a dependency to foo, and run npm install
  4. The dependency will be hoisted and installed at <root>/node_modules/<package>
  5. Update the dependency (either manually by editing the package.json file and running npm install, running npm install <package>@version from inside foo, or running npm install <package>@version -w <path/to/>foo`

At this point the new version of the dependency will be installed to <root>/<path/to/>foo/node_modules/<package>, and the old version will still be present at <root>/node_modules/<package>.

Environment

  • npm: 10.2.3
  • Node.js: v18.18.2
  • OS Name: macOS 13.6.1
  • System Model Name: Macbook Pro
  • npm config:
save-exact = true 
save-prefix = "" 
@thislooksfun thislooksfun added Bug thing that needs fixing Needs Triage needs review for next steps Release 10.x labels Nov 9, 2023
@thislooksfun
Copy link
Author

Additional info:

  • This also happens on npm v9, and probably earlier
  • Manually running npm dedupe does fix the issue, but installing with prefer-dedupe does not

@wraithgar
Copy link
Member

Cannot reproduce with the steps given. I suspect what's happening here is that the old version is valid for some other dependency in the tree, but the new version isn't, so the new version won't hoist.

~/D/n/s/a $ npm init -y; npm init -y -w workspace-a
~/D/n/s/a $ npm i [email protected] -w workspace-a
~/D/n/s/a $ npm i [email protected] -w workspace-a
~/D/n/s/a $ npm ls
[email protected] /Users/wraithgar/Development/npm/scratch/a
└─┬ [email protected] -> ./workspace-a
  └── [email protected]

~/D/n/s/a $ ls node_modules/
abbrev/      workspace-a@
~/D/n/s/a $ ls node_modules/workspace-a/
package.json

@wraithgar wraithgar closed this as not planned Won't fix, can't repro, duplicate, stale Nov 14, 2023
@thislooksfun
Copy link
Author

thislooksfun commented Nov 15, 2023

Ah. It seems the bug is more subtle than I first thought. It only happens if the root package.json has a non-empty overrides definition.

Here is a script that reproduces the issue:

npm init -y
npm init -y -w workspace-a

# Add an "overrides" section to the root package.json
pkg=$(jq '.overrides |= {"ava": "5.3.1"}' package.json)
echo "$pkg" > package.json

npm i --save-exact [email protected] -w workspace-a
npm i --save-exact [email protected] -w workspace-a

Note that the overrides do not need to interact with the affected packages at all to cause the bug, merely being there is enough.

@thislooksfun thislooksfun changed the title [BUG] Updating a dependency in a workspace duplicates the package [BUG] Updating a dependency in a workspace with overrides duplicates the package Nov 15, 2023
@thislooksfun
Copy link
Author

thislooksfun commented Nov 15, 2023

To be clear, the above script will "unhoist" abbrev (move it down into the package), but it will not duplicate it. If you also run npm i --save-exact [email protected] -w workspace-a before updating to [email protected] then it will be duplicated. Performing the same series of actions with out an overrides object will correctly update <root>/node_modules/abbrev in-place.

Examples

Update abbrev in-place (standalone)

Script:

npm init -y
npm init -y -w workspace-a

npm i --save-exact [email protected] -w workspace-a
npm i --save-exact [email protected] -w workspace-a

Result:

{
  "name": "npm-repro",
  "version": "1.0.0",
  "lockfileVersion": 3,
  "requires": true,
  "packages": {
    "": {
      "name": "npm-repro",
      "version": "1.0.0",
      "license": "ISC",
      "workspaces": [
        "workspace-a"
      ]
    },
    "node_modules/abbrev": {
      "version": "1.1.1",
      "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz",
      "integrity": "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q=="
    },
    "node_modules/workspace-a": {
      "resolved": "workspace-a",
      "link": true
    },
    "workspace-a": {
      "version": "1.0.0",
      "license": "ISC",
      "dependencies": {
        "abbrev": "1.1.1"
      }
    }
  }
}
Update abbrev in-place (external dependency)

Script:

npm init -y
npm init -y -w workspace-a

npm i --save-exact [email protected] -w workspace-a
# [email protected] depends on abbrev@^1.0.0
npm i --save-exact [email protected] -w workspace-a
npm i --save-exact [email protected] -w workspace-a

Result:

{
  "name": "npm-repro",
  "version": "1.0.0",
  "lockfileVersion": 3,
  "requires": true,
  "packages": {
    "": {
      "name": "npm-repro",
      "version": "1.0.0",
      "license": "ISC",
      "workspaces": [
        "workspace-a"
      ]
    },
    "node_modules/abbrev": {
      "version": "1.1.1",
      "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz",
      "integrity": "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q=="
    },
    "node_modules/nopt": {
      "version": "6.0.0",
      "resolved": "https://registry.npmjs.org/nopt/-/nopt-6.0.0.tgz",
      "integrity": "sha512-ZwLpbTgdhuZUnZzjd7nb1ZV+4DoiC6/sfiVKok72ym/4Tlf+DFdlHYmT2JPmcNNWV6Pi3SDf1kT+A4r9RTuT9g==",
      "dependencies": {
        "abbrev": "^1.0.0"
      },
      "bin": {
        "nopt": "bin/nopt.js"
      },
      "engines": {
        "node": "^12.13.0 || ^14.15.0 || >=16.0.0"
      }
    },
    "node_modules/workspace-a": {
      "resolved": "workspace-a",
      "link": true
    },
    "workspace-a": {
      "version": "1.0.0",
      "license": "ISC",
      "dependencies": {
        "abbrev": "1.1.1",
        "nopt": "6.0.0"
      }
    }
  }
}
Move abbrev into package

Script:

npm init -y
npm init -y -w workspace-a

# Add an "overrides" section to the root package.json
pkg=$(jq '.overrides |= {"ava": "5.3.1"}' package.json)
echo "$pkg" > package.json

npm i --save-exact [email protected] -w workspace-a
npm i --save-exact [email protected] -w workspace-a

Result:

{
  "name": "npm-repro",
  "version": "1.0.0",
  "lockfileVersion": 3,
  "requires": true,
  "packages": {
    "": {
      "name": "npm-repro",
      "version": "1.0.0",
      "license": "ISC",
      "workspaces": [
        "workspace-a"
      ]
    },
    "node_modules/workspace-a": {
      "resolved": "workspace-a",
      "link": true
    },
    "workspace-a": {
      "version": "1.0.0",
      "license": "ISC",
      "dependencies": {
        "abbrev": "1.1.1"
      }
    },
    "workspace-a/node_modules/abbrev": {
      "version": "1.1.1",
      "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz",
      "integrity": "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q=="
    }
  }
}
Duplicate abbrev

Script:

npm init -y
npm init -y -w workspace-a

# Add an "overrides" section to the root package.json
pkg=$(jq '.overrides |= {"ava": "5.3.1"}' package.json)
echo "$pkg" > package.json

npm i --save-exact [email protected] -w workspace-a
npm i --save-exact [email protected] -w workspace-a
npm i --save-exact [email protected] -w workspace-a

Result:

{
  "name": "npm-repro",
  "version": "1.0.0",
  "lockfileVersion": 3,
  "requires": true,
  "packages": {
    "": {
      "name": "npm-repro",
      "version": "1.0.0",
      "license": "ISC",
      "workspaces": [
        "workspace-a"
      ]
    },
    "node_modules/abbrev": {
      "version": "1.1.0",
      "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.0.tgz",
      "integrity": "sha512-c92Vmq5hfBgXyoUaHqF8P5+7THGjvxAlB64tm3PiFSAcDww34ndmrlSOd3AUaBZoutDwX0dHz9nUUFoD1jEw0Q=="
    },
    "node_modules/nopt": {
      "version": "6.0.0",
      "resolved": "https://registry.npmjs.org/nopt/-/nopt-6.0.0.tgz",
      "integrity": "sha512-ZwLpbTgdhuZUnZzjd7nb1ZV+4DoiC6/sfiVKok72ym/4Tlf+DFdlHYmT2JPmcNNWV6Pi3SDf1kT+A4r9RTuT9g==",
      "dependencies": {
        "abbrev": "^1.0.0"
      },
      "bin": {
        "nopt": "bin/nopt.js"
      },
      "engines": {
        "node": "^12.13.0 || ^14.15.0 || >=16.0.0"
      }
    },
    "node_modules/workspace-a": {
      "resolved": "workspace-a",
      "link": true
    },
    "workspace-a": {
      "version": "1.0.0",
      "license": "ISC",
      "dependencies": {
        "abbrev": "1.1.1",
        "nopt": "6.0.0"
      }
    },
    "workspace-a/node_modules/abbrev": {
      "version": "1.1.1",
      "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz",
      "integrity": "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q=="
    }
  }
}

Note: These scripts use the jq cli tool for some minor json editing, but those steps can be done by hand.

@wraithgar wraithgar added Priority 2 secondary priority issue and removed Needs Triage needs review for next steps labels Nov 28, 2023
@wraithgar
Copy link
Member

I still think this isn't a bug, just an inefficiency. The tree is still valid, and if you run npm install npm then builds the correct tree.

This is in the grey area of npm not trying to calculate the most efficient tree on every operation. There is no need, and it is ultimately an impossible task in some cases. It builds a correct tree.

The fact that the presence of overrides changes the behavior is definitely odd, and may be worth looking into if anyone has the time. This isn't something the npm team is going to prioritize, and I would be surprised if anyone in the community wanted to pick this up. You're welcome to dig into it if you'd like. I'll reopen this as Priority 2 and we'll see if anyone wants to look into it.

@wraithgar wraithgar reopened this Nov 28, 2023
@thislooksfun
Copy link
Author

I still think this is a bug. It's not uncommon for large applications to need to reach into dependencies of dependencies, and when there are multiple versions of that subdependency installed things can go weird; instanceof checks start failing (especially common when checking error types), Symbols don't line up, and code just generally breaks in weird ways. Yes there are typically workarounds for those cases, but it's still something that should just work out of the box.

@ljharb
Copy link
Contributor

ljharb commented Feb 10, 2024

The only way to guarantee one version of a dep is if everything that needs it declares it as a peer dependency - thus, react, babel, webpack, eslint, etc, because it's only when identity matters that you'd need to ensure one copy of a dep.

What dep do you need unduplicated but that isn't commonly declared as a peer dep?

@thislooksfun
Copy link
Author

I filed this issue after running into a problem with having multiple versions of the bson package installed in a monorepo (it's required by mongoose and has a Symbol that doesn't use Symbol.for, which was breaking comparisons), but unfortunately I don't remember exactly how I got into that situation and I'm having trouble recreating it from scratch. If/when I run into it again I'll update this issue.

@ljharb
Copy link
Contributor

ljharb commented Feb 11, 2024

in that case, that's a bug in either bson or mongoose - iow, either it needs to be a peer dep, or it needs to use a global symbol.

@owlstronaut
Copy link
Contributor

Confirmed that this is fixed in 11.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Priority 2 secondary priority issue Release 10.x
Projects
None yet
Development

No branches or pull requests

4 participants