Skip to content

Migrate from Create React App to Vite#305

Open
ankit0504 wants to merge 5 commits intoLanguage-Mapping:masterfrom
ankit0504:agupta/upgrade-phase-2-cra-to-vite
Open

Migrate from Create React App to Vite#305
ankit0504 wants to merge 5 commits intoLanguage-Mapping:masterfrom
ankit0504:agupta/upgrade-phase-2-cra-to-vite

Conversation

@ankit0504
Copy link
Copy Markdown
Collaborator

@ankit0504 ankit0504 commented Apr 20, 2026

⚠️ Stacked on #304 (phase 1: React 16.14 + TS 4.9 bumps).

Because this repo uses a forking workflow and I can't push a base branch to upstream, this PR targets master directly, which means the diff temporarily includes phase 1's commits. Once #304 merges to master, GitHub will automatically recompute this PR's diff to show only the phase 2 changes.

Please review and land #304 first.

Issues resolved by this pull request

None — this is phase 2 of the staged upgrade effort. It does not close a tracked issue but does eliminate the OpenSSL workaround required to run the project on any Node version above 16, and clears the pre-existing lint backlog the CRA preset had been hiding.

Prerequisites

  • Reviewer should run yarn install after checkout (new Vite/SVGR deps; old CRA deps removed).
  • After this merges, the NODE_OPTIONS=--openssl-legacy-provider flag is no longer required for yarn start or yarn build. The netlify.toml does not reference this flag, so no Netlify config change is needed.
  • No .env changes required — Vite is configured with envPrefix: ['REACT_APP_', 'VITE_'], so existing REACT_APP_* variables continue to work.

Review type

  • Functionality: build system swap. No intended UX change, but the entire build/dev pipeline is replaced, so a visual smoke test on the deploy preview is warranted.
  • Content/copy
  • FYI
  • Needs feedback
  • Code

What's in this PR

Commit 1 — CRA → Vite migration

Build system:

  • Replace react-scripts 3.4.1 with vite@^6, @vitejs/plugin-react@^4, vite-plugin-svgr@^4, vite-tsconfig-paths@^5.
  • New vite.config.ts at project root.
  • Move public/index.html → project root (Vite convention). Replace %PUBLIC_URL%/ tokens with / and add the module entry script.
  • Drop the eslintConfig stanza in package.json (CRA artifact; real config is in .eslintrc).

Env vars:

  • Rewrite six process.env.REACT_APP_* reads as import.meta.env.REACT_APP_*.
  • Vite loads the same .env file as CRA did.

Assets:

  • Logo.tsx SVG component import switched from CRA's { ReactComponent as X } to vite-plugin-svgr's ?react suffix convention.

Cleanup:

  • Delete src/serviceWorker.ts and its import (was already unregister()-only, so no runtime behavior change).
  • Delete src/setupTests.ts (orphaned CRA/jest scaffolding).
  • Exclude *.test.{ts,tsx} from tsconfig.json compile (there is no test runner anymore; files preserved on disk for future Vitest wiring).

ESLint knock-on:

  • Add a direct eslint@^7.32.0 devDep. CRA was hoisting a compatible version transitively; removing react-scripts let an older eslint@5 bubble up, which broke config parsing.
  • Drop the now-unresolvable react-app entry from .eslintrc extends.
  • Remove the deleted src/serviceWorker.ts path from ignorePatterns.

Commit 2 — Fix 55 pre-existing lint errors unmasked by removing react-app preset

The CRA preset had been silencing real errors. Cleaned up in-place rather than deferring:

  • 40 × @typescript-eslint/no-unused-vars: set { args: "none" } in the rule config. All were unused callback params (mostly createStyles((theme) => ...) and event handlers) required by type signatures. The project-level policy is now to not enforce unused function arguments; genuine unused variables and imports are still checked.
  • 5 × @typescript-eslint/no-use-before-define: reorder the five helper function definitions so each precedes its call site. Affected: createLayerStyles, handleClose, scrollToTop, clearFiltersBtnClick, excludeUTFtext.
  • 6 × jsx-a11y/anchor-is-valid: five MUI <Link> usages that are semantically buttons now use component="button" and drop the dummy href="#" plus e.preventDefault() plumbing. The sixth already uses component={Button}. All six also get an eslint-disable-next-line comment with an explanation, because the rule inspects JSX element names and doesn't analyze MUI's component prop — the markup is correct, the rule is a false positive.
  • 3 × parser errors in the excluded *.test.tsx files: add the same glob to .eslintrc ignorePatterns so ESLint skips what the tsconfig is already skipping.

yarn lint is now clean (0 errors, 0 warnings).

What to review

  • App loads on the deploy preview with a visible map and UI (no white screen).
  • No new DevTools console errors beyond the pre-existing Airtable 403 warnings.
  • Geocoder search (the location search box) — see known issue below.
  • Previously-anchor-now-button controls still behave correctly: "Read more/less" toggle (ReadMore), "Clear filters" inline link (FiltersWarning), "Show/Hide world map" toggle (WorldRegionMap), "View image" link (EndoImageModal), neighborhood-list popover trigger (LocationLink). Visual styling should be unchanged; behavior should be unchanged.
  • yarn start runs without NODE_OPTIONS=--openssl-legacy-provider.
  • yarn build emits to build/ (unchanged output dir) so Netlify's default publish behavior still works.

Verification done locally

  • yarn build — passes (tsc --noEmit + Vite build, ~5s vs ~33s with CRA).
  • yarn start — Vite dev server ready in ~125ms, HTTP 200 on localhost:3000.
  • yarn lint — 0 errors, 0 warnings.
  • Pre-commit hook (lint-staged → eslint) — passes on staged files.

Known follow-ups (out of scope)

  1. Geocoder Node-module warning. @mapbox/mapbox-gl-geocoder (via react-map-gl-geocoder) imports Node's events module. Vite externalizes it for the browser, which shows up as a build-time warning. If the search box breaks at runtime, the fix is to swap react-map-gl-geocoder for a direct mapbox-gl-geocoder integration — planned for phase 5.
  2. Large chunk warning. Vite notes the main bundle exceeds 500KB post-minify. Code-splitting opportunities exist (route-based React.lazy, manualChunks for Mapbox/material-table) but are not part of a build-system swap.
  3. Orphaned test deps. @testing-library/*, jest-canvas-mock, jest-environment-jsdom-sixteen, @types/jest are now unused. Left in place to minimize this PR's diff; should be removed (or replaced with Vitest) when a test strategy is decided.

What's next

  • Phase 3: React 16 → 17 → 18 (createRoot, strict-mode audit).
  • Phase 4: MUI v4 → v5 + replace material-table and react-swipeable-views.
  • Phase 5: Router v5 → v6, react-query v2 → TanStack Query v5, react-map-gl 5 → 7, replace react-map-gl-geocoder.

Remove the dead IE-only msMaxTouchPoints check in isTouchEnabled
(removed from lib.dom.d.ts in TS 4+; IE11 was EOL'd in 2022 and is
already excluded by the project browserslist).
Replace react-scripts 3.4.1 with Vite 6 + @vitejs/plugin-react. This
drops the NODE_OPTIONS=--openssl-legacy-provider workaround required
for Node 17+ on CRA, and shrinks cold dev start to ~125ms and prod
build to ~5s (from ~33s).

Changes:
- Swap scripts: yarn start -> vite; yarn build -> tsc --noEmit &&
  vite build; yarn preview -> vite preview; drop eject/test.
- Replace CRA deps with vite, @vitejs/plugin-react, vite-plugin-svgr,
  vite-tsconfig-paths.
- Add vite.config.ts configuring the React and SVGR plugins, the
  REACT_APP_ env prefix (keeps .env working as-is, with VITE_ also
  accepted), port 3000, and build output to build/ (preserving the
  Netlify publish dir).
- Move public/index.html to project root. Replace %PUBLIC_URL%/ with
  / and add the Vite module entry script.
- Remove the CRA eslintConfig stanza in package.json; real config
  lives in .eslintrc.
- Add a direct eslint@^7.32.0 devDep (previously hoisted by CRA),
  drop the react-app extends that no longer resolves, and remove the
  deleted serviceWorker.ts path from ignorePatterns.
- Delete src/serviceWorker.ts and its import; it was already calling
  unregister() only, so removal is a no-op at runtime.
- Rewrite six process.env.REACT_APP_* reads as import.meta.env.
- Change Logo.tsx SVG import to use the vite-plugin-svgr ?react
  suffix.
- Update tsconfig.json: target ES2020, add vite/client and
  vite-plugin-svgr/client types, exclude *.test.ts/tsx files from
  the build since there is no test runner anymore.
- Delete src/setupTests.ts (orphaned CRA jest setup).

Known follow-ups (deliberately out of scope for this PR):
- @mapbox/mapbox-gl-geocoder imports the Node events module, which
  Vite externalizes for the browser. If the geocoder search breaks at
  runtime, it can be addressed by swapping react-map-gl-geocoder for
  a direct mapbox-gl-geocoder integration in a later phase.
- yarn lint surfaces 55 pre-existing errors across the codebase that
  were previously masked by CRA's react-app eslint preset. These are
  unrelated to the build migration and should be addressed in a
  follow-up. Pre-commit lint-staged only runs against staged files,
  so new commits are unaffected.
Removing react-scripts in the previous commit unmasked pre-existing
eslint errors that CRA's react-app preset had been silencing.

Cleanup by category:
- 40 @typescript-eslint/no-unused-vars: set { args: "none" } in the
  rule config. These were all unused callback parameters (mostly
  createStyles theme args and event/props args in handlers) required
  by library type signatures. The project-level policy is to not
  enforce unused function arguments; genuine unused variables and
  imports are still checked.
- 5 @typescript-eslint/no-use-before-define: reorder the five helper
  function definitions so each precedes its call site. Affected:
  createLayerStyles (hooks.points.ts), handleClose (SplitCrumbs.tsx),
  scrollToTop (ResultsTable.tsx), clearFiltersBtnClick
  (ResultsToolbar.tsx), excludeUTFtext (exporting.ts).
- 6 jsx-a11y/anchor-is-valid: five MUI <Link> usages that are
  semantically buttons now use component="button" and drop the dummy
  href="#" plus its e.preventDefault() plumbing. The sixth
  (MapOptionsMenu) already uses component={Button}. All six get an
  eslint-disable-next-line comment because the rule inspects JSX
  element names and doesn't analyze MUI's component prop -- the
  markup is correct, the rule is a false positive.
- 3 parser errors in excluded test files: add src/**/*.test.ts(x) to
  .eslintrc ignorePatterns. The tsconfig already excludes these from
  the build.
@ankit0504 ankit0504 marked this pull request as ready for review April 21, 2026 19:46
@abettermap
Copy link
Copy Markdown
Contributor

@ankit0504 I have upgraded other repos from CRA to Vite in the past and I fully support this, but just note that this is a pretty big change so I would like to be ready to do the smoke test, so it might not be until this weekend.

Once we get past this housekeeping/upgrade stuff though, we should be able to move a little more quickly, especially if we can coordinate a good testing cadence with @rperlin-ela.

Stay tuned and thanks again for contributing!

The airtable package references process.env at runtime, which
webpack/CRA provided but Vite does not. Defining an empty object
prevents the ReferenceError without leaking server-side env vars.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@abettermap abettermap left a comment

Choose a reason for hiding this comment

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

@ankit0504 good stuff! Just make those Link -> Button changes unless you see a reason not to, and rename that vite config file (not sure why it worked for you but not me with the .ts extension), then I'll approve.

Comment thread vite.config.mts
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ankit0504 I was getting errors starting vite. GPT suggested workarounds:

  • rename it (this worked for me): mv vite.config.ts vite.config.mts
  • or set "type": "module" in package.json. This was not advised since it's an older app, however, so let's go with the renaming of the file.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

ah i think it might have to do with running different node versions? seems like .mts is the right fix so i'll push that up!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ankit0504 while this does work, I'd prefer to just use a legit solution since there are 6 instances of this.

    <Button
      color="secondary"
      // ...other props...
    >
       some text
    </Button>

I'm not sure why I used Link for this (maybe for styling?), but it's definitely not a link so let's go with Button (here and in the other 5 spots) unless you see any reason not to.

import React, { FC } from 'react'
import { createStyles, makeStyles, Theme } from '@material-ui/core/styles'
import { ReactComponent as ProjectLogo } from '../../img/logo.svg'
import ProjectLogo from '../../img/logo.svg?react'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion, not related to this pr, but TS does have a nice way to use absolute imports so we're not stuck with lots of ../../ like this. If it's set up properly, we could do img/logo.svg?react from anywhere in the repo.

Just a wishlist thing for later though.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same comment for here and the others, regarding Link -> Button

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same

- Rename vite.config.ts to vite.config.mts for ESM compatibility
- Replace MUI Link-as-button with Button in 6 components
- Pass Airtable API key explicitly instead of relying on process.env
- Migrate remaining process.env reference to import.meta.env

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ankit0504 ankit0504 requested a review from abettermap April 27, 2026 00:35
@ankit0504
Copy link
Copy Markdown
Collaborator Author

@abettermap thanks for the feedback! i think the vite issue was because i am running a different version of node. fixed that and addressed your other comments :)

also, i think i'm missing access to the mapbox tileset and the airtable census data base. are you able to share those two things with me?

@ankit0504
Copy link
Copy Markdown
Collaborator Author

also, i think i'm missing access to the mapbox tileset and the airtable census data base. are you able to share those two things with me?

ross was able to help me with this, we're all set here! tested this PR with access to those and it's running totally fine for me locally!

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.

2 participants