React 18, Vite (replacing CRA), dependency fixes - #456
Open
mikima wants to merge 13 commits into
Open
Conversation
Standardizes package management across all three RAWGraphs repos: core and charts already used npm, app was the only one on Yarn Classic (1.22, in maintenance mode since 2020). The local node_modules had already drifted from yarn.lock (React 19 installed against a ^17.0.2 declaration), a symptom of npm and yarn being run interchangeably against the same tree over time. - Removed yarn.lock, generated package-lock.json from a clean install. - Pinned react-data-grid and sparqljs to their exact previously-resolved versions instead of the loose caret ranges: without a lockfile, npm picked react-data-grid 7.0.0-canary.49 (33 releases ahead, which dropped the bundled CSS the app imports) and sparqljs 3.7.4 (ships `??` syntax that this CRA4/babel-loader setup can't parse). Both are pre-1.0/canary packages where semver ranges don't give real guarantees, so exact pins are the safer default here. - Added .npmrc with legacy-peer-deps=true: react-data-grid@canary.16 declares a peer on react@^16.8 that was never updated, which Yarn 1 never enforced but npm 7+ rejects outright even though the app has run fine on React 17 against it for years. - Baked the OpenSSL legacy-provider workaround into the start/build scripts via cross-env (previously misapplied as a react-scripts CLI arg on start, and undocumented-in-scripts on build, so build failed outright on Node >=17 unless you knew to export NODE_OPTIONS manually first, per the old README). - Bumped CI (dev.yml, prod.yml) from Node 14.x (EOL since April 2023) to 22.x, replaced the manual yarn-cache steps with setup-node's built-in npm cache, and switched yarn install/build to npm ci/npm run build. - Updated README install instructions accordingly. Verified: npm run build completes successfully (previously failed immediately with the OpenSSL digital-envelope-routines error). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Targets 18 rather than 19: it's the safer intermediate step from 17, with years of ecosystem compatibility, whereas several pinned old libraries here (react-data-grid canary, react-color) are untested against 19's bigger API removals. Revisit 19 after the create-react- app replacement (next phase). - react/react-dom 17.0.2 → 18.3.1. - src/index.js: ReactDOM.render → createRoot(...).render(...), since the legacy render API is deprecated in 18 (and removed outright in 19, so this was going to be needed regardless). - react-cookie-consent 6.2.3 → 9.0.0, not the latest 10.0.2: 10.x's build output uses `??` syntax that CRA4's babel-loader doesn't transpile for node_modules, breaking the production build. 9.0.0's peer range (react >=16) already covers 18 and its build predates that toolchain change (verified: zero `??` in its dist bundle). - Added an "overrides" entry pinning @babel/core to ^7.29.7 project- wide. Without it, npm install resolves two incompatible copies: react-scripts pins @babel/core@7.12.3 directly, but @svgr/webpack (pulled in by react-scripts, used for CRA's SVG-as-component imports) resolves a newer @babel/preset-env whose plugins require @babel/core ^7.13.0+, and npm's dedup logic can't reconcile both in one tree. Forcing a single modern copy resolves it. This is unrelated to the React bump itself — it's the same failure mode that made the Phase 1 attempt on this repo fail earlier, this time triggered by React 18 pulling in a different transitive graph. Verified: npm run build compiles cleanly. Smoke-tested the running dev server in a browser — data loading, the data grid (react-data- grid), and the chart mapping UI (drag-and-drop dimension cards) all render and respond with no uncaught errors. Only pre-existing findDOMNode (react-bootstrap's Transition/Overlay) and a new defaultProps-on-function-component deprecation warning from the same components, both cosmetic console noise, not functional breaks. Not touched in this pass, left for later: react-data-grid stays pinned to its canary.16 version regardless (still peers on react@^16.8 — legacy-peer-deps in .npmrc carries this through as before); react-bootstrap stays on 1.x (its React-18-clean rewrite is 2.x, a separate jump); react-scripts itself is unchanged (its replacement is the next phase). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Found while testing the React 18 upgrade: dragging a chart dimension
crashed the whole DataMapping screen with
"Uncaught Error: Cannot have two HTML5 backends at the same time."
React 18's StrictMode intentionally mounts, tears down, and re-mounts
effects once in development to surface exactly this kind of bug —
react-dnd v11's HTML5Backend (from 2020, predates React 18) doesn't
clean up its backend registration in a way that survives that second
mount.
react-dnd v14+ fixed this. Bumping to 16.0.1 (react-dnd and
react-dnd-html5-backend) is not just a version bump though: the
useDrag spec's shape changed — `type` moved out of `item` to become
a top-level spec field, and reading an item's type on the drop side
now goes through `monitor.getItemType()` instead of `item.type`
(`monitor.getItem()` no longer carries it). Fixed in:
- ColumnCard.js / ChartDimensionItem.js: useDrag specs restructured
(`item: { type: 'x', ... }` → `type: 'x', item: { ... }`).
- ChartDimensionItem.js / ChartDimensionCard.js: drop-side
`item.type === 'column'` / `monitor.getItem().type` reads replaced
with `monitor.getItemType() === 'column'`.
Left one dead branch alone (ChartDimensionItem.js's `if (false &&
item.type === 'column')`) — permanently short-circuited already, so
never evaluates the `.type` access regardless of API version.
Verified: build compiles, and manually exercised the mapping screen
in a browser (Iris sample data) — no more crash, only the pre-existing
findDOMNode/defaultProps deprecation warnings from react-bootstrap.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Found while testing: selecting a chart and mapping dimensions (e.g.
Circular dendrogram with two columns mapped to "hierarchy") left the
preview spinning forever with nothing in the console — the app just
looked frozen, no error anywhere.
Root cause: mapDataInWorker() (and parseDatasetInWorker()) called
Comlink.wrap(mappingWorker) fresh on every invocation, re-wrapping the
same long-lived Worker instance each time instead of wrapping it once
and reusing the proxy. React 18 StrictMode's intentional double-
invoke of effects on mount was enough to trigger this in practice
(two wraps just from mounting), and every subsequent chart or mapping
change added another wrap on the same worker. After enough of these,
Comlink's request/response bookkeeping stops routing responses back
correctly and the promise never resolves — not rejects, just hangs,
which is why nothing showed up anywhere.
Fixed by caching the Comlink.wrap() result alongside the worker
instance and reusing it, matching the "create once" comment already
in the code for the worker itself.
Also stopped silently swallowing errors in worker.js's mapData()
(`catch (err) {}`) — found while debugging this, since it was hiding
the real "missing required dimension" validation errors that made
this bug hard to diagnose in the first place. Now logs them.
Verified: repeatedly switching chart type (which re-triggers the
effect and re-invokes the worker call each time, the exact pattern
that broke before) now consistently returns proper validation errors
instead of hanging, across many more invocations than it took to
break previously.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Found while testing: after mapping the required dimensions for a chart (e.g. Bubble chart's X/Y axes), the preview kept saying the last-dropped dimension was still missing, even though the UI showed it mapped. Adding one more (non-required) dimension would make it render correctly — a one-operation-behind lag, always trailing by exactly one drop. Root cause: commitLocalMapping() read a `lastMapping` ref that was synced to the local optimistic mapping state via a useEffect with no dependency array (runs after every render). This assumes the effect (which updates the ref) always runs before the next drag's `end` callback reads it — true often enough under React 17's less strict scheduling, but React 18's automatic batching doesn't guarantee that ordering, so commitLocalMapping could commit a snapshot from before the just-finished drop. Fixed by reading the true latest local mapping through setLocalMapping's updater-function form instead: React guarantees that receives the actual latest state when it runs, regardless of batching, removing the race rather than working around it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Found while investigating the reported layout bug (data table wrapping below the parsing options panel instead of sitting beside it, with a large empty gap where it should have rendered). DataGrid computed its column widths by reading containerEl.current.getBoundingClientRect().width directly in the render body. On the ref's first population this can race the sibling flex column's own layout pass and read the full pre-flex-share width (e.g. the whole 1280px row) instead of its actual ~1067px allotment. Columns then get sized to fill that wider measurement, the grid overflows its real share, and the row wraps — after which the container legitimately does have close to the full width, so the wrong measurement becomes self-consistent and nothing ever prompts a re-measure to correct it. Moved the measurement into a useLayoutEffect backed by a ResizeObserver, so it happens once layout has actually settled and recomputes whenever the container is genuinely resized, rather than being a one-shot read racing the surrounding flex layout. Verified: reloaded with the "Highest grossing movies" sample (5 columns, wide enough to trigger this before) — parsing options and table now render side by side as expected, confirmed via getBoundingClientRect (both at the same top offset, widths summing to the full row width). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The DataGrid width-measurement fix (previous commit) reduced but didn't eliminate the wrap: it still showed correctly for a moment before flipping to stacked, meaning a later re-measurement (from the ResizeObserver) computed a column width that, once rendered with real borders/padding, ended up a few pixels over the row's actual share — just enough to make Bootstrap's flex-wrap kick in, after which the container legitimately has the full width and the bad state persists. Rather than chase exact-pixel column math against react-data-grid's own box-model overhead, added flex-nowrap to the four sidebar+content Row layouts that share this pattern (fixed-width Col next to a flexible one): DataLoader (parsing options + preview table), ChartSelector (chart preview + type grid), DataMapping (columns + chart variables), ChartPreviewWithOptions (chart options + preview). None of these are meant to reflow on narrower viewports anyway — the app already tells users it's designed for larger screens (see ScreenSizeAlert) — so wrapping was never the intended behavior; any child overflow should scroll within its own space, not push the row onto a new line. Verified: reloaded with the "Highest grossing movies" sample and checked repeatedly over ~1.5s (the previous fix looked correct immediately but flipped shortly after) — options and table stay side by side throughout. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Found while testing the previous flex-nowrap fix: the data preview table no longer wrapped, but visibly grew wider over roughly a second until it pushed past the viewport, like a runaway animation. Root cause: flex items default to min-width: auto, meaning they won't shrink below their content's intrinsic minimum width. DataGrid's column-width calculation (previous commit) measures its container via ResizeObserver and sizes columns to fill it — but since the container is a plain, unconstrained flex child, any measurement that came out even slightly wide let the grid render wider, which grew the flex item itself past its intended share (rather than clipping), which the ResizeObserver then measured as the new "available" width, computing still-wider columns next — an unbounded feedback loop, only stopped from actually wrapping by the flex-nowrap added earlier, so it just kept pushing the row wider instead. Fixed with the standard fix for this flexbox default: min-width: 0 on the flexible sidebar-content column in the three places that pair a fixed-width Col with a content-driven one (DataLoader's table, ChartSelector's chart-type grid, DataMapping's chart variables). This lets the column actually respect its flex-computed share regardless of what its content wants, so any real overflow scrolls within that space instead of expanding it. Verified: reloaded with the "Highest grossing movies" sample and polled the table's width over 4+ seconds (it grew within about a second last time) — stayed exactly at its flex-computed width (1067.5px) throughout, no growth. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CRA was archived by Meta in Feb 2025 and its react-scripts toolchain
(webpack 4, Babel 7 as configured by CRA, jest via react-scripts test)
had become the main blocker to further dependency upgrades. Vite 7 is
the direct, actively maintained CRA replacement: esbuild-based dev
server, Rollup for production builds, first-class React support via
@vitejs/plugin-react, and no meaningful migration away from Rollup
needed later since Vite already uses it for production.
Key changes and the bugs found while making dev/build/preview all work:
- vite.config.js: esbuild's default `exclude: /\.js$/` silently
overrides a bare `include`, so JSX-in-.js (a CRA convention this app
relies on throughout) needs both include and exclude set explicitly
for the production build; the dependency pre-bundling scan run at
dev server startup uses a separate esbuild config under
optimizeDeps.esbuildOptions needing the same loader override.
- Some transitive CJS dependencies (sparqljs, concat-stream via
sparql-http-client, js-sha3) assume a Node/webpack environment and
reference `process`/`global` at runtime. Polyfilled via esbuild
`define` (reaching both the app code and dependency pre-bundling)
plus a runtime `window.process` object in index.html.
- sparqljs's generated parser ends with a dead "run as CLI" guard
(`require.main === module && ...`) that's still evaluated eagerly on
import. esbuild renames this to `__require` in dev (so the process
polyfill was enough there), but Rollup's production bundling leaves
the bare `require` reference untouched, throwing a ReferenceError
before the app ever mounts - only caught by testing the actual
`vite preview` production build, not just `vite build` succeeding.
Fixed with a `window.require = { main: undefined }` stub alongside
the process polyfill.
- Renamed src/index.js -> src/index.jsx: Rollup's HTML-entry parser
doesn't apply the custom esbuild loader override the same way it
does for regular imports.
- worker-loader's `worker-loader!./worker` syntax -> Vite's native
`?worker` import.
- CRA's `ReactComponent` named SVG import -> vite-plugin-svgr's
default export (`?react` suffix).
- SCSS `@import '~pkg/...'` (webpack alias) -> bare `@import 'pkg/...'`
for Vite/modern Dart Sass; renamed the `Inter Web` font folder to
`Inter-Web` since a space in the path resolved differently.
- Dropped react-scripts, worker-loader, cross-env, @testing-library/*,
and the CRA-specific eslintConfig/browserslist/babel override in
package.json; REACT_APP_VERSION -> VITE_APP_VERSION in the GH Actions
workflows and the one place it was still referenced in Footer.js.
Verified: production build and vite preview both load the app cleanly
with zero console errors, dev server smoke-tested (sample data load,
DataGrid render, Web Worker/Comlink chart computation round-trip).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
sparql-http-client (used by the SPARQL query data loader) defaults to nodeify-fetch. Its browser entry patches a real streaming fetch Response by calling res.body.getReader() to build a wrapped readable, but then forwards .json()/.text() calls to the ORIGINAL Response object whose body stream that same getReader() call already locked - throwing "Failed to execute 'json' on 'Response': body stream is locked" on every query, plus noisy "util.inspect"/"util.debuglog externalized for browser" warnings from the same dependency chain trying to log the response. This is a pre-existing bug in nodeify-fetch's own patching logic, not something introduced by the Vite migration - it surfaced now because whichever fetch implementation the old webpack build resolved apparently didn't take the streaming-body code path, masking it. Fixed by passing SimpleClient our own native window.fetch instead of letting it fall back to nodeify-fetch, bypassing the broken patch entirely (the browser's real fetch/Response already works correctly without any Node-interop shimming). Verified with the real Wikidata eye-color-distribution query in both the dev server and the production build/preview - loads all 50 rows with no console errors. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The $font-prefix SCSS variable (src/styles/_fonts.scss) pointed at a
relative path ('../../..') meant to be resolved against each vendored
IBM-Plex partial's own file location - correct on paper, but Vite's
Sass url() rebasing couldn't reliably trace that relative path back to
a real file through the legacy @import chain, so it left the url()
unresolved in the compiled CSS. At runtime the browser then resolved
it relative to the built CSS file's own location instead, landing on
a nonexistent path (hence the "OTS parsing error: invalid sfntVersion"
in the console - the dev server's SPA fallback HTML being served and
misinterpreted as font data).
Sidesteps the relative-path resolution question entirely: the 7 font
files actually referenced by the "regular" weight (the only one the
app imports) now live under public/fonts/IBM-Plex-Mono, served as
static assets at a stable root-absolute path Vite's asset pipeline
doesn't need to rebase or hash.
Verified the font file is now served correctly in the production
preview build (200, font/woff2, correct byte size).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
sparqljs, sparql-http-client and lit-html/sparql-editor are only needed by the "SPARQL query" data-loading tab, but were pulled into the main bundle by two static import paths: DataLoader.js (rendering the tab) and ParsingOptions.js (the "Refresh data from query" button, which looks up a fetch implementation by data source type). Rollup only code-splits a module if every path to it is dynamic - one static importer is enough to force it back into the main chunk regardless of what the other importer does. Converted both to dynamic import() (React.lazy + Suspense for the component, a plain async import() for the plain function), moving sparqljs and friends into their own ~463kB (144kB gzipped) chunk that now only loads when a user actually opens the SPARQL tab or refreshes SPARQL-sourced data - main bundle drops from ~2.34MB to ~1.88MB. Verified in the production preview build: the new chunk only appears in network requests after clicking "SPARQL query", and both querying Wikidata and refreshing already-loaded SPARQL data still work. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Dependency modernization for rawgraphs-app: React 17→18, and the biggest piece — replacing create-react-app with Vite (CRA was archived by Meta in Feb 2025 and had become the main blocker to any further updates). Also includes a fix for a security vulnerability (
js-cookie) and some lower-risk cleanup.No user-facing feature changes — same app, same behavior, different build tooling underneath.
React 17 → 18
Required real fixes beyond the version bump, all found and verified through hands-on testing, not just a passing build:
useLayoutEffect+ResizeObserver)create-react-app → Vite 7
Chosen because it uses Rollup for production builds — the same bundler already used by rawgraphs-core and rawgraphs-charts — so there's no second migration waiting down the line.
Real bugs found during the migration (not just config/warnings):
require.main === module) that's inert in the browser. esbuild neutralizes it automatically in dev, but Rollup's production build left a barerequirereference that threw immediately on page load — this only surfaced by testing the actual production build, not justvite buildsucceeding.nodeify-fetch(a dependency ofsparql-http-client) has its own bug in its browser-compat patch — it locks a Response's body stream viagetReader()and then forwards.json()/.text()calls to the original, already-locked stream. Fixed by passing the browser's nativefetchinstead of letting the library use its own.process/globalnot defined: a few dependencies assume a Node/webpack environment. Fixed withdefineinvite.config.js+ a runtime polyfill inindex.html..jsfiles (a CRA convention this codebase relies on throughout) needed an explicit esbuildinclude/excludeoverride — Vite's default silently excludes.jsfrom JSX transforms.Follow-up cleanup (lower risk, same branch)
@importchain) — moved topublic/fonts/with an absolute path.Security fix
react-cookie-consentbumped 9→10, resolving ajs-cookieprototype-pollution vulnerability (js-cookie now resolves to 3.0.8, above the patched 3.0.7). Verified the cookie banner still renders and the accept flow still sets the consent cookie correctly.Not included, deliberately
The remaining
npm auditfinding (d3-colorReDoS) isn't fixable from this repo — it comes from rawgraphs-core and rawgraphs-charts' own published dependencies. See the companion PRs on those repos.Also not touched: the ~285 Sass deprecation warnings and the
findDOMNode/defaultPropsReact warnings, both coming from Bootstrap 4 / react-bootstrap 1.x. Fixing those needs Bootstrap 5 (breaking: drops jQuery, renames CSS classes) — worth its own dedicated migration, not bundled into this one.