Skip to content

Update toolchain (Fix #441) - #444

Merged
stephaniehobson merged 15 commits into
mainfrom
441--update-toolchain
Oct 1, 2026
Merged

stephaniehobson merged 15 commits into
mainfrom
441--update-toolchain

Conversation

@stephaniehobson

Copy link
Copy Markdown
Collaborator
  • CRA → Vite
  • Jest → Vitest
  • Nightwatch → Playwright

The site will build and display locally now 🎉

This comment was marked as outdated.

This comment was marked as outdated.

@stevejalim stevejalim left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

This comment was marked as outdated.

Base automatically changed from agents to main September 22, 2026 15:30
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>
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.
@stephaniehobson
stephaniehobson requested a lite review from Copilot September 22, 2026 19:09

This comment was marked as outdated.

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.
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.
- Move the donate.mozilla.org redirect-only check into utils.js's base
  loadsSuccessfully, where the general link-check logic lives, instead
  of leaving it only in the CI branch.
- Key Main.jsx's LazyBoundary on the route and wrap the component in
  withRouter so a failed lazy-chunk load no longer bricks every later
  navigation.
- Restore the pre-migration label/control association requirement via
  jsx-a11y/label-has-associated-control's assert option.
- Resync package-lock.json's engines.node with package.json.
- Only start Playwright's local dev server when PLAYWRIGHT_BASE_URL is
  unset, so remote/stage runs don't need a local toolchain.
- Document the one-time Playwright browser install in CONTRIBUTING.md.
- Correct the ExecPlan's "two known e2e failures" narrative: the
  Discourse link claim was already stale when written (removed from
  Contact.jsx in db91842, predating this plan), and the
  donate.mozilla.org failure is now fixed above. npm test passes clean.

This comment was marked as outdated.

Replace it with what's actually documented here: this repo produces
a static build via npm run build:app. How that build is deployed
lives outside this repository and isn't written down anywhere in it.
@stephaniehobson
stephaniehobson merged commit ab9ae53 into main Oct 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants