Update toolchain (Fix #441) - #444
Open
stephaniehobson wants to merge 11 commits into
Open
stephaniehobson wants to merge 11 commits into
stephaniehobson wants to merge 11 commits into
Conversation
stephaniehobson
added this pull request to stack #445
September 18, 2026 21:13
stephaniehobson
force-pushed
the
441--update-toolchain
branch
from
September 18, 2026 21:13
0fce0eb to
0a569cb
Compare
stephaniehobson
force-pushed
the
441--update-toolchain
branch
from
September 21, 2026 16:55
0a569cb to
0fce0eb
Compare
stephaniehobson
force-pushed
the
441--update-toolchain
branch
from
September 21, 2026 16:56
0fce0eb to
f829661
Compare
stephaniehobson
force-pushed
the
441--update-toolchain
branch
from
September 21, 2026 18:42
1f57c16 to
8f3a435
Compare
stephaniehobson
force-pushed
the
441--update-toolchain
branch
from
September 21, 2026 18:51
8f3a435 to
85c80b9
Compare
stevejalim
approved these changes
Sep 22, 2026
Collaborator
There was a problem hiding this comment.
rubberstamp r+ -- i think we should get CI into play in a future step (EDIT: ah ha! #447) , then we can add a GHA to deploy to the bucket (which can confirm CI is happy first)
Plans CRA to Vite, Jest to Vitest, Nightwatch to Playwright, ESLint unification, and a bounded dependency update, to get a clean install and build on Node 24.
Removes react-scripts, chromedriver's OpenSSL-legacy build hack, and the IE11 polyfill scaffolding it required. Renames REACT_APP_* env vars to VITE_* and switches call sites from process.env to import.meta.env, Vite's actual convention. Replaces react-loadable with React.lazy/Suspense (plus a small LazyBoundary error boundary) after confirming hands-on that react-loadable's dynamic-import interop doesn't survive the move from webpack to Rollup. Bumps stylus to 0.64.0, a hard prerequisite for installing Vite at all. Verified with a real headless-browser session: every route renders against the production build with zero console errors and live chart data.
The previous commit's `git add` silently failed on a stale pathspec (public/index.html, already renamed) and aborted before staging anything else, so it only captured the rename/deletion. This commit has the real content: package.json/lockfile changes, env var renames, IE11 scaffolding removal, and the react-loadable to React.lazy swap described in the previous commit's message.
react-scripts supplied Jest's config invisibly (jsdom environment, CSS-import handling, Babel preset); removing it in Milestone 1 removed that config too. Vitest reuses the same Vite transform pipeline already added, needing no separate Babel setup. Both existing test files pass unmodified: 2 suites, 4 tests.
Removing react-scripts in Milestone 1 also removed its invisible, bundled eslint-config-react-app, which was the only thing linting .jsx files at all. Replaces the two-config split (.eslintrc.extra.js, never covering .jsx, plus CRA's invisible config) with one eslint.config.js covering both .js and .jsx. Pinned to eslint@9.39.5 rather than the 10.x line: eslint-plugin-react and eslint-plugin-jsx-a11y don't support ESLint 10 yet. Fixed the 9 stylistic warnings and no-undef errors this surfaced in previously-unlinted .jsx files. Confirmed JSX coverage is real by spot-checking that an introduced eqeqeq violation is caught.
chromedriver's postinstall fails outright on Apple Silicon, blocking npm install entirely; Playwright manages its own browser binaries. Ports all 13 files under src/tests/nightwatch/ (12 specs + a shared helper module) to tests/playwright/, updating Nightwatch's implicit first-match element semantics to Playwright's explicit .first() where needed, and its request-based external link checker to page.request. Chromium only, matching nightwatch.conf.js's own scope exactly - a WebKit project was tried and dropped after finding a reproducible, WebKit-only navigation stall unrelated to this port's correctness. Two tests fail for reasons external to this migration and are left failing rather than papered over: a dead forum link in Contact.jsx, and a donate.mozilla.org redirect that blocks non-browser requests. Both are new findings only possible because the suite can run at all now. Everything else (49 of 51 tests) passes reliably.
Bumps react-router-dom (patch), d3-scale/d3-selection/d3-shape, markdown-it/markdown-it-sup, memoize-one, react-ga, react-spinners, and source-map-explorer to their latest versions, each verified with a full build/test/lint pass. Swaps the unmaintained npm-run-all for its actively maintained npm-run-all2 fork (same binary name, no script changes needed). Removes d3-transition, confirmed unused. Fixes an unrelated pre-existing bug this work surfaced: npm run size has failed outright since the Vite migration (Milestone 1) because source-map-explorer treats Vite's entry-chunk source map as invalid; --no-border-checks resolves it. React, react-dom, react-test-renderer, enzyme-adapter-react-16, eslint, metrics-graphics/react-metrics-graphics, dateformat, and distinct-colors are deliberately left alone - see this milestone's Decision Log and Surprises & Discoveries for why each one is capped. npm audit: 17 vulnerabilities (3 moderate, 14 high, 0 critical), down from the pre-migration baseline of 251 (12 low, 136 moderate, 82 high, 21 critical).
Rewrites CONTRIBUTING.md's build/test commands, ESLint section, and testing guidelines for the new toolchain; updates the footguns table to remove what Milestones 1-4 fixed and add what they discovered. Updates docs/architecture/frontend.md's security-posture numbers and file layout, and README.md's stale create-react-app references. Final validation from a clean `rm -rf node_modules && npm install` with no flags, on Node v24.19.0: build, lint, unit tests, dev server, and the full Playwright suite all run cleanly. npm audit: 17 vulnerabilities (3 moderate, 14 high, 0 critical), down from the pre-migration baseline of 251 (12 low, 136 moderate, 82 high, 21 critical). This completes the toolchain modernization from issue #441.
stephaniehobson
force-pushed
the
441--update-toolchain
branch
from
September 22, 2026 15:31
85c80b9 to
ab6bb17
Compare
stephaniehobson
added a commit
that referenced
this pull request
Sep 22, 2026
Tighten engines.node to match npm-run-all2's floor, extend ESLint's plain-JS rules to .mjs/.cjs (vite.config.mjs was unlinted), harden the Playwright link-checker against a booted-but-blank page and against bot-blocked page.request.get() calls, drop the stale Jest/Nightwatch paragraph from README, and stop tracking generated Playwright test-results output. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Tighten engines.node to match npm-run-all2's floor, extend ESLint's plain-JS rules to .mjs/.cjs (vite.config.mjs was unlinted), harden the Playwright link-checker against a booted-but-blank page and against bot-blocked page.request.get() calls, drop the stale Jest/Nightwatch paragraph from README, and stop tracking generated Playwright test-results output.
stephaniehobson
force-pushed
the
441--update-toolchain
branch
from
September 22, 2026 17:55
036141c to
5c4120e
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Critical and moderate review findings remain unresolved, including route error recovery and test/configuration problems.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 3
Open (9)
Resolved since last review (5)
| "build:app": "vite build", | ||
| "build:version.json": "node scripts/generateVersionJSON", | ||
| "test": "npm-run-all lint test:jest test:nightwatch:dev", | ||
| "test": "npm-run-all lint test:jest test:playwright", |
Comment on lines
+20
to
+21
| <LazyBoundary> | ||
| <Switch> |
Comment on lines
+24
to
+28
| if (this.state.hasError) { | ||
| return ( | ||
| <ErrorComponent | ||
| title="Error" | ||
| message="Load error" |
Comment on lines
+22
to
+23
| npm run test:playwright # playwright test; starts the dev server itself if one | ||
| # isn't already running on :3000 |
Comment on lines
+28
to
+29
| files: ['**/*.{js,jsx}'], | ||
| ...jsxA11y.flatConfigs.recommended, |
| "private": true, | ||
| "engines": { | ||
| "node": ">=8" | ||
| "node": "^24.15.0" |
Comment on lines
+28
to
+35
| webServer: { | ||
| // npm start, not watch:app alone - a fresh checkout has no compiled | ||
| // CSS yet (src/components/views/css/ is gitignored), and watch:app on | ||
| // its own never runs the Stylus build that produces it. | ||
| command: 'npm start', | ||
| url: 'http://localhost:3000', | ||
| reuseExistingServer: !process.env.CI, | ||
| }, |
Comment on lines
+209
to
+210
| The user has not yet confirmed this six-milestone breakdown. Per this repository's ExecPlan | ||
| addendum, no milestone below is executed until that confirmation is given. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The site will build and display locally now 🎉