Enable React Compiler for @spatialdata/react and @spatialdata/vis, add scoped ESLint - #73
Conversation
Biome lacks the React Compiler / Rules-of-React analysis, so add a
narrowly-scoped ESLint setup (eslint-plugin-react-hooks v7) that runs only
against the React-shipping packages via `pnpm lint:react`. Biome remains the
primary linter.
- eslint.config.mjs: flat config, react-hooks recommended ruleset, scoped to
packages/{react,vis}/src.
- package.json: add eslint, eslint-plugin-react-hooks, @typescript-eslint/parser
devDeps and the lint:react script.
- test.yml: add a non-blocking `react-lint` CI job (existing backlog of
findings); flip continue-on-error once cleared to make it required.
Fix the low-risk findings:
- Tree: resolve the display value before returning JSX so a throw from
toJSON() is actually caught (JSX is lazy; try/catch around the return is not).
- SpatialCanvasViewer: document the two intentional granular dep arrays.
useLayerData returns a fresh object each render, so depending on `layerData`
would re-run the memo/effect every render; the stable members are the correct
deps.
The remaining refs / set-state-in-effect findings in the canvas code are left
for the tooltip-performance follow-up.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
.claude/launch.json with a `vis-demo` entry (pnpm --filter @spatialdata/vis dev:demo on port 5173) for previewing the visualization demo. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 52 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR integrates the React Compiler via ChangesReact Compiler Integration and Lint Enforcement
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Job-level continue-on-error kept the overall workflow green but still reported the react-lint check itself as failed, which trips CI-failure monitors and reads as a broken required check. Move continue-on-error to the lint step so the job concludes green while still running eslint; findings remain visible in the step log. Remove it to make the check a required gate once the lint backlog is cleared. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/launch.json:
- Around line 4-9: The vis-demo entry in launch.json is not a valid VS Code
debug configuration because it omits the required type/request fields and uses
the wrong schema version. Update the launch configuration to use version 0.2.0
and add an appropriate node-based launch setup for the pnpm command in the
vis-demo entry, including console: "integratedTerminal" if it should run in the
terminal. Remove port 5173 unless this config is explicitly attaching to a
debugger, and if browser debugging is needed, split that into a separate config
rather than keeping it in the same launch profile.
In @.github/workflows/test.yml:
- Around line 16-25: The react-lint job is using broader default token access
and checkout credentials than it needs. Add a minimal permissions block to the
job so it only has read access, and update the actions/checkout step to disable
persisted credentials. Keep the changes scoped to the react-lint job in the
workflow so the job still runs normally while reducing token exposure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b6aff3f8-32ed-4b61-b84b-d6ef090da5c5
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
.claude/launch.json.github/workflows/test.ymleslint.config.mjspackage.jsonpackages/react/package.jsonpackages/react/vite.config.tspackages/vis/package.jsonpackages/vis/src/SpatialCanvas/SpatialCanvasViewer.tsxpackages/vis/src/Tree/index.tsxpackages/vis/vite.config.demo.tspackages/vis/vite.config.tspnpm-workspace.yamlvite.config.base.ts
Add a minimal `permissions: contents: read` block and set `persist-credentials: false` on checkout for the react-lint job, per CodeRabbit/zizmor review. The lint job only needs read access; this limits token blast radius and avoids persisting checkout credentials. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
What
Enables the React Compiler (
babel-plugin-react-compiler1.0) for the two React-shipping packages —@spatialdata/reactand@spatialdata/vis— and adds a narrowly-scoped ESLint setup to surface the Rules-of-React analysis Biome doesn't cover.Why
The compiler auto-memoizes components/hooks, removing manual
useMemo/useCallbackchurn and reducing re-renders. We're on React 19, so it uses the built-inreact/compiler-runtime(no runtime shim). Biome has no equivalent to the compiler's Rules-of-React linting, so a thin ESLint layer gives that feedback without replacing Biome as the primary linter.Changes
React Compiler wiring
@vitejs/plugin-reactv6 dropped the oldbabeloption, so the compiler is wired via the plugin's exportedreactCompilerPreset()+@rolldown/plugin-babelas a separate plugin.vite.config.base.ts: opt-inreactCompilerflag ondefineViteConfig— enabled only forreact/vis(the other packages are data/zarr/deck.gl, not React component libs, so they don't pay the cost or carry the dep).react/compiler-runtimeto the Rollupexternallist — without it the compiler runtime gets bundled into the published libs (the react bundle ballooned with React internals inlined); externalizing keeps React a single peer dep.packages/vis/vite.config.demo.ts: same wiring for the demo.Scoped ESLint (Biome stays primary)
eslint.config.mjs: flat config running theeslint-plugin-react-hooksv7 recommended ruleset (includes the granular React Compiler diagnostics), scoped topackages/{react,vis}/src.pnpm lint:reactscript; non-blockingreact-lintCI job intest.yml(there's an existing backlog of findings — flipcontinue-on-errortofalseonce cleared to make it required).Low-risk lint fixes
Tree: resolve the display value before returning JSX so a throw fromtoJSON()is actually caught (JSX is lazy; the old try/catch around the return never caught render errors). Behavior preserved.SpatialCanvasViewer: documented two intentional granular dep arrays (useLayerDatareturns a fresh object each render, so depending onlayerDatawould re-run the memo/effect every render).Tooling
.claude/launch.json:vis-demolaunch config used to verify the change.Verification
react-compiler-healthcheck: 100% — 3/3 (react) and 30/30 (vis) components compile, no bailouts, no incompatible libraries.visdemo (compiler-on confirmed in the served bundle:SpatialCanvascompiled with 43 memo slots) against a real Xenium HTJ2K dataset: store parses, image renders via viv/deck.gl, layer auto-selection works, hover tooltips resolve cells correctly — zero console errors (notably no "maximum update depth").Notes for reviewers
@spatialdata/react/@spatialdata/vispackages — consider whether a patch changeset is warranted before release.react-hooks/refs×10,set-state-in-effect×9), all in the SpatialCanvas/tooltip hot path — pre-existing, not compiler regressions (the healthcheck still compiles them). These overlap with known hover-tooltip readPixels sluggishness and are tracked for a dedicated pass; the CI job stays non-blocking until then.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores
Documentation