Conversation
The previous rules ignored /ui/build/*/*/*, which encoded the directory depth of the old asset layout: build/static/js/main.js sits three levels under build, so it matched. Assets emitted directly into build/static are two levels down and matched nothing, leaving 229 build artifacts totalling 9.9 MB staged for commit. Ignore everything under build and re-include the one file that is tracked there, so the rules stop depending on how the build tool happens to nest its output. (cherry picked from commit 5fd7fc8)
react-scripts and react-app-rewired are replaced by Vite. The configuration preserves the two output contracts the Go server depends on: the build directory stays at ui/build, which static.go embeds, and emitted assets stay under static/, which internal/router/ui.go serves as a route. Emitted files keep the static/js, static/css and static/media grouping, because both the repository's .gitignore and the analyze script match on that layout. Production sourcemaps stay enabled to match the previous build, and the REACT_APP_ prefix is retained so scripts/env.js remains the single source of truth for configuration shared with the server. sass and @types/node are raised to the versions Vite requires; the previous sass predates the async compiler API the current toolchain calls. (cherry picked from commit 70cf6ee)
Values still come from .env files generated by scripts/env.js under the REACT_APP_ prefix, so no configuration keys change. The public URL needs care rather than a direct substitution. The previous toolchain stripped the trailing slash before exposing the value, so it read as an empty string at the site root. import.meta.env.BASE_URL keeps the slash, and substituting it directly produced a protocol-relative //custom.css. The trailing slash is stripped here to preserve the previous behaviour at the root and to stay correct if the site is ever served from a sub-path. (cherry picked from commit 5e1625c)
require() in application source relied on the previous bundler's CommonJS interop. Each call is replaced with the import form matching how the package actually publishes itself: a namespace import for diff, which ships ESM with named exports and no default, and named imports for semver, whose single static export object is statically analyzable. (cherry picked from commit 5ce56c9)
The tilde prefix is a webpack resolver convention with no equivalent elsewhere. bootstrap-icons is given its explicit entry path rather than relying on the package's sass field being consulted during bare-specifier resolution. (cherry picked from commit 49b0ff5)
Routes loaded pages with a dynamic import built from an aliased template literal. That shape cannot be analyzed, so no page received its own chunk and the specifier reached the browser untransformed, leaving every lazily routed page unable to load. The entry bundle absorbed the pages it should have split out, growing to over a megabyte. Pages are now enumerated with a bounded glob covering the three directory depths routes actually use, excluding component subtrees so their own index files do not become route chunks. A page path with no matching module now rejects with the requested path and the list of known keys, surfacing through the existing route error boundary. Two routes are already in that state, pages/403 and pages/Admin/UserOverview, neither of which has a module in the tree; both failed the same way before this change, silently. (cherry picked from commit 4fca611)
GetStyle scraped index.html with regexes matching one exact tag shape: classic scripts with defer first, and stylesheet links with href before rel. Any bundler emitting a different shape returned nothing, and server-rendered pages would load with no JavaScript and no stylesheet while every build step still reported success. The tags are now read from the parsed document, so attribute order, attribute set and quoting no longer matter. header.html emits the scraped paths as script tags itself, and those were classic scripts. A module bundle loaded that way fails on its first import, so fixing only the parsing would have left server-rendered pages broken; the tag is now declared as a module. The self-check fixtures are replaced. The previous two asserted failure on module scripts and on rel-before-href, both of which the parser now accepts, so they would have inverted into false alarms. The replacements cover a stylesheet with no script, a script with no stylesheet, and an inline script with no src. golang.org/x/net moves to a direct requirement, matching its use here. (cherry picked from commit eab6f9d, ui/template/header.html only)
react-scripts, react-app-rewired, customize-cra and config-overrides.js are no longer reachable now that the build runs on Vite. yaml-loader is replaced by the equivalent Vite plugin. Three of the removed packages were already inert before this migration began. Both purgecss packages were declared but wired nowhere: no postcss config exists in the repository, config-overrides.js never referenced them, and no script invoked them. buffer was aliased and provided as a global, but no application source uses it, and the one dependency that requires it declares buffer as false in its browser field, so bundlers stub it out. The eslint config no longer extends react-app/jest, which shipped inside react-scripts and configured rules for a test suite this project does not have. (cherry picked from commit 1605fa7)
Plugin i18n modules call initI18nResource while they are being evaluated. i18next only attaches its resource-store methods to the instance during init, so if a plugin module evaluates before i18next.init has run, the immediate addResourceBundle call throws. Whether that happens is decided by the bundler: it is a function of how modules are grouped into chunks and in what order those chunks evaluate. Nothing in the application controls it and nothing reports it. The build succeeds, the dev server works, the page returns 200 with the server-rendered markup present, and the console stays empty, because the throw happens while the entry module is still evaluating and takes the whole application down before it mounts. The user gets the loading spinner forever. Register immediately only when there is an initialised instance to register into, and otherwise let the existing initialized handler do it. That makes the call correct in either order rather than correct in the one order the current chunking happens to produce. (cherry picked from commit 8a2b44e)
None of these are referenced any more, verified by searching the source, the scripts, the workflows, the Makefile, the Dockerfile and the shell scripts: - postcss, declared but wired to nothing; no postcss config exists, and the build tooling pulls its own copy - @testing-library/dom, already an indirect dependency of @testing-library/react - @testing-library/user-event, imported nowhere The ignore file still listed a config file that no longer exists, and two comments pointed at an index.html path that moved. The env generator also emitted three variables the current build never reads: two compiler flags belonging to the old toolchain, and a bare PUBLIC_URL that bypassed the REACT_APP_ prefix. The prefix itself stays, since the build config reads exactly that prefix and the generator remains the single source of truth for both sides. (cherry picked from commit ffa3160)
The config is written in ES module syntax but was being loaded as CommonJS, which the build tool warns about on every run and has announced it will stop supporting. The warning printed on every check run too, which is noise in output that is meant to be read. Renaming to .mts makes it load as an ES module, where __dirname does not exist, so derive the directory from import.meta.url instead. Using fileURLToPath rather than import.meta.dirname keeps it working on the whole Node range the package declares support for. (cherry picked from commit 1b59fa7, ui/vite.config.ts to ui/vite.config.mts only)
Module scripts are deferred by definition, so defer has no effect on them. Leaving it there reads as though it controls load behaviour. (cherry picked from commit f3eba46)
src/App.test.tsx is the test that ships with a fresh Create React App project. It asserts the text "learn react", which appears nowhere in this application, so it would fail if it ran, and it cannot run: no test runner is installed, and there is no test script, runner config or setup file. It was the only reason three devDependencies and a tsconfig include pointing into node_modules were still present. Verified before removing that it is the only test file, that nothing else imports the testing library, that no plugin workspace package depends on it, and that the project still type-checks with the include gone. The commit that removed the rest of that toolchain missed this file. (cherry picked from commit bcdee31)
bootstrap-icons.scss builds its @font-face url()s from $bootstrap-icons-font-dir, which defaults to "./fonts", a path meant to be relative to the partial's own location inside node_modules. Dart Sass does not rebase url()s during @import, so that relative path survived unchanged into the compiled CSS. Vite then had no way to resolve "./fonts/bootstrap-icons.woff2" from the entry stylesheet, so the production CSS shipped the literal unresolved path and the build emitted zero font files. Every icon in the app rendered as a missing-glyph box. Setting the font-dir variable to a package-resolvable specifier before importing the partial lets Vite's asset pipeline resolve and emit the woff/woff2 files under static/media and rewrite the url()s to point at them. (cherry picked from commit d78f566)
configs/config.yaml's ui.public_url used to flow into the old CRA build's PUBLIC_URL, which set webpack's public path and was read directly by PageTags for the custom.css href. The Vite migration dropped that bare PUBLIC_URL variable as apparently unused, but nothing replaced its job: vite.config.mts never set base, so import.meta.env.BASE_URL stayed at the default "/" regardless of config, and index.html's manifest link had been hardcoded to a root-relative path during the migration. A deployment served from a non-root public_url would silently lose asset and manifest prefixing. Derive base from the same REACT_APP_PUBLIC_URL value scripts/env.js already generates from config.yaml, normalizing the trailing slash Vite requires without double-slashing the root case. Let the manifest link use Vite's %BASE_URL% html macro instead of a hardcoded path, so it resolves the same way the rest of the built asset tags do. PageTags' existing custom.css href logic already derives correctly from BASE_URL once it carries the real value, so it needed no change. Verified with a build against a non-root public_url: the emitted index.html's script src, stylesheet hrefs, modulepreload links and manifest link all came out prefixed with the configured path, and the default root config still builds byte-identical manifest and asset paths to before. (cherry picked from commit 9c4b557)
CRA's webpack build surfaced type errors through its fork-ts-checker plugin, so a broken type would fail the build. Vite's build has no equivalent checker wired in, and the migration dropped type checking from the build entirely with nothing else covering it. Run tsc in check mode before the bundler so the build fails again on type errors. tsc --noEmit is clean today and takes about 4 seconds. (cherry picked from commit 794abd8)
(cherry picked from commit 11ee0e8)
Every build prints dozens of Sass deprecation warnings (color function renames, mixed-decls) that originate entirely inside bootstrap 5.3.3's own scss files. There is nothing to act on here, and the volume buries any warning that points at our own code. quietDeps only silences warnings whose source sass file is loaded as a dependency, so warnings from our own stylesheets still show. (cherry picked from commit 0fbf501)
yaml@2.6.1's core schema kept bare dates as strings and left merge keys unresolved. @modyfi/vite-plugin-yaml defaults to js-yaml's DEFAULT_SCHEMA, which resolves bare YYYY-MM-DD scalars to JS Date objects and turns on merge key support. Nothing in the current i18n or plugin yaml corpus hits either case, so this is a guard against a future translator adding one and silently changing a string field into a Date. Pin CORE_SCHEMA, which the plugin accepts directly as a js-yaml Schema value. js-yaml is already a direct devDependency used by scripts/env.js and scripts/loadPlugins.js, so this adds nothing new to package.json. Verified with a standalone parse of every file under i18n/*.yaml and ui/src against both schemas: all 54 files parse identically, so the pin changes nothing for the real corpus. A synthetic bare-date and a synthetic merge-key fixture each parse differently under the two schemas, confirming the pin has real effect where it would matter. (cherry picked from commit 3340c22)
Module scripts fetch in CORS mode, so the tag the bundler emits for this entry carries the crossorigin attribute. The server-rendered template was missing it, which meant the two render paths for the same script produced different tag shapes for no functional reason. Adding crossorigin here brings the template in line with the built output. Note that this attribute does not itself impose a new requirement: any CDN origin serving these files already needs to send Access-Control-Allow-Origin for module scripts, because that follows from type="module" regardless of whether crossorigin is present on the tag. (cherry picked from commit 80ec426)
The release pipeline builds the frontend with a pinned Node version before running goreleaser. Vite, the bundler this project now uses, declares an engines floor of node ^20.19.0 || >=22.12.0, and the pinned 20.18.1 sits below that floor. Bump the pin to 20.19.0 so the release build runs on a Node version Vite actually supports. Checked every other workflow file under .github/workflows: none of them pin a Node version for a frontend build. The Docker image workflows only invoke docker buildx against the root Dockerfile, which installs Node from an unpinned Alpine package, a separate concern not touched here. (cherry picked from commit f0ee9b9)
Vite's resolveBaseUrl keeps an absolute external base exactly as configured only when the command is build. For dev and preview (both command=serve) it silently reduces the same base to its pathname, dropping scheme and host. Confirmed by reading resolveBaseUrl in the installed vite package: the external branch only survives when isBuild is true. Our base wiring passes REACT_APP_PUBLIC_URL straight through with no mode check, and that is correct: only vite build output ever reaches the Go server, which is the only thing actually deployed. Local dev and preview serving a reduced base is a difference in a throwaway artifact, not a bug. Recorded the constraint next to the code so a future change does not "fix" serve mode into breaking the one output that matters. (cherry picked from commit 7a65192)
The declared floor was >=20, which admits Node 20.0 through 20.18, 21.x, and 22.0 through 22.11, all of which vite 8 warns about on every invocation: its own supported range is ^20.19.0 || >=22.12.0. Declare that exact range so the engines field and the tool agree on which Node versions are expected to work. pnpm reports a mismatch as a warning either way, since engine-strict is not set. (cherry picked from commit 36cd19a)
No ImportMetaEnv augmentation existed, so import.meta.env.REACT_APP_* fell back to vite/client's index signature and typed as any. A typo in one of those keys compiled clean and would only surface at runtime as a missing value. Added the strictImportMetaEnv marker interface vite/client checks for, which drops that fallback, plus an ImportMetaEnv augmentation for the keys actually read that way. Grepped import.meta.env.REACT_APP_ across src and vite.config.mts first: only REACT_APP_API_URL (request.ts) and REACT_APP_BASE_URL (App.tsx, router/alias.ts) are read through import.meta.env, so only those two are declared. REACT_APP_PUBLIC_URL exists too, but vite.config.mts reads it through loadEnv, never through import.meta.env, so it stays out. Verified with a throwaway probe referencing import.meta.env.REACT_APP_TYPO: tsc failed on it naming the probe file, then passed again once the probe was deleted. vite/client's own BASE_URL, MODE, DEV, PROD and SSR keys still type-check unchanged, since strictImportMetaEnv only removes the fallback for keys nothing declares. (cherry picked from commit d3f21dd)
The declaration read var(-bs-body-bg) with a single leading dash, which is not a valid custom property reference, so the background-color was discarded and the editor never received the themed background it asks for. The previous CSS minifier accepted the invalid value and passed it through; the current one rejects it outright, which is how it surfaced. This changes rendering: the editor now takes the themed background it was always meant to have. (cherry picked from commit 4f1ae4f)
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
1 participant
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.
Fixes #1580. Part of #1578. Step 2 of 3, atomic. Depends on #1582 (the Go asset contract, now merged into
dev), which is why the server-side parsing does not appear in this diff.Replaces
react-scriptsandreact-app-rewiredwith Vite.The configuration preserves the output contracts the server depends on: the build directory stays at
ui/build, whichui/static.goembeds, and emitted assets stay understatic/, whichinternal/router/ui.goserves as a route. Files keep thestatic/js,static/cssandstatic/mediagrouping, which theanalyzescript matches on; the ignore rules forui/build, previously pinned to that layout's directory depth, now ignore everything underbuildexcept the tracked favicon. Production sourcemaps stay on, and theREACT_APP_prefix is retained soui/scripts/env.jsremains the single source of truth for configuration shared with the server. That same script'spublic_urlvalue now also drives Vite'sbaseconfiguration, and the manifest link inui/index.htmluses Vite's%BASE_URL%macro instead of a hardcoded path, so a non-root deployment keeps every asset and manifest reference prefixed the way it did under the previous toolchain'sPUBLIC_URLwiring.Agentic tooling did the mechanical work in this series; every change was reviewed by a human before being committed.
The one server-side line
header.htmlre-emitted the pathsGetStyle()finds as classic scripts. An ES module loaded through a classic script tag fails on its first import, so the tag is now declared as a module and carriescrossorigin, matching the tag Vite's own build emits for the client-rendered entry point. Rewriting the builtindex.htmlinstead was not an option, becauseinternal/router/ui.goserves that same file to boot the SPA and it cannot misdeclare its own script type. Atype=modulescript fetches in CORS mode, so any CDN origin serving these files needs to send the matching CORS headers, with or withoutcrossoriginpresent; default same-origin deployments are unaffected. The note for CDN users is in the CDN plugin READMEs, apache/answer-plugins#326, per the placement decided on #1567, and the release notes for the version that ships this should carry a short pointer to it.Route code splitting
Routes loaded pages with
lazy(() => import(`@/pages/${pagePath}`)). That shape cannot be statically analyzed, so no page received its own chunk and the specifier reached the browser untransformed, leaving every lazily routed page unable to load. Pages are now enumerated with a bounded glob covering the three directory depths routes actually use, excluding component subtrees so their own index files do not become route chunks. A page path with no matching module now rejects with the requested path and the list of known keys, surfacing through the existing route error boundary.Behaviour changes, called out deliberately
The custom stylesheet link now derives its href from the build's own base, not a separately computed value.
PageTagsbuilt the/custom.csslink fromprocess.env.PUBLIC_URL, which the previous toolchain exposed with its trailing slash already stripped; at the default configuration that value was an empty string, resolving to/custom.css.import.meta.env.BASE_URL, the direct equivalent under the new toolchain, keeps the trailing slash, so substituting it in the same place would resolve the same default configuration to//custom.cssinstead. The trailing slash is stripped explicitly before the substitution, reproducing the previous output at the default configuration and staying correct away from it. The output here is unchanged; only the mechanism it depends on is.Two routes were dead ends and now fail loudly instead of silently.
pages/403andpages/Admin/UserOverviewboth referenced modules that did not exist in the tree, and both failed the same way under the previous toolchain, silently rendering blank. With the glob above, both now report the missing path through the route error boundary. Step 3 repointspages/403atpages/404/403, an existing component, so that route renders;pages/Admin/UserOverviewstays a visible gap because creating the missing page is a content decision. Neither is a regression.The markdown editor now renders its themed background.
src/components/Editor/index.scssreadvar(-bs-body-bg)with a single leading dash. That is not a valid custom property reference, so the declaration was discarded and the editor never received the background it asks for. The previous CSS minifier accepted the invalid value; the current one rejects it outright and fails the build ([lightningcss minify] Unexpected token Ident("-bs-body-bg")), which is how it surfaced and why the one-line fix is part of this PR rather than a follow-up. Fixing it changes rendering.Dependency removals
react-scripts,react-app-rewired,customize-craandconfig-overrides.jsare gone, andyaml-loaderis replaced by the equivalent Vite plugin, pinned to the schema the previous loader used so bare dates and merge keys keep parsing the same way they did before. The scaffold test the old toolchain generated (App.test.tsx) and its jest packages go with it; this project has no test runner and no unit tests, before or after.Three removed packages were already inert before this migration. Both purgecss packages were declared but wired nowhere: no postcss config exists, the overrides file never referenced them, and no script invoked them.
bufferwas aliased and provided as a global, but no application source uses it, and the one dependency requiring it declaresbuffer: falsein its own browser field.sassand@types/nodeare raised to the versions the toolchain requires; the previoussasspredates the async compiler API it now calls.sassis pinned below the release that begins deprecating@import, which this project uses across 30 files. Migrating those to@useis a separate concern. Bootstrap's own Sass internals print dozens of dependency deprecation warnings on every build, unrelated to anything in this project's own styles;vite.config.mtssetscss.preprocessorOptions.scss.quietDeps: trueto silence those specifically while still surfacing warnings from this project's own stylesheets. One such app-own warning remains in this PR's build output, a mixed-declarations notice fromComment/index.scss; the one-line reorder that clears it is a step 3 nit.The eslint config no longer extends
react-app/jest, which shipped insidereact-scriptsand configured rules for a test suite this project does not have.A failure found only by running the built application
With the build green and the dev server working, the built application did not boot. React never mounted, the page showed its loading spinner indefinitely, and the browser console was empty.
i18nextattaches its resource-store methods to the instance insideinit(). The builtin plugins register their translations while their modules are being evaluated. Whether that happens before or afterinitdepends on how the bundler groups and orders chunks, so the previously working order was incidental rather than guaranteed. When it inverts, the registration throws while the entry module is still evaluating, which takes the application down before it mounts and produces no console output.Registration now happens immediately only when there is an initialised instance to register into, and otherwise falls to the
initializedhandler the code already installed, which is correct in either order. The check that guards both orders is part of step 3.Parity fixes found in review
Bootstrap icon fonts. The bootstrap-icons stylesheet points at font files under a path the bundler could not resolve on the first migration pass, so the build silently emitted zero font files and every icon rendered as a missing-glyph box. The stylesheet now overrides the package's font-directory variable to a path Vite can resolve; the fonts are emitted, and the boot test below confirms they are served.
Type checking.
pnpm buildnow also runstsc --noEmitbefore the bundler runs, and is clean today. The previous toolchain ran its checker in the dev server (blocking) and during builds (downgraded to warnings by this project'sTSC_COMPILE_ON_ERRORsetting); none of that carried over when the bundler changed, so until this fix a type error shipped with no signal at all.Yaml parsing. The Vite yaml plugin defaults to js-yaml's more permissive schema, which resolves bare dates to JS
Dateobjects and enables merge keys; the previous loader's schema kept both as plain strings. The plugin is now pinned to that same schema.Declared Node range. The bundler's declared support range is
^20.19.0 || >=22.12.0.ui/package.json's ownengines.nodeallowed>=20, which admits versions below that floor, and now matches it exactly. The release workflow's pinned Node, which also sat below the floor, is raised to20.19.0to match. That version bump is the only change this PR makes anywhere under.github/; it does not add a job.Typed environment variables.
import.meta.env.REACT_APP_*previously typed asany, since noImportMetaEnvaugmentation existed; a typo'd key would compile clean and only surface as a missing value at runtime. The two keys the app reads this way,REACT_APP_API_URLandREACT_APP_BASE_URL, are now declared, withstrictImportMetaEnvenabled so an undeclared key is a type error instead of a silentany.Verification
On
devatace02c49, which includes step 1, plus these commits:pnpm install --frozen-lockfileunder the pinnedpnpm@9.7.0andpnpm build(which now includestsc --noEmit) complete; the build prints three warnings, each once:front-matter, a dependency unrelated to this project's own pinnedjs-yaml@^4.1.0, pulls in a legacyjs-yaml@3.xcopy whosebufferimport gets externalized for browser compatibility; the Sass mixed-declarations notice fromComment/index.scssmentioned above; and one chunk over the default size threshold, see the note on chunking below.go build ./...andgo vet ./...pass;TestGetStyleResolvesBuiltAssetspasses against the Vite output; a search forreact-scripts,react-app-rewired,customize-craandconfig-overridesfinds nothing outside the lockfile. The built binary was booted against sqlite3 and loaded in a browser:/renders with the client mounted (React's fiber container present on the root element), the module entry script and both entry stylesheets are fetched, and the console shows no errors. The same holds for/tags. The server-rendered/response itself carries the module entry script and both entry stylesheets, which isGetStyle()from step 1 finding them in the Vite output andheader.htmlre-emitting them.The three
make check-uiguards from #1567 (asset paths, non-default locale, plugin i18n order) are not in this PR; they arrive as step 3 and were run green against the equivalent tree there.Measurements
Captured for #1567 at its head
d06b623, againstmainat3b9f137, same machine, same Node and package manager versions, clean tree and clean install on both sides, five runs per timing metric. They are carried over rather than recaptured: the frontend build inputs here are the same as at that head, minus the step 3 nits and plus the newer locale files ondev.Five things worth stating rather than leaving to be inferred:
Not included
This PR adds no CI job for the frontend. The project runs no frontend job today, and a build on every push is a cost a maintainer should choose to take on, not one this migration should impose. One constraint carries forward for whoever wires that job later: a bare
go test ./...reports ok while asserting nothing about the built asset paths, becauseTestGetStyleResolvesBuiltAssetsskips when no frontend build is embedded. Any future CI job needs to build the frontend first.The dev server now binds to loopback only by default; reaching it from another device on the network needs an explicit
--hostflag.Create React App's SVG-as-component imports (
import { ReactComponent as X } from './x.svg') are not carried over to this configuration. Nothing in this codebase used them.The commits carry
(cherry picked from commit ...)lines pointing at the branch behind #1567, where these changes were first reviewed and where the regression matrix (subdirectory deploy, OAuth callbacks, absolute CDNpublic_url) was run.