WIP: Modernize frontend build tooling and dependencies#244
Open
GoliathLabs wants to merge 7 commits into
Open
Conversation
The bundled ui/ frontend was on react-scripts 3.2 (2019-era CRA), which is long deprecated and drags in ~1900 transitive deps with hundreds of known vulnerabilities. This is the first of a staged modernization: swap only the build tool, no framework/library changes yet, so this step is independently verifiable before touching routing/Apollo/MUI in later stages. - Replace react-scripts with Vite 6 (classic Rollup-based, not the newer Rolldown-default majors, which have CJS-interop bugs with the legacy apollo-boost/react-apollo dependency chain still in place until a later stage removes them). - Move public/index.html to ui/index.html per Vite convention, drop %PUBLIC_URL% tokens, add the explicit module script tag Vite requires. - Add vite.config.ts: dev proxy to the Go backend, classic JSX runtime (React is still 16.12 pending a later bump), global -> globalThis define (webpack polyfilled this, Vite doesn't), events polyfill dependency for recharts' use of Node's EventEmitter. - Update ui/serve.go for Vite's flat build/assets/* output instead of CRA's build/static/js|css/*, and drop the service-worker.js/ asset-manifest.json routes since Vite never produces those CRA-only artifacts (serveFile panics on startup if a registered file is missing). - Fix a deep MUI v4 import (styles/createMuiTheme) that no longer exists in current 4.12.x patch releases, in favor of the still-exported barrel import. - Drop the now-unneeded NODE_OPTIONS=--openssl-legacy-provider CI workaround and pin an explicit Node 20 install in Dockerfile.dev instead of whatever Debian's apt happened to ship. Verified: yarn build/test, make build-js/test-js, a real go build + running binary serving the app, and a full browser smoke test (login, all nav routes, calendar/tags rendering) with zero console errors.
…nfig Second step of the frontend modernization: get the code checking under a current TypeScript/lint toolchain, still without touching MUI/Apollo/ router. tslint has been archived upstream for years; ESLint + typescript- eslint is the maintained replacement. - typescript 3.7 -> 5.9 (registry "latest" is a new 7.x line that typescript-eslint doesn't support yet, so pinning to the newest 5.x that the rest of the toolchain understands). tsconfig target bumped es5 -> es2020, dropped the "suppressImplicitAnyIndexErrors" option TS removed a few majors ago; all strict flags kept as-is. - Replace tslint/tslint-sonarts with a flat eslint.config.mjs (typescript-eslint + eslint-plugin-react + the two long-stable react-hooks rules). Full ruleset parity with the old tslint-sonarts config (cognitive-complexity, no-any, etc.) and the newer react-hooks rules that need actual code fixes are deferred to a later stage rather than rushed in here. - Prettier 1 -> 3, including the jsxBracketSameLine -> bracketSameLine rename, plus a one-time reformat. - Found and fixed a duplicate @types/react problem: several transitive @types/react-router*, @types/react-grid-layout and @types/recharts packages pin their own newer @types/react range, which pulled in a second, structurally different React.Component/ElementClass and broke every class-component library's JSX typing under TS 5's stricter checks. Pinned via yarn "resolutions" to a single @types/react/ @types/react-dom version, which cleared ~20 of the ~25 new type errors the TS bump surfaced. - Fixed several real, previously TS/tslint-invisible issues the stricter toolchain caught: a conditional React hook call in DateTimeSelector.tsx (hook was called after early returns), a mutated- props anti-pattern flagged by react-hooks lint, a dead `required` prop in FormTagSelector.tsx that never reached the underlying <FormControl>, and a moment.js import-style bug in DateTimeSelector.tsx (`import * as moment`, unlike every other file's `import moment`) that Rollup's stricter CJS interop turned into a runtime crash the moment a timer was started - webpack's looser interop had silently tolerated it. - Two remaining tsc errors are left in place on purpose, since fixing them now would be thrown away almost immediately: DateTimeSelector's @material-ui/pickers date-type mismatch (that component gets rewritten against @mui/x-date-pickers in the MUI v5 stage) and notistack's SnackbarProvider prop typing (notistack gets bumped in the dependency- cleanup stage). Verified: eslint/prettier/tsc/vitest all clean (bar the two noted exceptions), make lint-js/test-js/build-js, a real go build + running binary, and a browser walkthrough covering login, all nav routes, settings, tag autocomplete, and starting/viewing an active timer (the exact path that exercised the moment.js bug) with zero console errors.
Third step of the frontend modernization. Router v6 is a rewrite, not a
mechanical import swap: Switch is gone, Route no longer takes component/
render/exact, and non-Route children of a router (the v5 idiom this app
used to gate the whole app on login state) are no longer supported.
- react-router-dom bumped to ^6.30.4. Note: v7 is npm's "latest" tag now,
but its peer range requires React >=18 - our React bump is deliberately
deferred to a later stage alongside MUI v5, so v6 (still maintained,
supports React >=16.8) is the right fit for this stage rather than
pulling React 18 in early. react-router (the v5 "core" package) and
@types/react-router(-dom) are dropped; v6 ships its own types.
- Root.tsx is untouched - HashRouter still comes from react-router-dom
in v6, and the constraint to keep hash-based routing (no server-side
catch-all routes needed) still holds.
- Router.tsx: the old flat Switch containing a login Route, a bare
conditional <Redirect> (v5-only "non-Route child matches
unconditionally" idiom), and a <Page> wrapping a flat un-Switched list
of per-page Routes becomes two top-level <Routes>: one for
/user/login, and a /* catch-all whose element is the <Page> wrapping
its own nested <Routes> for the actual pages, keyed off relative
paths.
- common/Page.tsx: the routerLink helper's `innerRef` (an RRDv5-only
escape hatch, itself already deprecated in v5) becomes a plain
forwarded `ref`, which v6's Link supports directly. The nested
<Switch> that existed purely to compute a toolbar title string (never
rendered a route) doesn't translate mechanically - v6's Routes
renders the first match's element outright rather than allowing
pure-matching traversal - so it's replaced with useLocation() +
matchPath() over a plain title table.
- dashboard/DashboardPage.tsx: the RouteChildrenProps<{id}> match/
history props (injected automatically by v5's `component=`) become
useParams()/useNavigate(), since v6's `element=` doesn't inject
anything.
Verified: eslint/tsc/vitest clean (same two pre-existing exceptions
noted in the previous stage), make lint-js/test-js/build-js, a real go
build + running binary, and a browser walkthrough of every route
including creating a dashboard and navigating into its
/dashboard/:id/* detail page (the one route pattern that needed a
splat instead of a mechanical path translation) - title bar, sidebar
links, and back/forward all behave like before, zero console errors.
…ql-code-generator
Fourth step of the frontend modernization. All data access already used
hooks exclusively (zero <Query>/<Mutation> render-prop usage), so the
call-site diff is a mostly-mechanical import swap; the real work was the
client setup and replacing the deprecated apollo-boost codegen tool.
- provider/ApolloProvider.tsx: the apollo-boost client plus dual
react-apollo/@apollo/react-hooks ApolloProvider wrapping collapses to a
single @apollo/client ApolloClient + ApolloProvider. Construct HttpLink
explicitly rather than the `uri` shorthand, which apollo-boost carried
over from AC2 defaults but AC3 warns is deprecated.
- Every @apollo/react-hooks / react-apollo / apollo-boost / apollo-cache
import across ~40 files becomes @apollo/client (identical hook
signatures in v3, so no call-site logic changes).
- Codegen: replaced the `apollo` CLI (2.21, apollo-boost's codegen tool)
with @graphql-codegen/cli + typescript + typescript-operations,
generating into a single src/gql/__generated__/index.ts (still
gitignored/generated-at-build-time, same as before) instead of
apollo-codegen's one-file-per-operation layout. Pinned to the 5.x line
of the codegen packages after finding that the current 6.1.0 release
duplicates every schema-level enum/input type when typescript and
typescript-operations share one output file - a real regression in
that version, not a config mistake (confirmed by downgrading and
re-generating with no code changes).
- Naming: graphql-code-generator's typescript-operations plugin inlines
nested selections as anonymous object types instead of apollo-codegen's
per-nesting-level named interfaces (e.g. old `Dashboards_dashboards_
items` has no equivalent generated name anymore). Root operation types
become `<Name>Query`/`<Name>Mutation` (+ Variables) - a scripted,
whole-codebase rename - and the ~9 nested types the app referenced by
name got hand-derived indexed-access aliases in the new, hand-written
gql/types.ts (Dashboard, DashboardItem, StatEntry, Tag, TimeSpanItem,
Tracker, etc.), which is the one new non-generated file in this stage.
- Found and fixed two real bugs the migration surfaced:
- utils/strip.ts's stripTypename() used to delete the __typename
property in place. Apollo Client 3 deep-freezes cache result objects
in dev mode specifically to catch this, so the first cache read after
login crashed the entire app. Rewritten to build a fresh
stripped-down object instead of mutating the input.
- tag/suggest.ts's SuggestTagValue query fired with its required
`query` GraphQL variable undefined whenever a tag key was typed
without a `:value` suffix, since `skip` only checked for an exact key
match. Same request shape would have failed under the old stack too
(JSON.stringify drops undefined keys either way) - a pre-existing,
silently-swallowed bug, not a migration regression - fixed by also
skipping the query while no value segment has been typed yet.
Verified: eslint/tsc/vitest clean (same two pre-existing exceptions from
prior stages), make lint-js/test-js/build-js, a real go build + running
binary, and a full browser walkthrough - login, tag creation, starting/
stopping a timer, calendar view, dashboard creation and detail view,
settings - with zero console errors and zero unexpected network errors.
…time picker Fifth and highest-risk step of the frontend modernization: MUI v5, its React 17+ requirement (bumping React itself), and the fact that @material-ui/pickers has no direct v5 successor. - React 16.12 -> 18.3.1 (+ matching @types/react(-dom)). index.tsx now uses react-dom/client's createRoot instead of legacy ReactDOM.render. Targeted 18, not the newer 19, since several still-in-use packages (react-infinite, recharts@1.8.5 - both scheduled for replacement/staying as-is per the modernization plan) declare peer ranges that only cover React 16, and 18 is the better-tested landing spot for that mismatch than jumping straight to 19. - MUI: ran the official `@mui/codemod v5.0.0/preset-safe` first (handles ~38 of 40 files' import renames and overrides->components.styleOverrides translation) via a temporarily-restored @material-ui/core so the codemod's import resolution didn't fail, then hand-fixed the 2 files the codemod's optimal-imports pass couldn't process (deep NativeSelect import) and a handful of codemod artifacts it left broken: a ThemeProvider/MuiThemeProvider naming collision (the codemod renamed the MUI import to the same name as this project's own exported component), a redundant/malformed StyledEngineProvider wrapper the codemod duplicated into Root.tsx pointing at the wrong module, and palette.type (still passed through the codemod's adaptV4Theme() compatibility helper, kept rather than hand-rewriting the theme objects to native v5 shape) needing to become palette.mode to satisfy the v5 TypeScript types even though adaptV4Theme handles it identically either way. `Hidden` turned out to still exist in MUI v5 (removed only in v6), so no rewrite was needed there. `makeStyles` stays via the `@mui/styles` compatibility package, deliberately not rewritten to sx/styled - a separate, lower-urgency follow-up. - Pickers: common/DateTimeSelector.tsx and Root.tsx's MuiPickersUtilsProvider rewritten against @mui/x-date-pickers' LocalizationProvider/AdapterMoment and DesktopDateTimePicker (closest match to the old inline, non-modal KeyboardDateTimePicker UX). PopoverProps' onEntered/onExited became onOpen/onClose props directly on the picker; InputProps/margin/title moved under slotProps.textField. Verified interactively: the field's segmented text editing, the calendar-icon popup, month/day/hour/minute/AM-PM selection, and write-back into the timer all work. - React 18's types removed the implicit `children: ReactNode` that `React.FC` used to carry - every provider/dumb-wrapper component that destructured `children` without declaring it (Page, ApolloProvider, SnackbarProvider, ThemeProvider, BootUserSettings, Fade, Center, ConfirmDialog, DefaultPaper, FullCalendarStyling, ...) needed `React.FC<React.PropsWithChildren<...>>`. Also removed the MUI-v5-deleted `theme.mixins.gutters()` (6 files) in favor of its documented literal replacement, and added narrow, commented `as React.ComponentType<any>` casts for react-grid-layout and react-infinite's pre-React-18 type declarations - both packages are due for a version bump/replacement in the dependency-cleanup stage, which will carry current types and drop the need for the cast. - notistack 0.6.1 turned out to be a hard runtime blocker, not just a peer-dependency warning deferred to later: its SnackbarItem imports `@material-ui/core/styles` directly, which is a dynamic import a bundler can't resolve once that package is gone, crashing the entire app on load. Bumped to notistack 3.0.2 now (MUI-independent styling, no MUI peer dependency at all) rather than waiting - the per-variant color `classes` override API it used doesn't exist in v3's rewritten styling system, so that customization was dropped in favor of v3's default variant colors, which are visually very close to the removed custom ones. Verified: eslint/tsc/vitest fully clean (no exceptions left - both previously-deferred errors from earlier stages resolved incidentally), make lint-js/test-js/build-js, a real go build + running binary, and a full browser walkthrough covering every previously-tested flow plus a dedicated interactive pass on the date/time picker (open, navigate months, pick a day, change hour/minute/AM-PM) - zero console errors throughout.
Sixth step of the modernization. Stage 2 got the codebase off tslint with a minimal ruleset; this stage does the deliberate ruleset expansion that was deferred, now that the code reflects its final shape from stages 1-5. - Added eslint-plugin-sonarjs, the actively maintained ESLint equivalent of tslint-sonarts (same organization, largely the same rule names: no-identical-functions, no-redundant-jump, no-collection-size-mischeck, cognitive-complexity, no-duplicated-branches, no-nested-template-literals, etc.), rather than hand-porting that whole rule family individually. - Mapped the remaining core tslint.json rules to their typescript-eslint/ ESLint-core equivalents: no-any -> no-explicit-any, member-access -> explicit-member-accessibility (a no-op today - the codebase has zero class declarations, everything is hooks-based), variable-name -> naming-convention, triple-equals -> eqeqeq, no-var-keyword -> no-var, forin -> guard-for-in, switch-default -> default-case, no-shadowed-variable -> no-shadow, object-literal-shorthand -> object-shorthand, no-duplicate-imports, radix, use-isnan, no-debugger, no-conditional-assignment -> no-cond-assign, no-inferrable-types, no-namespace, no-duplicate-variable -> no-redeclare, no-string-throw -> no-throw-literal, no-unused-expression, and cyclomatic-complexity's ESLint-core equivalent (complexity). - Turning on no-duplicate-imports immediately surfaced ~50 warnings: the mechanical Apollo-migration rename in an earlier stage preserved apollo-codegen's old one-import-per-operation structure even though every operation now comes from the same combined gql/__generated__ barrel file. Merged those into a single import per source across 20 files rather than suppressing the rule. - The remaining ~14 real findings were genuine, fixable: a `const Error` component shadowing the global Error constructor (renamed to ErrorDisplay), a scattering of leftover `tslint:disable-next-line: no-any` comments that don't do anything under ESLint (replaced with real eslint-disable-next-line comments, and one of them - a DOM event target cast - could just be typed as HTMLElement instead of any), three nested ternaries extracted into named variables/if-else, a redundant trailing `return;` as the last statement of a function, and one pre-existing, already-tested, inherently-branchy relative-time parser whose cognitive complexity (37 vs the 20 allowed) is real but not something to fix as a side effect of a lint-config change - suppressed with a comment explaining why, same as the pickers/ react-grid-layout/react-infinite `any` casts from earlier stages. Verified: eslint/tsc/vitest clean, a deliberate sanity check that the config isn't silently inert (introduced a throwaway `any` + unused var, confirmed both rules fired, then fixed the file back), make lint-js/ test-js/build-js, a real go build + running binary, and the same full browser walkthrough as prior stages - zero console errors.
Seventh and final step of the frontend modernization: the remaining
small, independently-swappable dependencies flagged in the original
audit.
- downshift: removed entirely (declared but zero usages - TagSelector.tsx
already implements its own combobox logic by hand).
- lodash.clonedeep: removed; its one call site (cloning a dashboard entry
before editing) now uses structuredClone, standard in all supported
runtimes.
- @rooks/use-interval / @rooks/use-timeout: inlined as ~15-line custom
hooks in utils/hooks.ts (ref-based latest-callback pattern, matching
each package's actual behavior) - each package was a thin wrapper
around setInterval/setTimeout, not worth a dependency for 5 call sites.
- color-hash: 1.0.3 -> 2.0.2, dropped the `@ts-expect-error` in favor of
the real @types/color-hash declarations.
- react-grid-layout: 0.16.6 -> 1.5.3, not the current 2.x line - v2 is a
hooks-based rewrite that dropped WidthProvider (and the whole class-
component API this app's DashboardPage.tsx is built around) in favor
of new hooks, which would be a real rewrite, not a dependency bump. 1.x
keeps the same WidthProvider/class-component shape (confirmed by
diffing its source against 0.16) while being multiple years of
bugfixes newer. Paired with @types/react-grid-layout@1.3.6 (the last
version with real declarations - 2.x's types package is a stub
pointing at the source package's own bundled types, which only exist
in react-grid-layout v2). This also let the React-18-children cast
added in the MUI stage be removed - 1.3.6's types already declare
children correctly.
- react-color (ancient 3.0.0-beta.3, unmaintained) -> react-colorful:
swapped the hue-only SliderPicker for HexColorPicker (react-colorful
has no dedicated hue-slider export, only full pickers) in the two tag
color-editing dialogs - a small, visible UI change but a maintained,
dependency-free, actively-secure package.
- react-infinite: removed. Its virtualized-scroll usage in
timespan/DoneTrackers.tsx was replaced with a plain IntersectionObserver
sentinel that calls the same fetchMore() on scroll-into-view - this is
a personal time log, not large enough to need DOM virtualization, and
it deleted the height-tracking ref/effect machinery that existed only
to feed react-infinite's layout calculations.
- @fullcalendar/*: 4.3.x -> 6.1.21 across all six packages. This was the
largest real API migration in this stage: initialView (was
defaultView), datesSet (was datesRender), headerToolbar (was header),
dayHeaderFormat (was columnHeaderFormat), eventContent + eventDidMount
replacing the single DOM-mutating eventRender (v5+ moved event
rendering to a declarative model - content injection is now separate
from post-mount DOM side effects), EventChangeArg's oldEvent (was
prevEvent on the resize handler specifically, inconsistent with the
drop handler's already-correct oldEvent - once fixed both handlers
were identical and got merged into one). The bundled per-package CSS
imports (main.css) are gone - v5+ injects its own styles - which also
meant the app's custom FullCalendarStyling.tsx overrides needed their
selectors updated for the new DOM/class names (.fc-content ->
.fc-event-main, .fc-time-grid-event -> .fc-timegrid-event, .fc-today ->
.fc-day-today, etc.); the dead v4 day-grid/month-view rules were
dropped since this app only ever shows timeGrid views. Also had to
add an explicit `.fc { height: 100% }` rule, since fc's height:"parent"
option stopped auto-filling its container - unclear whether that's a
behavior change or just newly needing the CSS assist since the
removed default stylesheet controlled it before.
Verified: eslint/tsc/vitest clean, make lint-js/test-js/build-js, a real
go build + running binary, and browser walkthroughs of every affected
surface individually - the react-colorful picker (dialog open, pick a
color, save), the react-grid-layout dashboard (create, enter edit mode,
add-entry grid tile), and FullCalendar (week view renders pixel-similar
to before, an active tracker's event shows the live-updating STOP button
via eventContent, drag/resize/click handlers all still fire) - all zero
console errors. Final `yarn audit`: 114 critical/664 high/350 moderate/89
low -> 0 critical/8 high/6 moderate/0 low, all of which are transitive
lodash/d3-color findings from recharts (deliberately kept unchanged per
the modernization plan - bumping it is a separate, larger effort with
its own breaking-change surface, noted as a follow-up rather than done
here).
GoliathLabs
changed the title
Member
|
Thanks for the PR, it'll probably take some time for me to review this. |
Author
I've published a container image for my PR: https://github.com/GoliathLabs/server/pkgs/container/server Currently running & testing my traggo instance with it. If you're interested, I've built a gh-workflow for it to automate the build process: https://github.com/GoliathLabs/server/blob/ghcr-image-test/.github/workflows/ghcr-test.yml |
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
2 participants
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.
Disclaimer: I haven't had much time, so I asked Claude to take a look at this. I will test it further and report back on whether everything is working as expected. Feel free to review it in the meantime.
Summary
The bundled web frontend hadn't been updated since 2019 and was flagged by
yarn auditwith 114 critical / 664 high / 350 moderate / 89 low advisories across ~1900 transitive dependencies (CRA 3.2/react-scripts, React 16.12, TypeScript 3.7, MUI v4, Apollo Boost, react-router v5, tslint). This PR is a full modernization to current, actively-maintained major versions, done as 7 sequential, independently-reviewable commits rather than one large diff.This is a frontend/build-tooling only change — no changes to the Go backend, database schema, or user data/migrations.
What changed, by commit
vite.config.ts, dev proxy to/graphql, updated Go-side static asset route inserve.gofor Vite's flatbuild/assets/*output, dropped the Node OpenSSL legacy-provider workaround in CI.Switch→Routes, hooks-based params/navigation, rewrote the route-matching logic used to compute the page title.ApolloClient/ApolloProvidersetup; replaced the deprecatedapollocodegen CLI withgraphql-code-generator.createRoot, new theme API (palette.mode,components.styleOverrides),@mui/stylescompat package to avoid a fullsx/styledrewrite, and a from-scratch date/time picker rewrite against@mui/x-date-pickers(no direct v5 successor existed for the old picker).eslint-plugin-sonarjsfor cognitive-complexity checks) so review strictness didn't regress from the tooling swap.downshift,react-infinite,lodash.clonedeep,@rooks/*), and swappedreact-color→react-colorful,react-grid-layout0.16→1.5.3, and@fullcalendar/*v4→v6 (incl. fixing a calendar-body layout regression from the CSS/class-name changes).Version choices generally favored the safer intermediate major over the absolute latest where a newer major would have meant a much larger rewrite for no security benefit (e.g. MUI v5 not v9, react-router v6 not v7, react-grid-layout 1.x not the hooks-rewritten 2.x line).
Security outcome
The residual 8 high / 6 moderate findings are all transitive
lodash/d3-coloradvisories fromrecharts, which was deliberately left unchanged — bumping it is a separate effort with its own breaking-change surface and is out of scope here.Test plan
tsc/eslint/prettier/vitestclean at every stagemake lint-js,make test-js,make build-jspassgo vet ./...clean, realgo build+ running binary