| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
There was a problem hiding this comment.
![]()
Sorry, something went wrong.
|
FYI: Right now, this PR fixes only npm ci after npm install. package-lock.json file still has node_modules/node-fetch version 2.7.0 instead of 2.6.11 after first call to npm install. I will try to fix this too. |
Sorry, something went wrong.
…ptional When a dependency is placed (OK) and creates an invalid peerOptional edge with an already-processed node, re-queue that node for re-resolution. This handles the case where the peerOptional holder is processed before the dep exists in the tree (due to alphabetical processing order), and the dep is later placed by another path with a version that doesn't satisfy the peer spec. Also adds npm install tests for issue npm#8726 covering both the existing-lockfile and fresh-install (no lockfile) scenarios. Refs: npm#8726 Co-authored-by: Cursor <cursoragent@cursor.com>
|
Now npm install produces a correct package-lock.json file. |
Sorry, something went wrong.
There was a problem hiding this comment.
Could you unwrap the multi-line comments so each sentence/paragraph is on a single line? We avoid hard-wrapping prose in comments for accessibility as it doesn't play well with screen readers (we've been slowly converting the codebase to that anyway, it is still pretty prevalent)
Sorry, something went wrong.
Sorry, something went wrong.
|
@Saibamen looks like it needs 2 more lines of code coverage from tests as well |
Sorry, something went wrong.
|
sounds like this should also resolve #6787, so @Saibamen might want to put that in the PR title body so that gets linked and closed when this is landed. Thanks for working on this ❤️ |
Sorry, something went wrong.
Linking doesn't work in PR title - you need to have it inside PR description (first comment). |
Sorry, something went wrong.
|
yup I mistyped - I meant "body" rather than "title", and for you to do a "Fixes #xyz" for each issue 🙂 |
Sorry, something went wrong.
…from the !edge.valid peerOptional check. This branch was unreachable because add: [...] changes peerOptional edges to regular dependency edges before they're added to #explicitRequests, so a peerOptional edge can never be both explicitly requested and still typed as peerOptional. The simplified condition edge.type !== 'peerOptional' || this.options.save !== false still correctly handles all reachable cases.
…=false This test verifies that invalid peerOptional edges are not treated as problems when the save option is set to false, mimicking the behavior of `npm ci`. It ensures that the dependency tree is built correctly without raising issues for optional peer dependencies that do not meet the specified version requirements.
|
@owlstronaut Please run GH Actions again - I have 100% CC on my local git repo |
Sorry, something went wrong.
|
hope this bc break will be fixed in the next release |
Sorry, something went wrong.
There was a problem hiding this comment.
@Saibamen Thank you for working on this, it is an incredible contribution. I'm very pleased to accept this into the codebase ❤️
Sorry, something went wrong.
|
Also fixes #4859, #5854 and #8674 (see my comments there). And probably 3 more open issues from #6787, but they don't have repro steps with package.json file (at least not in the first comment). I will ping issue authors when 11.10.1 #8987 will be released. |
Sorry, something went wrong.
|
In my case peerDependenciesMeta[optional] seems still ignored and all optional peer deps of dependent package still installed by default Sorry, seems it's Gitlab Private Registry issue with metadata |
Sorry, something went wrong.
|
Thanks a lot for all the effort on this fix, I see it was added to Node 25 which eventually will flow into Node 26. Is the plan to also include this fix in Node 24? thanks in advance |
Sorry, something went wrong.
|
@owlstronaut: Can you add this to NodeJS LTS 24? |
Sorry, something went wrong.
|
@rvillane: In latest NodeJS 24.14.1, NPM was updated to version 11.11.0 |
Sorry, something went wrong.
) `npm ci` failed on a clean checkout with EUSAGE, naming a version that appears nowhere in package-lock.json: npm error Missing: esbuild@0.28.2 from lock file ... (+26 @esbuild/* platform packages, all 0.28.2) while `npm install --package-lock-only --dry-run` reported "up to date". The lockfile is not at fault and is unchanged here. `vitest`'s nested `vite@8.2.2` declares esbuild as an *optional* peerDependency (`^0.27.0 || ^0.28.0`, `peerDependenciesMeta.esbuild.optional = true`), and nothing in the lock satisfies that edge at that location — which is correct, because an unsatisfied optional peer should be skipped. Older npm instead resolves it against the registry (arborist's `loadPeerSet`), picks up whatever is current, then rejects the lock as out of sync. That is also the install-vs-ci contradiction: the two commands build the tree by different paths and disagree about optional peers. Fixed upstream by npm/cli#8981 (npm 11.10.1) and npm/cli#9083 (npm 11.11.1). Node 24.15.0 is the first 24.x bundling npm >= 11.11.1. CI was never affected: `.nvmrc` is bare `24`, so setup-node floats to v24.19.0 / npm 11.17.0. The failure only reached environments sitting on an old-but-in-range Node 24, which `engines.node: ">=24"` permitted. Regenerating the lockfile under npm 11.17.0 reproduces it byte for byte; regenerating under an affected npm instead commits esbuild 0.28.2 and 26 platform packages that CI does not install. `.nvmrc` deliberately stays bare `24` — the repo pins by major, and scripts/check-node-version.sh compares majors only, so it still passes. Also corrects two references the new floor invalidated: the Dockerfile comment that quoted "node >=24" (now states the floor without repeating a value that can drift) and the docs/ops.md local-dev requirement.
O lock foi gerado no Windows com npm 11.6.2, que sofre de um bug conhecido (npm/cli#8981, corrigido na 11.10.1): ao pular dependencias opcionais de outra plataforma, ele mantem as referencias a elas no lock mas descarta as entradas dos pacotes. Na pratica faltavam @emnapi/core e @emnapi/runtime, exigidos por @img/sharp-wasm32 e @napi-rs/wasm-runtime. No Windows nada quebra, porque npm install tolera a lacuna; no runner Linux e no build da imagem Docker, npm ci recusa o lock por estar fora de sincronia com o package.json. Lock regerado com npm 12, que resolve o grafo completo. Continua no formato lockfileVersion 3 e nenhuma versao de dependencia direta mudou. Co-authored-by: Cursor <cursoragent@cursor.com>
| Back | FazBrowse Home | New Git URL |
Fixes #4859, fixes #5854, fixes #6787, fixes #8674, fixes #8726, fixes #8767, fixes #8777, fixes #8805
Tested on Windows & NodeJS v24.12.0 with package.json from #8726 (comment):
{ "name": "testcase", "version": "1.0.0", "devDependencies": { "addons-linter": "6.13.0", "htmlhint": "1.1.4" } }