Repository navigation
deps(npm): consolidate the 25 Dependabot PRs into one (#1218-#1242) - #1244
Alexandre Zollinger Chohfi (azchohfi) merged 8 commits into
Conversation
Enabling Dependabot for the npm projects produced 25 separate PRs in its
first run, one per package per directory. This supersedes all of them.
Taken (15):
src/vscode-reactor @types/vscode ^1.136.0 -> ^1.137.0
samples/apps/headtrax/service @faker-js/faker ^10.5.0 -> ^10.6.0
tests/startup_perf/BlankRNW react 19.1.1 -> 19.3.0
@types/react ^19.1.1 -> ^19.3.0
all three RN harnesses @react-native-community/cli 20.1.3 -> 20.2.0
@react-native/eslint-config 0.82.0 -> 0.87.1
@react-native/typescript-config 0.82.0 -> 0.87.1
prettier 2.8.8 -> 3.9.6
Held (10), each blocked by a measured upstream constraint rather than a
judgement call, and recorded as an `ignore` entry so Dependabot stops
re-proposing them:
eslint 8 -> 10 @react-native/eslint-config@0.87.1 declares
`peer eslint@"^8.0.0 || ^9.0.0"`, so npm fails with
ERESOLVE; the harnesses also configure lint through
.eslintrc.js, the format eslint 10 removed.
@babel/preset-env and @babel/preset-env@8.0.5 declares
@babel/runtime 7 -> 8 `peer @babel/core@"^8.0.0"`, but
@react-native/babel-preset@0.82.0 depends on
`@babel/core@^7.25.2`. Babel 8 would put two major
Babel cores in one tree. @babel/core major is
ignored for the same reason.
better-sqlite3 11 -> 13 v13 removed prebuild-install (no install script, no
optionalDependencies, no `binary` field, and zero
prebuilt assets on its GitHub release), so
`gypfile: true` makes every install compile SQLite
from source -- which is what the Node 22 pin in
.github/workflows/node.yml exists to avoid, and it
breaks `npm ci` without a C++ toolchain.
Also adds minor/patch `groups:` rules to both npm ecosystem entries, so a
future run arrives as one PR per directory while majors still get an
individual PR for review.
The @react-native/eslint-config bump raises @typescript-eslint/eslint-plugin
8.59.1 -> 8.70.0, which newly flags `const X = React.memo(function X(...))`
under no-shadow. Both harness components are rewritten as a named declaration
plus `React.memo(XImpl)`, which is semantics-preserving and restores lint
output to exactly the pre-change baseline.
Validation: .github/workflows/node.yml mirrored locally on Node 22 (the
version CI pins) across all six npm projects -- `npm ci`, every declared check
script, and the committed-build-output freshness assertion: 21/21 pass, with
lint warnings identical to baseline (BlankRNW 0, StocksGrid 2, VirtualList 7).
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 504b5020-f687-4a59-9212-aa2c90c4b7ec
Alexandre Zollinger Chohfi (azchohfi)
added
the
dependencies
📦 Build metricsArtifact sizes for Packages (compressed .nupkg)
Assemblies in Microsoft.UI.Reactor
Assemblies in Microsoft.UI.Reactor.Advanced
Assemblies in Microsoft.UI.Reactor.Devtools
No size change beyond the noise floor. ✅ ✅ smaller / |
There was a problem hiding this comment.
🟡 Changes recommended
Open review comments remain on dependency rationale and Node engine compatibility.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Consolidates 25 Dependabot npm updates across the VS Code extension, HeadTrax service, and React Native harnesses.
Changes:
- Updates supported dependencies and lockfiles.
- Groups routine Dependabot updates and records selected major-version holds.
- Refactors memoized renderers for updated lint rules.
File summaries
| File | Summary |
|---|---|
tests/stress_perf_rn/VirtualList/package.json |
Updates React Native tooling and Prettier; Node engine compatibility requires adjustment. |
tests/stress_perf_rn/VirtualList/package-lock.json |
Refreshes locked dependencies. |
tests/stress_perf_rn/VirtualList/App.tsx |
Refactors the memoized row renderer. |
tests/stress_perf_rn/StocksGrid/package.json |
Updates React Native tooling and Prettier; Node engine compatibility requires adjustment. |
tests/stress_perf_rn/StocksGrid/App.tsx |
Refactors the memoized cell renderer. |
tests/startup_perf/BlankRNW/package.json |
Updates React Native tooling and React; Node engine compatibility requires adjustment. |
src/vscode-reactor/package.json |
Updates VS Code type definitions. |
src/vscode-reactor/package-lock.json |
Refreshes the extension lockfile. |
samples/apps/headtrax/service/package.json |
Updates Faker. |
samples/apps/headtrax/service/package-lock.json |
Refreshes the HeadTrax lockfile. |
.github/dependabot.yml |
Adds dependency grouping and major-version holds; several rationale comments require correction. |
Review details
Files not reviewed (5)
- samples/apps/headtrax/service/package-lock.json: Generated file
- src/vscode-reactor/package-lock.json: Generated file
- tests/startup_perf/BlankRNW/package-lock.json: Generated file
- tests/stress_perf_rn/StocksGrid/package-lock.json: Generated file
- tests/stress_perf_rn/VirtualList/package-lock.json: Generated file
Suppressed comments (3)
.github/dependabot.yml:184
- The explanation here conflates two independent constraints: the workflow's Node 22 pin exists because retained
better-sqlite311.x cannot build on Node 26 and uses a prebuilt binary on Node 22 (.github/workflows/node.yml:128-130), not because it preventsbetter-sqlite313 from compiling. Please state the v13 source-build concern separately so this rationale does not mislead future dependency decisions.
# SQLite from source. That is what the Node 22 pin in
# .github/workflows/node.yml exists to avoid, and it breaks `npm ci` for
# any contributor without a C++ toolchain. Security patches on 11.x still
.github/dependabot.yml:158
- The rationale groups
@babel/runtimeunder apeer @babel/coreconstraint, but@babel/runtime@8.0.5does not declare that peer; that constraint belongs to@babel/preset-env. This makes the ignore policy and the claim that all three packages must move together inaccurate. Please document a runtime-specific compatibility reason with evidence, or allow its major update independently after validating the Metro output.
# @babel/preset-env @babel/preset-env@8.0.5 declares
# @babel/runtime `peer @babel/core@"^8.0.0"`, but
# @babel/core @react-native/babel-preset@0.82.0 *depends* on
# `@babel/core@^7.25.2`. Taking Babel 8 for any one of
# these puts two major Babel cores in one tree and
# breaks the Metro transform. These three Babel
.github/dependabot.yml:181
- This description does not match
better-sqlite3@13.0.3: its package metadata setsgypfile: false, and its loader explicitly looks forprebuilds/<platform>-<arch>.nodebefore falling back to node-gyp; the package file list includesprebuilds/**. Therefore the claims thatgypfile: truemakes every install compile and that the GitHub release asset list proves the npm install path are false. Please base this ignore on verified npm-tarball behavior and the actual Node-engine/toolchain constraint, or remove it if the Node 22 CI environment is supported.
# better-sqlite3 13 removed `prebuild-install` (v11's install script was
# `prebuild-install || node-gyp rebuild`; v13 has no install script, no
# optionalDependencies, no `binary` field, and its GitHub release carries
# zero prebuilt assets), so `gypfile: true` makes every install compile
- Files reviewed: 8/13 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
🧪 Merged coverageCoverage for
No coverage change beyond the noise floor. ✅ ✅ higher / |
There was a problem hiding this comment.
🔵 Needs a closer look
Add the React Native minor/patch pin and HeadTrax Node requirement, and correct the stale CI compatibility comment.
Review details
Files not reviewed (5)
- samples/apps/headtrax/service/package-lock.json: Generated file
- src/vscode-reactor/package-lock.json: Generated file
- tests/startup_perf/BlankRNW/package-lock.json: Generated file
- tests/stress_perf_rn/StocksGrid/package-lock.json: Generated file
- tests/stress_perf_rn/VirtualList/package-lock.json: Generated file
Suppressed comments (3)
.github/dependabot.yml:140
- The
"*"group also matches each harness's directreact-native: "0.82.0". Because the ignore list below blocks only semver-major updates, a future 0.82.x/0.83.x React Native update can be proposed as a routine grouped change, contrary to the documented pin and potentially moving the Metro/native toolchain without the coordinated upgrade this configuration says is required. Add a minor/patch ignore forreact-native(or otherwise make that pin explicit); excluding it from the group alone would still allow the update.
npm-minor-patch:
patterns:
- "*"
update-types:
- "minor"
- "patch"
samples/apps/headtrax/service/package.json:18
- This bump makes the Node-version rationale in
.github/workflows/node.yml:128-130incorrect: it still saysbetter-sqlite3 ^11.7.0is limited by removed V8 APIs and only works via a Node 22 prebuild, while the new13.0.3package declaresnode >=22and usesnode-addon-api. Please update that workflow comment (and retain only any still-applicable reason for pinning CI to Node 22) so future maintainers do not infer a false compatibility constraint from this dependency.
"better-sqlite3": "^13.0.3",
samples/apps/headtrax/service/package.json:18
better-sqlite3@13.0.3now declaresengines.node >=22in the updated lockfile, but this service manifest still has no top-level Node requirement. A developer following the sample'snpm installinstructions on Node 20 can get only a nested dependency warning (and npm may continue), leaving the service's supported runtime undefined even though the native dependency is outside that runtime's contract. Add a rootengines.nodedeclaration so installs and tooling surface the same requirement as the dependency.
"better-sqlite3": "^13.0.3",
- Files reviewed: 8/13 changed files
- Comments generated: 0 new
- Review effort level: Lite
…block Windows Addresses round three of the Copilot review. All three suppressed comments were valid. 1. The react-native pin was documented but not enforced. Every hold in this file is justified by "react-native is pinned at 0.82.0", yet nothing stopped Dependabot from moving it: semver treats 0.82.0 -> 0.83.0 as a *minor*, so the new `npm-minor-patch` group (`patterns: ["*"]`) would have swept a whole React Native generation in as a routine grouped update and silently invalidated the eslint and Babel rationales. react-native, @react-native/babel-preset and @react-native/metro-config now ignore minor as well as major; 0.82.x patches still flow. 2. node.yml's Node 22 comment was made stale by the better-sqlite3 bump. It still claimed "better-sqlite3 ^11.7.0 cannot build on Node 26, which removed the V8 APIs it uses". That constraint left with v13. The comment now states the real reason: 22 is the floor every npm project here declares (better-sqlite3 13 requires >=22; the RN harnesses require ^22.13.0 || ^24.3.0 || >= 26.0.0 via @react-native/eslint-config). 3. The HeadTrax service had no root engines despite now depending on a package that requires Node >=22. Added, and the lockfile root regenerated to match. Windows regression found while validating (3): better-sqlite3 13 keeps a binding.gyp, so npm's implicit `node-gyp rebuild` runs even though the package ships prebuilds. On Windows that generates build/binding.sln inside node_modules and hands it to MSBuild, which walks *up* for Directory.Build.props, finds the repo-root one, and tries to build src/Reactor.Analyzers -- failing with NETSDK1004 and taking `npm ci` down with it. CI never saw this because it runs on Linux. An empty Directory.Build.props/.targets pair in samples/apps/headtrax/service stops that traversal. Nothing in that folder is an MSBuild project (the HeadTrax WinUI app is in ../HeadTrax/ and is unaffected), so it costs nothing. Measured before and after on Windows: `npm ci` went from exit 1 to exit 0 in 54s, with `build/Release/better_sqlite3.node` absent and `prebuilds/win32-x64.node` present -- the prebuild is used, nothing compiles -- and `npm run smoke` passes. Full suite: 21/21 on Node 22, lint output unchanged from baseline. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 504b5020-f687-4a59-9212-aa2c90c4b7ec
|
All three suppressed comments from round 3 were valid. Fixed in 98e08c3. The
|
| before | after | |
|---|---|---|
npm ci |
exit 1 | exit 0 (54s) |
build/Release/better_sqlite3.node |
— | absent (nothing compiled) |
prebuilds/win32-x64.node |
— | present (prebuild used) |
npm run smoke |
fails, ERR_MODULE_NOT_FOUND |
passes |
That also closes the loop on the earlier better-sqlite3 argument: v13 really does use its shipped prebuild and compiles nothing — the install failure was MSBuild traversal, not the package.
Full suite is back to 21/21 on Node 22, lint output unchanged from baseline.
There was a problem hiding this comment.
🔵 Needs a closer look
Align the HeadTrax Node engine range and add validation for the Windows native-build workaround.
Review details
Files not reviewed (5)
- samples/apps/headtrax/service/package-lock.json: Generated file
- src/vscode-reactor/package-lock.json: Generated file
- tests/startup_perf/BlankRNW/package-lock.json: Generated file
- tests/stress_perf_rn/StocksGrid/package-lock.json: Generated file
- tests/stress_perf_rn/VirtualList/package-lock.json: Generated file
Suppressed comments (4)
.github/dependabot.yml:127
- This comment says the first run opened 24 PRs, but the PR description says it supersedes #1218 through #1242, which is 25 PRs. Please keep the historical count consistent with the consolidation this rule documents.
# Ungrouped, the first run of this entry opened 24 PRs in four minutes — one
.github/workflows/node.yml:135
better-sqlite3still has an install script (package-lock.jsonrecordshasInstallScript: true), and the new service props explicitly say that node-gyp can run when no prebuild is available. Saying it “no longer compiles on install” is therefore inaccurate and obscures why the Windows MSBuild isolation is needed; describe the prebuilt path as the normal case and the source-build fallback accurately.
# under prebuilds/ and no longer compiles on install.)
samples/apps/headtrax/service/Directory.Build.props:11
- This workaround is only exercised when Windows has to build
better-sqlite3from source, but the npm workflow's matrix runs onubuntu-latest, wherenpm ciuses the Linux prebuild. Consequently CI cannot validate that these empty props/targets actually stop the MSBuild traversal; add a Windowsnpm cicheck for this service or an equivalent deterministic probe before relying on this fix.
better-sqlite3 13 ships prebuilt binaries but keeps a binding.gyp, so npm's
implicit `node-gyp rebuild` still runs on install. On Windows that generates
build/binding.sln under node_modules/better-sqlite3 and hands it to MSBuild.
MSBuild then walks up looking for Directory.Build.props, finds the repo-root
one, and applies Reactor's whole C# build, which fails with NETSDK1004
against src/Reactor.Analyzers because nothing has been restored. `npm ci` in
this folder dies as a result.
samples/apps/headtrax/service/package.json:23
- This manifest advertises Node 22.0–22.12 (and Node 23.0–23.4) as supported, but the bumped
@faker-js/faker@10.6.0used by the declaredgenerate*andsmokescripts requires^22.13.0 || ^23.5.0 || >=24.0.0(see its locked engine range). Align this project engine with the intersection of Faker andbetter-sqlite3, otherwise npm reports an engine mismatch for runtimes this manifest currently claims to support.
"node": ">=22"
- Files reviewed: 11/16 changed files
- Comments generated: 0 new
- Review effort level: Lite
Addresses round four of the Copilot review. The substantive one: the Windows MSBuild workaround added in the previous commit had no CI coverage. node.yml's matrix was ubuntu-only, so the very regression it fixes -- `npm ci` for the HeadTrax service dying because MSBuild walked up to the repo-root Directory.Build.props -- is invisible to CI by construction, and a future change could silently undo the fix. The check job now runs on both ubuntu-latest and windows-latest. That is also the right default for a Windows-first repo: Linux alone cannot see native-module install behaviour on the platform every contributor actually uses. The two bash steps get an explicit `shell: bash`, since the Windows runner defaults to pwsh where `set -euo pipefail` is not valid. HeadTrax engines were still over-claiming. `>=22` advertises 22.0-22.12, but @faker-js/faker@10.6.0 -- used by the declared `generate*` and `smoke` scripts -- requires `^20.19.0 || ^22.13.0 || ^23.5.0 || >=24.0.0`. Intersected with better-sqlite3 13's `>=22`, the honest range is `^22.13.0 || ^23.5.0 || >=24.0.0`. Lockfile root regenerated to match. node.yml's better-sqlite3 note said v13 "no longer compiles on install", which is wrong: the lockfile records `hasInstallScript: true` and npm's implicit `node-gyp rebuild` does run. What actually happens is that binding.gyp sets every target to `type: none` when a prebuild matches the host, so the normal path compiles nothing while a source build remains the fallback on a platform with no prebuild. Reworded to say that. The "24 PRs" figure in dependabot.yml was correct -- #1219-#1242 came from that entry, and #1218 came from the separate /src/vscode-reactor entry -- but it read as contradicting the PR description's 25. Spelled the split out. Validation: 21/21 on Node 22 locally, lint output unchanged from baseline. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 504b5020-f687-4a59-9212-aa2c90c4b7ec
|
Round 4 — all four addressed in a9d3986. The CI-coverage one was the good catch. The Windows workaround had no CI coverage — fixed properlyYou're right, and this was the real gap. The The check job now runs on both
|
There was a problem hiding this comment.
🔵 Needs a closer look
CI should test the declared Node.js 22.13.0 minimum rather than the latest Node 22 release.
Review details
Files not reviewed (5)
- samples/apps/headtrax/service/package-lock.json: Generated file
- src/vscode-reactor/package-lock.json: Generated file
- tests/startup_perf/BlankRNW/package-lock.json: Generated file
- tests/stress_perf_rn/StocksGrid/package-lock.json: Generated file
- tests/stress_perf_rn/VirtualList/package-lock.json: Generated file
Suppressed comments (1)
.github/workflows/node.yml:154
node-version: '22'resolves the latest available 22.x release; it does not exercise the22.13.0lower bound declared by the React Native harnesses. The current job can therefore pass code that relies on APIs added after the supported floor, despite the surrounding comment saying this tests the minimum. Pin the exact floor (or revise the stated compatibility goal).
node-version: '22'
- Files reviewed: 11/16 changed files
- Comments generated: 0 new
- Review effort level: Lite
The Windows matrix added in the previous commit paid for itself on its first
run: every ubuntu-latest job passed and
`samples/apps/headtrax/service (windows-latest)` failed outright.
gyp ERR! find VS Could not find any Visual Studio installation to use
gyp ERR! configure error
npm error gyp ERR! not ok
This settles an argument that went back and forth twice. v13 genuinely does
ship prebuilds, and its binding.gyp genuinely does set every target to
`type: none` when one matches the host -- both of those corrections were
right. What neither of us accounted for is *ordering*: node-gyp evaluates
binding.gyp only after its `configure` step, and configure requires a Visual
Studio it can detect. So the prebuild short-circuit is unreachable on a
machine without one, and GitHub's windows-latest runner does not have one that
node-gyp 11.5.0 accepts. v11 never hits this because its
`install: prebuild-install || node-gyp rebuild` fetches a prebuilt binary and
only falls through to node-gyp when no prebuild matches.
My earlier local failure was the same mechanism; I misread it as a quirk of
my Visual Studio 2026 install and worked around the wrong layer.
So better-sqlite3 stays on ^11.7.0 and the ignore entry comes back, this time
citing the CI job rather than an argument. The MSBuild traversal stoppers
(Directory.Build.props/.targets in the service folder) are removed with it:
with v11 node-gyp never runs, so they guard nothing and their comment
describes a situation that can no longer occur.
Other changes, from round five:
- node-version is pinned to 22.13.0 rather than '22'. `'22'` resolves to the
newest 22.x, so it never exercised the floor the projects declare, despite
the comment claiming exactly that. 22.13.0 is the lowest version satisfying
both the RN harnesses (`^22.13.0 || ^24.3.0 || >= 26.0.0`) and the HeadTrax
service.
- HeadTrax engines are now `^20.19.0 || ^22.13.0 || ^23.5.0`: the intersection
of @faker-js/faker@10.6.0's declared range with the ABIs better-sqlite3
11.10.0 actually publishes prebuilds for (108/115/127/131 = Node
18/20/22/23). Above that range it would compile, which is the failure this
commit exists to avoid.
Validation: 21/21 on Node 22.13.0 exactly, the version CI now pins.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 504b5020-f687-4a59-9212-aa2c90c4b7ec
|
Round 5 addressed in 5bfb9b5 — and the Windows matrix you asked for in round 4 immediately justified itself. The Windows matrix caught a real regression on its first runEvery This settles the What neither of us accounted for is ordering. node-gyp evaluates My earlier local failure was the same mechanism; I misread it as a quirk of my VS 2026 install and patched the wrong layer. So
|
There was a problem hiding this comment.
🔵 Needs a closer look
The broad dependency, lockfile, and cross-platform CI changes warrant final human review.
Review details
Files not reviewed (5)
- samples/apps/headtrax/service/package-lock.json: Generated file
- src/vscode-reactor/package-lock.json: Generated file
- tests/startup_perf/BlankRNW/package-lock.json: Generated file
- tests/stress_perf_rn/StocksGrid/package-lock.json: Generated file
- tests/stress_perf_rn/VirtualList/package-lock.json: Generated file
Suppressed comments (1)
.github/workflows/node.yml:139
- The statement that Node 22.13.0 is “the floor every npm project here declares” is not true:
src/vscode-reactor/package.jsondeclares onlyengines.vscode, andtools/figma-plugin/package.jsondeclares no Node engine. The pin is justified by the React Native harnesses and HeadTrax service, so scope the wording to those constrained projects to avoid misleading future maintainers.
# Node 22.13.0 exactly, not '22': that is the floor every npm project
# here declares, and pinning the floor is what makes the declaration
# meaningful. `node-version: '22'` resolves to the newest 22.x, which
- Files reviewed: 9/14 changed files
- Comments generated: 0 new
- Review effort level: Lite
…n engine Round six of the Copilot review. The comment claimed 22.13.0 is 'the floor every npm project here declares', which is not true: only four of the six do. src/vscode-reactor declares engines.vscode and no Node engine, and tools/figma-plugin declares no engine at all, so neither constrains the pin. Scoped the wording to the React Native harnesses and the HeadTrax service, which are the projects the pin is actually derived from. Comment-only; no behaviour change. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 504b5020-f687-4a59-9212-aa2c90c4b7ec
|
Round 6 fixed in 04e3484 — comment-only, no behaviour change. You're right, and I verified it rather than taking it on faith. Only four of the six projects declare a Node engine:
So "the floor every npm project here declares" overstated it. The comment now scopes the rationale to the four that constrain the pin, and explicitly notes that the other two don't — they just run on whatever it selects. All 12 |
There was a problem hiding this comment.
🔵 Needs a closer look
The broad dependency, lockfile, and CI changes span multiple projects and warrant final human review.
Review details
Files not reviewed (5)
- samples/apps/headtrax/service/package-lock.json: Generated file
- src/vscode-reactor/package-lock.json: Generated file
- tests/startup_perf/BlankRNW/package-lock.json: Generated file
- tests/stress_perf_rn/StocksGrid/package-lock.json: Generated file
- tests/stress_perf_rn/VirtualList/package-lock.json: Generated file
Suppressed comments (2)
.github/dependabot.yml:215
- This explanation says the GitHub runner has no Visual Studio, but the repository documents VS 2026/18.x on the RNW setup at
tests/stress_perf_rn/README.md:50-55. The observed failure is that this node-gyp/RNW tooling cannot detect the installed VS 18.x toolset; please state that accurately so the dependency hold is not justified by a false prerequisite claim.
# configure needs a Visual Studio it can detect. On GitHub's
# windows-latest runner there is none, so `npm ci` dies at
# "find VS ... Could not find any Visual Studio installation to use"
.github/workflows/node.yml:124
- This explanation says the GitHub runner has no Visual Studio, but the repository documents VS 2026/18.x on the RNW setup at
tests/stress_perf_rn/README.md:50-55. The observed failure is that this node-gyp/RNW tooling cannot detect the installed VS 18.x toolset; please state that accurately so the dependency hold is not justified by a false prerequisite claim.
# rebuild` then needs a Visual Studio the runner does not have. An
- Files reviewed: 9/14 changed files
- Comments generated: 0 new
- Review effort level: Lite
… a missing VS
Round seven of the Copilot review, and it was right. Both comments said the
windows-latest runner has no Visual Studio. The CI log says otherwise:
find VS unknown version "undefined" found at
"C:\Program Files\Microsoft Visual Studio\18\Enterprise"
find VS could not find a version of Visual Studio 2017 or newer to use
Visual Studio 2026 is installed; node-gyp 11.5.0 (bundled with npm 10.9.x)
cannot identify it. That is the same VS 18.x detection gap
tests/stress_perf_rn/README.md already documents for the react-native-windows
CLI, and node-gyp 13 does detect VS 2026 -- so the hold unblocks itself when
npm ships a newer bundled node-gyp, which is worth saying because it tells a
future maintainer when to revisit.
Justifying a dependency hold with a false prerequisite claim would have been
the worse outcome even though the hold itself is correct.
Comment-only; no behaviour change.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 504b5020-f687-4a59-9212-aa2c90c4b7ec
|
Round 7 fixed in 15d2bcc — comment-only, and you were right again. I checked the CI log rather than taking it on faith, and it contradicts what I wrote: Visual Studio 2026 is installed on the That's the same VS 18.x detection gap Worth adding, since it tells a future maintainer when to revisit: node-gyp 13 does detect it — I measured The hold itself is unchanged and still correct — but justifying it with a false prerequisite claim would have been the worse outcome, so thanks for catching it. |
There was a problem hiding this comment.
🔵 Needs a closer look
The broad dependency, lockfile, and cross-platform CI changes warrant final human review.
Review details
Files not reviewed (5)
- samples/apps/headtrax/service/package-lock.json: Generated file
- src/vscode-reactor/package-lock.json: Generated file
- tests/startup_perf/BlankRNW/package-lock.json: Generated file
- tests/stress_perf_rn/StocksGrid/package-lock.json: Generated file
- tests/stress_perf_rn/VirtualList/package-lock.json: Generated file
- Files reviewed: 9/14 changed files
- Comments generated: 0 new
- Review effort level: Lite
…tory (#1252) * deps: group Dependabot majors by package family, not one PR per directory #1244 grouped routine minor/patch updates into a single PR, which worked -- the next run collapsed 29 of its 37 pending updates into one PR (#1245). But majors were deliberately left ungrouped, and Dependabot reads "ungrouped" as one PR per package *per directory*. Adopting TypeScript 7 therefore arrived as four identical PRs (#1246, #1247, #1250, #1251) for a single decision, and jest 30 as two more (#1248, #1249). "Majors get their own PR" was meant per breaking change, not per directory. Majors are now grouped by package family -- typescript, jest, react, @react-native-community/cli, prettier -- so one breaking change is one PR spanning every directory it touches, and two unrelated breaking changes still never share a diff. The families are the ones that genuinely move together, the same rule the nuget entry already applies to the roslyn groups. Verified rather than assumed. A simulator for Dependabot's group resolution (patterns/update-types/ignore, first match wins, grouped PRs span directories) was run against the 37 real pending updates scraped from #1245-#1251: old config -> 9 PRs, new config -> 3 PRs As a control the same simulator was run against the pre-change config on main, where it predicts 9 against the 7 Dependabot actually opened -- the difference is entirely Dependabot bundling a package with its @types (jest + @types/jest in two directories = 2 PRs, not 4), which the simulator does not model. The new config produces no ungrouped PRs at all, so that nuance cannot alter the 3. Separately, this holds @rnx-kit/jest-preset, which the simulation surfaced as mis-sorted. It is required directly by both harnesses' jest.config.windows.js, and 0.3.3 adds `peer @react-native/jest-preset: ">=0.85"` where 0.1.x declared no such peer -- a react-native 0.85 generation package, while these harnesses pin react-native at 0.82.x and are blocked from moving. Being a 0.x package, that arrives as a *minor*, so it was sitting inside the routine batch in #1245. Measured: `npm install` resolves 0.3.3 and silently omits the unsatisfiable peer from the tree rather than failing, which is why it is suppressed here instead of left to surface downstream. No code or lockfile changes; this only affects what Dependabot opens next run. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 504b5020-f687-4a59-9212-aa2c90c4b7ec * deps: sort @rnx-kit/jest-preset by its real (0.x minor) breaking axis Copilot review on #1252. The `jest-major` group listed @rnx-kit/jest-preset but is scoped to `update-types: [major]`, and that package is 0.x -- its breaking axis is semver-MINOR. So the listing was inert: once the react-native upgrade lifts the ignore, a breaking 0.3 -> 0.4 would have matched the earlier catch-all `npm-minor-patch` group instead, which is precisely how the incompatible 0.1 -> 0.3 bump reached #1245 in the first place. Given it its own group, `update-types: [minor, major]`, placed before npm-minor-patch so first-match-wins sorts it there, and removed it from jest-major where it could never have matched. Verified with the group simulator in two scenarios: - today (ignore active): unchanged, still 3 PRs for the 37 pending updates - ignore lifted, synthetic 0.3.3 -> 0.4.0: lands in rnx-kit-jest-preset-breaking, while a routine jest 30.5.1 -> 30.5.2 patch still batches into npm-minor-patch Config comments only; no behaviour change while the ignore stands. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 504b5020-f687-4a59-9212-aa2c90c4b7ec --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 504b5020-f687-4a59-9212-aa2c90c4b7ec
Enabling Dependabot for the npm projects produced 25 separate PRs (#1218–#1242) in its first run — one per package per directory. This supersedes all 25.
Each bump was applied, then kept or held based on what upstream actually allows, verified against real package metadata and CI.
Taken — 15
src/vscode-reactor@types/vscode^1.136.0→^1.137.0samples/apps/headtrax/service@faker-js/faker^10.5.0→^10.6.0tests/startup_perf/BlankRNWreact/@types/react19.1.1→19.3.0@react-native-community/cli20.1.3→20.2.0@react-native/eslint-config0.82.0→0.87.1@react-native/typescript-config0.82.0→0.87.1prettier2.8.8→3.9.6Held — 10
Each hold is an upstream constraint, recorded as an
ignoreentry so Dependabot stops re-proposing it. Onlyversion-update:semver-majoris blocked, so security and minor/patch updates still flow.eslint8 → 10 (deps(npm): bump eslint from 8.57.1 to 10.10.0 in /tests/startup_perf/BlankRNW #1225, deps(npm): bump eslint from 8.57.1 to 10.10.0 in /tests/stress_perf_rn/StocksGrid #1231, deps(npm): bump eslint from 8.57.1 to 10.10.0 in /tests/stress_perf_rn/VirtualList #1236) —@react-native/eslint-config@0.87.1declarespeer eslint@"^8.0.0 || ^9.0.0", sonpm installfails withERESOLVE. The harnesses also configure lint through.eslintrc.js, the format eslint 10 removed.@babel/preset-env7 → 8 (deps(npm): bump @babel/preset-env from 7.29.3 to 8.0.5 in /tests/startup_perf/BlankRNW #1222, deps(npm): bump @babel/preset-env from 7.29.3 to 8.0.5 in /tests/stress_perf_rn/StocksGrid #1234, deps(npm): bump @babel/preset-env from 7.29.3 to 8.0.5 in /tests/stress_perf_rn/VirtualList #1239) — declarespeer @babel/core@"^8.0.0", but@react-native/babel-preset@0.82.0depends on@babel/core@^7.25.2.@babel/coremajor is held with it.@babel/runtime7 → 8 (deps(npm): bump @babel/runtime from 7.29.2 to 8.0.5 in /tests/startup_perf/BlankRNW #1223, deps(npm): bump @babel/runtime from 7.29.2 to 8.0.5 in /tests/stress_perf_rn/StocksGrid #1235, deps(npm): bump @babel/runtime from 7.29.2 to 8.0.5 in /tests/stress_perf_rn/VirtualList #1240) — declares no peers at all; held because it's whattransform-runtime's emitted@babel/runtime/helpers/*imports resolve against, so its major tracks the Babel core doing the emitting.better-sqlite311 → 13 (deps(npm): bump better-sqlite3 from 11.10.0 to 13.0.3 in /samples/apps/headtrax/service #1220) — cannot be installed on a stock Windows machine. v11 declaresinstall: prebuild-install || node-gyp rebuild, so it downloads a prebuilt binary and never invokes node-gyp. v13 declares no install script, so npm falls back to its implicitnode-gyp rebuild. v13 does ship prebuilds and itsbinding.gypdoes set every target totype: nonewhen one matches the host — but node-gyp evaluatesbinding.gyponly afterconfigure, and configure needs a detectable Visual Studio. On GitHub'swindows-latestrunner there is none, sonpm cidies atfind VS ... Could not find any Visual Studio installation to usebefore the short-circuit is reached.Stopping the next burst
Both npm ecosystem entries declare a minor/patch
groups:rule, so a future run arrives as one PR per directory; majors still get an individual PR.react-nativeand its two build-coupled satellites (@react-native/babel-preset,@react-native/metro-config) additionally ignore minor, because semver treats0.82.0 → 0.83.0as a minor — without that, the"*"group would have swept a whole React Native generation in as routine and silently invalidated every hold above.Why two
App.tsxfiles changedThe
@react-native/eslint-configbump raises@typescript-eslint/eslint-plugin8.59.1→8.70.0, which newly flagsconst X = React.memo(function X(...))underno-shadow. Both harness components are rewritten as a named declaration plusReact.memo(XImpl)— semantics-preserving, and it restores lint output to exactly the pre-change baseline.Harness
engines.nodewas raised from>=20to^22.13.0 || ^24.3.0 || >=26.0.0(what@react-native/eslint-config@0.87.1requires), and the HeadTrax service gained^20.19.0 || ^22.13.0 || ^23.5.0— faker's range intersected with the ABIsbetter-sqlite311.10.0 publishes prebuilds for.CI now covers Windows
node.yml's check matrix previously ranubuntu-latestonly, which is structurally blind to native-module install behaviour on the platform every contributor here actually uses. It now runs bothubuntu-latestandwindows-latest.That change caught a real regression on its first run: an earlier revision of this PR took
better-sqlite313, passed all six ubuntu jobs, and failedwindows-latestoutright. Without the Windows matrix this PR would have shipped a sample no Windows contributor could install.node-versionis also pinned to22.13.0rather than'22', so CI actually exercises the floor the projects declare instead of the newest 22.x.Validation
.github/workflows/node.ymlmirrored locally on Node 22.13.0 (the exact version CI pins) across all six npm projects —npm ci, every declared check script, and the committed-build-output freshness assertion. 21/21 pass, with lint warnings identical to baseline:tests/startup_perf/BlankRNWtests/stress_perf_rn/StocksGridtests/stress_perf_rn/VirtualListSame rules at the same sites; only line numbers shifted by the refactor.
npm cisucceeding also independently verifies every regenerated lockfile hash, since npm checksintegrityon install.Supersedes #1218, #1219, #1220, #1221, #1222, #1223, #1224, #1225, #1226, #1227, #1228, #1229, #1230, #1231, #1232, #1233, #1234, #1235, #1236, #1237, #1238, #1239, #1240, #1241, #1242.