chore(ailab): migrate to vite + vitest + typescript toolchain#73010
chore(ailab): migrate to vite + vitest + typescript toolchain#73010sanchitmalhotra126 wants to merge 1 commit into
Conversation
Follow-up #1 to the mechanical migration (#72976). Replace the package's webpack + babel + jest toolchain with the monorepo-standard vite + vitest + tsc setup, restoring a green build/typecheck and wiring the package into the turbo pipeline. No consumer rewiring: apps/ still pulls npm @code-dot-org/ml-playground, so nothing in the studio build changes yet. Build: a vite library build emits dist/index.{mjs,cjs,d.ts}. peerDependencies (react, react-dom, redux, react-redux) are externalized so the consumer's single instances are used; everything else is bundled. The five UI images are inlined as data-URIs (no runtime path), and the 132 dataset files are emitted to dist/assets/datasets/ for the host to serve. A dev-only middleware serves them at /datasets/ for the standalone harness (index.html moved to the package root as the vite entry). Deleted the webpack __webpack_public_path__ machinery (setPublicPath.ts): with images inlined, setAssetPath now governs only the runtime dataset URLs. Tests: jest -> vitest, 108 passing. Lint/prettier adopt the shared flat eslint + prettier config; the .test.js suite gets vitest globals. The migrated source predates some shared-config strictness. Relaxed in this package with a documented, tracked cleanup follow-up: tsconfig verbatimModuleSyntax + noUnusedLocals/Parameters off; eslint import-x/no-cycle, no-unused-vars, no-explicit-any, and two react/papaparse CJS-interop false-positives off. The one genuine type error (a papaparse parse() overload) is fixed properly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
A friendly pass over the vite + vitest + typescript migration 👋
Thanks for taking this on — toolchain swaps are fiddly, thankless work, and the overall shape here is really solid. I went through it three times wearing different hats (Vite/bundling, Vitest, TypeScript) and wanted to share notes in the spirit of "here's what I'd polish with you," not a gate. I've left inline comments with concrete, applyable suggestions wherever I could.
Tests — nicely done. I actually ran the suite (vitest --run): 108 passing across 13 files, clean exit, no hangs. I also poked it by injecting a deliberately-failing assertion to confirm the tests genuinely assert (they do). The only changes to the *.test.js files are import reordering — you carried the behavior over faithfully, which is exactly what you want in a migration like this. Lovely and disciplined.
TypeScript — also in good shape. strict is still on, the typecheck is genuinely green, and the papaparse fix is correct: the original error was real (the non-literal download flag matched no overload, and it only compiled before because papaparse was an untyped phantom dep), and your branch split lines up with the real @types/papaparse overloads with no casts. I re-enabled the deferred tsconfig flags in a throwaway copy just to be sure nothing scary was hiding — it's all cosmetic (≈111 import type-style + 16 unused bindings), no real bugs. The deferral-plus-follow-up plan reads as very reasonable.
A few Vite details I'd love to look at together (all inline, with fixes):
- The "five UI images are small" comment isn't quite holding up in practice — about 626 KB of base64 is landing in the bundle. The
oceanspackage has a nice pattern we can borrow. vite buildis exiting 0 even though the dts step logs a couple ofTS6307errors — easy one-line fix, and I left the exact suggestion.- The dev
/datasets/middleware can be walked out of its directory with../— dev-only, but a quick guard closes it. - A tiny stale
lodashmention in a comment (it rode along from oceans).
None of these are a knock on the work — they're exactly the kind of thing that's easy to miss in a big mechanical swap. Happy to pair on any of them!
🤖 Written by Claude Code.
| // Library mode inlines imported assets as data URIs (it cannot know the | ||
| // deploy base URL). The five UI images are small, so this is exactly what | ||
| // we want — they ride along in the JS with no runtime path dependency. | ||
| lib: { | ||
| entry: ['src/index.tsx'], | ||
| name: 'ailab', | ||
| formats: ['es', 'cjs'], | ||
| }, |
There was a problem hiding this comment.
One thing I noticed when I built this locally: the comment says the five UI images are small, but a few are actually pretty hefty once inlined. dist/index.mjs comes out around 1,430 KB, and ~626 KB of that is base64 image data — blue-scanner.png (293 KB), results-background-light.jpg (117 KB), and ai-bot-border.png (47 KB). Library mode always inlines (it can't know the deploy URL, and it ignores assetsInlineLimit), so that base64 gets parsed on every page load and can't be cached separately. The old webpack setup kept these as separate cacheable files.
The nice part is that oceans already solves exactly this — it emitFiles its large binary instead of inlining. We could borrow that pattern here: import the big images with ?url and emit them through the same emitDatasets plugin you've already written, then resolve their path off setAssetPath like the datasets do. If that feels like more than you want in this PR, even just updating the comment so the next person isn't surprised would be a win. Totally workable either way — happy to pair on it.
🤖 Written by Claude Code.
| emitDatasets(), | ||
| devDatasets(), | ||
| // Emit `.d.ts` from the library entry. | ||
| dts({tsconfigPath: './tsconfig.app.json', entryRoot: 'src'}), |
There was a problem hiding this comment.
Small one I bumped into when running the build: vite-plugin-dts logs a couple of TS6307 errors here — src/datasetManifest.ts:1 imports ../public/datasets-manifest.json and src/i18n.ts:3 imports ../i18n/mlPlayground.json, both living outside this tsconfig's include: ["src"]. The slightly sneaky part is that vite build still exits 0, so CI stays green even though the dts step is unhappy. It works today, but it's the kind of thing that could quietly break the published types after a future plugin bump. Good news — it's a one-liner; I left the exact fix as a suggestion over on tsconfig.app.json. 🙂
🤖 Written by Claude Code.
| "@public/*": ["public/*"] | ||
| } | ||
| }, | ||
| "include": ["src"] |
There was a problem hiding this comment.
Here's the fix for those TS6307 dts errors I mentioned on vite.config.ts:72 — just teaching the tsconfig about the two JSON files that get imported from outside src. Should be safe to apply directly:
| "include": ["src"] | |
| "include": ["src", "public/datasets-manifest.json", "i18n/mlPlayground.json"] |
🤖 Written by Claude Code.
| const file = path.join(DATASETS_SRC, req.url.slice(prefix.length)); | ||
| if (fs.existsSync(file)) { | ||
| req.url = `/@fs${file}`; | ||
| } |
There was a problem hiding this comment.
One to tuck away for safety: because this does path.join(DATASETS_SRC, req.url…) on the raw URL, a request like GET /datasets/../../../../etc/passwd can normalize its way out of the datasets folder and then get served via /@fs. It's only the dev server on localhost, so the real-world risk is low — but it's a cheap one to close, and a nice habit to build in. Decoding, resolving, and checking the path stays under DATASETS_SRC does it:
| const file = path.join(DATASETS_SRC, req.url.slice(prefix.length)); | |
| if (fs.existsSync(file)) { | |
| req.url = `/@fs${file}`; | |
| } | |
| const rel = decodeURIComponent(req.url.slice(prefix.length)); | |
| const file = path.resolve(DATASETS_SRC, rel); | |
| if (file.startsWith(DATASETS_SRC + path.sep) && fs.existsSync(file)) { | |
| req.url = `/@fs${file}`; | |
| } |
🤖 Written by Claude Code.
| // Externalize peerDependencies only (react, react-dom, redux, react-redux, | ||
| // lodash) so the consumer's single instances are used at runtime; bundle |
There was a problem hiding this comment.
Tiny nit, no worries at all: this comment mentions lodash as an externalized peer dep, but it looks like it rode along from the oceans config — this package's peerDeps are just react/react-dom/redux/react-redux, and lodash isn't imported here. Trimming it keeps the comment honest for the next reader:
| // Externalize peerDependencies only (react, react-dom, redux, react-redux, | |
| // lodash) so the consumer's single instances are used at runtime; bundle | |
| // Externalize peerDependencies only (react, react-dom, redux, | |
| // react-redux) so the consumer's single instances are used at runtime; bundle |
🤖 Written by Claude Code.
Why
Follow-up #1 to the mechanical migration (#72976), which inlined
@code-dot-org/ml-playgroundintofrontend/packages/labs/ailab/but deliberately left its standalone webpack + babel + jest toolchain in place (and therefore out of the turbobuild/typecheckpipeline).This PR swaps that toolchain for the monorepo-standard vite + vitest + tsc setup (mirroring
frontend/packages/labs/oceans), restoring a greenbuildandtypecheckand wiring the package into turbo so CI sees it.What this PR does
dist/index.{mjs,cjs,d.ts}(replacing the old webpack multi-entry output).package.jsongainsexports/main/module/typespointing atdist/, plusbuild/dev/typecheckscripts so the package joins the turbo pipeline.react,react-dom,redux,react-reduxmove topeerDependenciesand are externalized (vite-plugin-externalize-deps), so the consumer's single instances are used at runtime — preserving the redux "one instance per page" invariant the old webpackexternalsdocumented. Everything else (chart.js, ml-knn, papaparse, messageformat, …) is bundled.dist/assets/datasets/by a small rollup plugin for the host to serve; a dev-only middleware serves them at/datasets/for the standalone harness.index.htmlmoved to the package root (vite's dev entry);indexDev.tsxis the dev harness.setAssetPathsimplification — deletedsetPublicPath.tsand the__webpack_public_path__machinery. With images inlined, nothing needs a webpack public path;setAssetPathnow governs only the runtime dataset-fetch base, and its call-ordering contract relaxes from "before importing the bundle" to "beforeinitAll().".test.jsfiles get vitest globals via the shared config.eslint.config.mjs+ prettier config; deleted.eslintrc.js,babel.config.json, the local prettierindex.mjs,webpack.config.js,src/indexProd.tsx.Decisions worth a look
tsconfig.app.json:verbatimModuleSyntax,noUnusedLocals,noUnusedParametersoff (~110import type+ ~16 unused-binding violations).eslint.config.mjs:import-x/no-cycle(real store↔index↔component cycles),@typescript-eslint/no-unused-vars,@typescript-eslint/no-explicit-any, and twoimport-xCJS-default-interop false positives on react/papaparse.import-x/orderstays on (auto-fixed across the source — most of the source diff is this reordering).papaparseparse()overload mismatch (the source importspapaparsedirectly but onlyreact-papaparsewas declared). Fixed properly: declarepapaparse+@types/papaparse, drop the deadreact-papaparseshim, and split the call by thedownloadflag so each branch matches a real overload.What this PR does NOT do
apps/still imports the npm@code-dot-org/ml-playgroundand the Gruntfile still copies datasets from it. Nothing in the studio build consumes this package yet → near-zero risk. RepointingAilabView.tsx+ the Gruntfile to@code-dot-org/ailab(and sequencing the frontend build before the apps build) is follow-up store sprite.value again, use it properly in setSpriteSize #2.Test plan
turbo run build typecheck test lint prettier --filter=@code-dot-org/ailabgreen (verified locally — 8/8 tasks).yarn workspace @code-dot-org/ailab devserves the standalone harness; datasets load and AI-bot images render (verified locally:/datasets/*.csv200 via middleware, images inlined).dist/containsindex.{mjs,cjs,d.ts}exportinginitAll/instructionsDismissed/setAssetPath, plusassets/datasets/(132 files).apps/still on npm@code-dot-org/ml-playground.Follow-ups
apps/at the workspace package; ensure the frontendbuildruns before the apps Grunt copy.playwright+@axe-core/playwrightin the catalog, atest:ui:citurbo task). Tracked for a later update.assetPathintoinitAll(...)options to retire theglobalThiswrite entirely (kills the last ordering caveat).🤖 Generated with Claude Code