Repository navigation
Migrate ui/ build tooling from CRA to Vite - #14
Open
b-eyselein wants to merge 4 commits into
Open
b-eyselein wants to merge 4 commits into
b-eyselein wants to merge 4 commits into
Conversation
react-scripts (webpack 5, ~900 transitive packages) is unmaintained; Vite gives a faster dev server and drops most of that dependency weight. Swaps react-scripts for vite + vitest, moves process.env.* to import.meta.env.*, and rebuilds the service worker registration on vite-plugin-pwa (injectManifest mode, reusing the existing Workbox-based src/service-worker.ts) so the offline-capable /OXTED standalone editor keeps working unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01528tdgcQt4KEUmzGX9RAm3
vitest@4 requires Node ^20 || ^22 || >=24, breaking the 18.x CI job; downgrade to vitest@3, which supports 18/20/22+. Its jsdom dep also got pulled to jsdom@30 (Node >=22 only) - pin jsdom@26, the latest release supporting Node 18. Also drop vite-tsconfig-paths, which isn't needed (tsconfig.json has no path aliases) and was forcing an unrelated typescript@^5 peer dep that fought with the project's pinned typescript@^4.9.5, producing an out-of-sync package-lock.json that `npm ci` rejected in CI. Restored @types/jest as a devDependency: the vendored simple_xml and simtex git dependencies each run `tsc` as a postinstall step against their own *.spec.ts files, which rely on ambient Jest globals resolved from the hoisted root node_modules/@types - removing it broke their install-time build. To keep those global Jest types from colliding with vitest's own globals in our own type-check, scope tsconfig.json's "types" to only the packages this project needs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01528tdgcQt4KEUmzGX9RAm3
b-eyselein
marked this pull request as draft
September 1, 2026 12:00
The PWA plugin's default minifier (terser) pulls in serialize-javascript, which reads the global crypto object at module load time - unavailable on Node 18, breaking the 18.x CI job with "ReferenceError: crypto is not defined". esbuild doesn't have that dependency and is already Vite's default minifier for the main app bundle. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01528tdgcQt4KEUmzGX9RAm3
Root cause of the CI failure: vite-plugin-pwa transitively requires workbox-build, whose generate-sw code path is loaded eagerly as part of the package's CommonJS require graph even though we only use its injectManifest strategy. That path requires @rollup/plugin-terser -> serialize-javascript, which reads the global `crypto` object at module-load time - only unconditionally available in Node 19+, so it crashed the build under Node 18 regardless of any Vite-level minify setting (tried and confirmed insufficient). Node 18 has been EOL since April 2025 and Node 20 is past its practical relevance too; rather than polyfill around an EOL runtime, move the CI matrix (and the previous vitest@3/jsdom@26 downgrade from the last commit, now reverted back to latest) to the current LTS pair (22.x maintenance, 24.x active). Verified locally under both via nvm: npm ci, build, test, and lint all pass cleanly on each. Also bumps actions/checkout and actions/setup-node to v4 - the v3 versions were forced onto a deprecated Node runtime by GitHub Actions and printing a deprecation warning on every run. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01528tdgcQt4KEUmzGX9RAm3
b-eyselein
marked this pull request as ready for review
September 1, 2026 12:17
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.
Summary
Migrates the
ui/frontend build tooling from Create React App (react-scripts) to Vite. CRA has been unmaintained for a while and its dependency tree (~900 transitive packages viareact-scripts) is heavy and slow; Vite gives a faster dev server and drops most of that weight without changing app code or behavior.react-scriptsforvite(dev/build) andvitest(test runner).npm run start/build/testkeep the same names and behavior.process.env.REACT_APP_*/NODE_ENV→import.meta.env.VITE_*/import.meta.env.PROD/DEV(NavBar.tsx,ForgotPasswordForm.tsx,urls.ts).public/index.html→ rootindex.html(Vite convention),%PUBLIC_URL%placeholders replaced with relative paths.vite-plugin-pwa(injectManifestmode), reusing the existing hand-written Workbox logic insrc/service-worker.tsunchanged in substance — only swapped the CRA-specificprocess.env.PUBLIC_URLURL construction for a scope-relative one.serviceWorkerRegistration.ts(CRA's manual registration boilerplate) is replaced byvite-plugin-pwa's built-inregisterSW().*.spec.tsfiles (165 tests) were actually running underreact-scripts test/ Jest — these were nearly dropped in an earlier pass of this migration on the assumption there were no tests (only*.test.*was checked). They now run unchanged under Vitest with zero code changes, since none used Jest-only APIs.eslintConfig: {extends: "react-app"}block frompackage.json(the standalone.eslintrc.ymlalready covers linting) and the CRA-onlyhomepagefield.[18.x, 20.x]to[22.x, 24.x](current LTS pair) and adds anenginesfield toui/package.json— see "CI Node matrix" below for why.No application code changes — routing, GraphQL, Redux, forms, and the XML editor are untouched.
Why the service worker matters here
The
/OXTEDroute (StandAloneOXTED.tsx) is a deliberately server-independent XML editor: it loads documents from local file upload, persists state tolocalStorage, and never calls the GraphQL backend. The precaching service worker is what lets that route keep working with no server reachable at all once the app shell has loaded once — confirmed below.CI Node matrix: why 18.x/20.x had to go
The old matrix (
18.x, 20.x) broke under the new toolchain:vite-plugin-pwaeagerlyrequire()s all ofworkbox-build— including itsgenerateSWcode path — regardless of which strategy is configured (we only useinjectManifest). That path pulls in@rollup/plugin-terser→serialize-javascript, which reads the globalcryptoobject at module-load time. That global is only unconditionally available starting in Node 19, so the build crashed under Node 18 withReferenceError: crypto is not defined— before any of our own build config was even consulted, so no Vite-levelminifysetting could work around it (confirmed by testing).Since Node 18 has been EOL since April 2025 (and 20.x is past its practical relevance too), rather than polyfilling around an EOL runtime the matrix moves to the current LTS pair: 22.x (maintenance) and 24.x (active). Verified locally under both via
nvm:npm ci,npm run build,npm test, andnpm run lintall pass cleanly on each.Also bumps
actions/checkoutandactions/setup-nodefrom v3 to v4 — the v3 versions were being forced onto a deprecated Node runtime by GitHub Actions, printing a warning on every run.Test plan
npm run build— type-checks (tsc --noEmit) and builds cleanly; service worker precache manifest generated (4 entries, ~1.4 MB)npm test(vitest run) — 165/165 tests passingnpm run lint— cleanvite) boots, all modules transform, app renders/tlh_editor/${VITE_VERSION}/public, matchingoxted.hethport.net) — app renders, service worker registers and activates/OXTED— app shell served fully from the service worker precache, uploaded a sample XML file (test/KBo 32.14.xml), transliteration reconstructed correctly with zero network accesslocalStorage, Apollo auth header wired up) → create manuscript (mutation + redirect to manuscript detail page) — all via a throwaway test user/manuscript, since cleaned up from the DBbuild (22.x)andbuild (24.x)Follow-ups (not in this PR)
browserslistconfig inpackage.jsonis now vestigial (Vite doesn't consume it directly) — left in place since autoprefixer still reads it; could be replaced withbuild.targetinvite.config.tslater if needed.🤖 Generated with Claude Code
https://claude.ai/code/session_01528tdgcQt4KEUmzGX9RAm3