Skip to content

Clear Biome 2.x lint backlog in packages/*/src; add CI gate - #83

Merged
xinaesthete merged 4 commits into
mainfrom
claude/confident-tereshkova-57fc60
Jul 14, 2026
Merged

Clear Biome 2.x lint backlog in packages/*/src; add CI gate#83
xinaesthete merged 4 commits into
mainfrom
claude/confident-tereshkova-57fc60

Conversation

@xinaesthete

@xinaesthete xinaesthete commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

What

Clears the ~128 error / ~88 warning Biome backlog surfaced by the 1.9.4 → 2.5.3 upgrade (#82) across packages/*/src. Result: 0 errors, 40 warnings (all noExplicitAny, configured as a non-blocking warn). Build green, formatting clean, unit tests pass.

Changes

Safe autofixes (89 files): formatting, organizeImports, unused imports, optional chaining, useConst, etc.

4 regressions that Biome's "safe" autofixes introduced — each caught via pnpm build and repaired:

  • noPrototypeBuiltins rewrote Object.prototype.hasOwnProperty.callObject.hasOwn (×2), which fails under lib: ES2020. Bumped lib ES2020 → ES2022 across the 6 package tsconfigs (target stays ES2020 — Object.hasOwn is a runtime API, not syntax) and kept the modern form.
  • noTsIgnore rewrote //@ts-ignore//@ts-expect-error on two let x: any lines with no error (TS2578). Removed the now-pointless directives.
  • noUselessConstructor deleted a load-bearing constructor(...args) on LabelsBitmaskTileLayer that widens the base XRLayer signature. Restored + biome-ignore.
  • noUnusedVariables renamed a dead component Repr_Repr, breaking the PascalCase hooks heuristic. Restored + biome-ignore (dev-only <Repr/> scaffolding).

Manual fixes: typed a let footer, dropped a redundant non-null assertion, fixed a precision-loss literal (value-identical), removed dead destructured params/vars, suppressed two a11y drag-handle findings inline with TODOs.

React-hooks rules: kept useExhaustiveDependencies + useHookAtTopLevel enabled (they retain signal for future code). The 6 intentional-violation sites carry per-site // biome-ignore comments with rationale. Biome and eslint don't share a suppression syntax, so vis sites (linted by both) get both a biome-ignore and the pre-existing eslint-disable; avivatorish sites (biome-only — outside lint:react scope) get a biome-ignore alongside their existing eslint-disable. These are intentional stable-key-dep patterns and the out-of-Viv useMetadata try/catch guard.

CI: added a biome-lint job (pnpm lint:biome = biome ci packages/*/src) as an errors-only gate over the cleaned library source.

Also: zarrextra dev build fix (unrelated, folded in)

Pre-existing bug found while testing, not caused by the lint work (vite.config.ts / package.json were untouched by it; it reproduces on main).

codec-worker.js is emitted by a second vite pass (--mode codec-worker), but the default pass had emptyOutDir: true. So dev (vite build --watch) emptied dist and never ran the codec-worker pass — deleting codec-worker.js while workers.js still references it via new URL('./codec-worker.js', import.meta.url). Any consumer of zarrextra/dist after a dev run then broke, e.g. the docs dev server:

ERROR in ../packages/zarrextra/dist/workers.js
Module not found: Error: Can't resolve './codec-worker.js'
  • vite.config.ts: emptyOutDir: false in both modes.
  • build: explicit rm -rf dist so clean builds stay stale-free.
  • dev: build the codec worker once, then watch.

Verified: clean build emits codec-worker.js + .d.ts (27 files); the watch pass and subsequent rebuilds no longer delete it; docs dev server renders with no module-resolution error.

Reviewer notes

  • The CI gate is scoped to packages/*/src (the cleaned code). Demo, scripts, tests, and dev_scripts still carry ~15 biome errors and are intentionally out of scope — so pnpm lint (biome check ., whole repo) is still red. Clearing those to enable a repo-wide gate is a reasonable follow-up.
  • The lib → ES2022 bump touches the recently-upgraded TS toolchain; target is unchanged, so no downlevel-emit impact.
  • The React-hooks rules stay enabled; suppressions are per-site. Note that a site linted by both biome and eslint needs both a biome-ignore and an eslint-disable comment (no shared syntax) — see SpatialCanvasViewer.tsx.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Added automated Biome lint and formatting checks to the continuous integration pipeline.
    • Updated build workflows to clean generated output reliably and preserve codec worker assets during development and watch builds.
    • Updated supported JavaScript library definitions to ES2022 across packages.
  • Refactor
    • Improved consistency of imports, exports, formatting, type annotations, and internal property checks across the codebase.
    • Clarified layer-data handling and rendering safeguards without changing core functionality.

Clears the ~128 error / ~88 warning backlog surfaced by the Biome
1.9.4 -> 2.5.3 upgrade across packages/*/src. Result: 0 errors,
43 warnings (all noExplicitAny, configured as a non-blocking warn).

Safe autofixes (89 files): formatting, organizeImports, unused
imports, optional chaining, useConst, etc.

Reverted 4 regressions that Biome's "safe" autofixes introduced
(each caught via `pnpm build`):
- noPrototypeBuiltins rewrote Object.prototype.hasOwnProperty.call ->
  Object.hasOwn (x2). Instead of leaving it broken under lib:ES2020,
  bumped lib ES2020 -> ES2022 across the 6 package tsconfigs (target
  stays ES2020; Object.hasOwn is a runtime API, not syntax) and kept
  the Object.hasOwn form.
- noTsIgnore rewrote //@ts-ignore -> //@ts-expect-error on two
  `let x: any` lines with no error, tripping TS2578. Removed the
  now-pointless directives.
- noUselessConstructor deleted a load-bearing `constructor(...args)`
  on LabelsBitmaskTileLayer that widens the base XRLayer signature;
  restored with a biome-ignore.
- noUnusedVariables renamed a dead React component Repr -> _Repr,
  breaking the PascalCase hooks heuristic; restored + biome-ignore
  (dev-only <Repr/> scaffolding).

Manual fixes: typed a `let footer`, dropped a redundant non-null
assertion, fixed a precision-loss literal (value-identical), removed
dead destructured params/vars, and suppressed two a11y drag-handle
findings inline with TODOs.

biome.json: disabled useExhaustiveDependencies and useHookAtTopLevel.
React-hooks correctness is owned by eslint (lint:react) +
babel-plugin-react-compiler, and the flagged sites are intentional
stable-key-dep patterns already carrying eslint-disable comments.

CI: added a biome-lint job (pnpm lint:biome = `biome ci
packages/*/src`) as an errors-only gate over the cleaned library
source. Demo, scripts, and tests are intentionally out of scope
(still carry ~15 errors); this gate is narrower than `pnpm lint`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@xinaesthete, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 45 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d9d004f6-8e80-47b1-8d60-34a02ebd1bb3

📥 Commits

Reviewing files that changed from the base of the PR and between dce7d8a and cef3fed.

📒 Files selected for processing (7)
  • packages/avivatorish/package.json
  • packages/core/package.json
  • packages/core/src/parquetWasmLoader.ts
  • packages/layers/package.json
  • packages/react/package.json
  • packages/vis/package.json
  • packages/zarrextra/package.json
📝 Walkthrough

Walkthrough

The pull request adds a Biome CI job and script, updates package TypeScript targets to ES2022, and applies broad lint-driven import, export, formatting, unused-parameter, hook-suppression, and selected layer-data cleanup changes across the monorepo.

Changes

Repository linting and cleanup

Layer / File(s) Summary
Biome CI validation
.github/workflows/test.yml, package.json
Adds lint:biome and a CI job running Biome checks against package source directories.
Package source cleanup
packages/avivatorish/..., packages/core/..., packages/layers/..., packages/react/..., packages/vis/..., packages/zarrextra/...
Reorganizes imports and exports, updates several TypeScript libraries to ES2022, adds lint suppressions, and applies formatting and unused-value naming changes.
Visualization layer-data updates
packages/vis/src/SpatialCanvas/useLayerData.ts
Refactors cache invalidation and renderability guards, adds explicit picking flow for shape rendering, and preserves the existing shapes, points, labels, and image layer composition paths.
Zarr worker build wiring
packages/zarrextra/package.json, packages/zarrextra/vite.config.ts
Builds the codec worker before watch mode, clears dist before full builds, and disables automatic output-directory cleanup during multi-pass builds.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

Suggested reviewers: github-actions[bot]

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 24.14% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: clearing the Biome lint backlog in packages/*/src and adding a CI gate.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/confident-tereshkova-57fc60

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 2

🧹 Nitpick comments (3)
packages/avivatorish/src/utils.ts (1)

17-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove redundant as boolean type assertion.

Array.isArray() already returns boolean, which is directly assignable to the value is unknown[] type predicate. The as boolean assertion is unnecessary and violates the coding guideline to avoid type assertions when the type system can already express the fact.

As per coding guidelines: "Avoid type assertions (as ...) in TypeScript when a library overload, local type guard, schema parser, discriminated union, or narrower API contract can express the same fact."

♻️ Proposed fix
 function _isArray(value: unknown): value is unknown[] {
-  return Array.isArray(value) as boolean;
+  return Array.isArray(value);
 }
🤖 Prompt for 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.

In `@packages/avivatorish/src/utils.ts` around lines 17 - 19, Remove the redundant
boolean type assertion from the return statement in _isArray, returning
Array.isArray(value) directly while preserving the existing type-predicate
signature.

Source: Coding guidelines

packages/core/src/workers/points-worker.ts (1)

267-267: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove dead _hasZ variable instead of underscore-prefixing it.

_hasZ is computed but never read anywhere in scanPayloadByFeatureCodes. The caller handleScanParquetByFeatureCodes (line 336) computes its own hasZ. The rename silences the lint warning but leaves dead code — prefer deleting the line entirely.

♻️ Proposed fix
-  const _hasZ = request.axisNames.includes('z');
   const columns = [
🤖 Prompt for 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.

In `@packages/core/src/workers/points-worker.ts` at line 267, Remove the unused
_hasZ declaration from scanPayloadByFeatureCodes; do not replace or rename it,
and leave handleScanParquetByFeatureCodes’s separate hasZ calculation unchanged.
packages/vis/src/SpatialCanvas/useLayerData.ts (1)

1206-1210: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Narrow AvailableElement instead of asserting its payload type.

Use an isPointsAvailableElement guard and the existing shapes guard so elem.element narrows naturally. This also rejects mismatched config/element types rather than hiding them with as PointsElement or as ShapesElement.

As per coding guidelines, avoid type assertions when a local type guard or narrower API contract can express the same fact.

Also applies to: 1323-1351

🤖 Prompt for 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.

In `@packages/vis/src/SpatialCanvas/useLayerData.ts` around lines 1206 - 1210,
Update resolvePointsTarget to use the existing isPointsAvailableElement guard so
the resolved element and payload narrow naturally without asserting elem.element
as PointsElement. Apply the same pattern in the corresponding shapes handling
around the additional referenced range, using the existing shapes guard to
reject mismatched configuration and element types.

Source: Coding guidelines

🤖 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 `@packages/vis/src/SpatialCanvas/LayerOrderList.tsx`:
- Around line 75-82: Add keyboard-accessible reordering to the layer-order
control around the drag handlers in LayerOrderList, using focusable semantic
controls or equivalent key handling to move the targeted layer and update the
same ordering state as onDrop. Ensure assistive technologies can identify and
operate the reorder action, then remove both biome-ignore accessibility
suppressions for the drop target and drag handle.

In `@packages/vis/src/SpatialCanvas/useLayerData.ts`:
- Around line 1140-1165: Update reloadElement to trigger a rerender after
clearing caches, and use resolveLayerElement to determine the element associated
with each shapes layer before deleting its shapePrebuiltData and
shapeFillColorData. Preserve the existing cache invalidation for all element
types while ensuring the load effect runs immediately after reloadElement.

---

Nitpick comments:
In `@packages/avivatorish/src/utils.ts`:
- Around line 17-19: Remove the redundant boolean type assertion from the return
statement in _isArray, returning Array.isArray(value) directly while preserving
the existing type-predicate signature.

In `@packages/core/src/workers/points-worker.ts`:
- Line 267: Remove the unused _hasZ declaration from scanPayloadByFeatureCodes;
do not replace or rename it, and leave handleScanParquetByFeatureCodes’s
separate hasZ calculation unchanged.

In `@packages/vis/src/SpatialCanvas/useLayerData.ts`:
- Around line 1206-1210: Update resolvePointsTarget to use the existing
isPointsAvailableElement guard so the resolved element and payload narrow
naturally without asserting elem.element as PointsElement. Apply the same
pattern in the corresponding shapes handling around the additional referenced
range, using the existing shapes guard to reject mismatched configuration and
element types.
🪄 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: 523a92c4-ad7a-4bb6-995f-2bf7abc6bd89

📥 Commits

Reviewing files that changed from the base of the PR and between 43a83f9 and 1dbd2ce.

📒 Files selected for processing (101)
  • .github/workflows/test.yml
  • biome.json
  • package.json
  • packages/avivatorish/src/hooks.ts
  • packages/avivatorish/src/index.ts
  • packages/avivatorish/src/layerChannelState.ts
  • packages/avivatorish/src/state.tsx
  • packages/avivatorish/src/useChannelSelectionStats.ts
  • packages/avivatorish/src/utils.ts
  • packages/avivatorish/tsconfig.json
  • packages/core/src/index.ts
  • packages/core/src/models/VAnnDataSource.ts
  • packages/core/src/models/VPointsSource.ts
  • packages/core/src/models/VShapesSource.ts
  • packages/core/src/models/VTableSource.ts
  • packages/core/src/models/VZarrDataSource.ts
  • packages/core/src/models/index.ts
  • packages/core/src/parquetFooterStats.ts
  • packages/core/src/pointsFeatures.ts
  • packages/core/src/pointsLimits.ts
  • packages/core/src/pointsLoader.ts
  • packages/core/src/schemas/index.ts
  • packages/core/src/store/index.ts
  • packages/core/src/tooltip.ts
  • packages/core/src/transformations/index.ts
  • packages/core/src/transformations/operations.ts
  • packages/core/src/transformations/transformations.ts
  • packages/core/src/types.ts
  • packages/core/src/workers/index.ts
  • packages/core/src/workers/points-worker.ts
  • packages/core/src/workers/pointsWorkerClient.ts
  • packages/core/src/workers/pointsWorkerProtocol.ts
  • packages/core/src/workers/pointsWorkerScan.ts
  • packages/core/tsconfig.json
  • packages/layers/src/LabelsBitmaskTileLayer.ts
  • packages/layers/src/PointsLayer.ts
  • packages/layers/src/SpatialLayer.ts
  • packages/layers/src/engine/PointsDataEngine.ts
  • packages/layers/src/geoArrowStrategies.ts
  • packages/layers/src/index.ts
  • packages/layers/src/mortonTiledStrategy.ts
  • packages/layers/src/pointsBbox.ts
  • packages/layers/src/pointsFeatureCodes.ts
  • packages/layers/src/pointsFeatureColor.ts
  • packages/layers/src/pointsFeatureColorExtension.ts
  • packages/layers/src/pointsLoadPlan.ts
  • packages/layers/src/pointsLoader.ts
  • packages/layers/src/pointsLoaderAdapter.ts
  • packages/layers/src/pointsRenderStrategies.ts
  • packages/layers/src/pointsScatterLayer.ts
  • packages/layers/src/pointsTileDebug.ts
  • packages/layers/src/pointsTileLoadCallbacks.ts
  • packages/layers/src/pointsTiledDebugHooks.ts
  • packages/layers/src/preloadedScatterStrategy.ts
  • packages/layers/src/renderStack.ts
  • packages/layers/src/resolvePointsRenderResource.ts
  • packages/layers/src/shapesLayer.ts
  • packages/layers/src/spatialLayerProps.ts
  • packages/layers/tsconfig.json
  • packages/react/src/index.ts
  • packages/react/src/provider/SpatialDataProvider.tsx
  • packages/react/tsconfig.json
  • packages/vis/src/ImageView/index.tsx
  • packages/vis/src/Sketch/index.tsx
  • packages/vis/src/SpatialCanvas/ImageChannelPanel.tsx
  • packages/vis/src/SpatialCanvas/ImageLayerContext.tsx
  • packages/vis/src/SpatialCanvas/LayerOrderList.tsx
  • packages/vis/src/SpatialCanvas/PointsFeatureFilterPanel.tsx
  • packages/vis/src/SpatialCanvas/PointsFeatureState.tsx
  • packages/vis/src/SpatialCanvas/PointsLayerPanel.tsx
  • packages/vis/src/SpatialCanvas/ShapeFillColorPanel.tsx
  • packages/vis/src/SpatialCanvas/SpatialCanvasViewer.tsx
  • packages/vis/src/SpatialCanvas/SpatialFeatureTooltip.tsx
  • packages/vis/src/SpatialCanvas/SpatialViewer.tsx
  • packages/vis/src/SpatialCanvas/VivLoaderRegistry.tsx
  • packages/vis/src/SpatialCanvas/VivSpatialViewer.tsx
  • packages/vis/src/SpatialCanvas/context.tsx
  • packages/vis/src/SpatialCanvas/featureTooltipHover.ts
  • packages/vis/src/SpatialCanvas/hooks.ts
  • packages/vis/src/SpatialCanvas/imageLoaderChannelDefaults.ts
  • packages/vis/src/SpatialCanvas/index.tsx
  • packages/vis/src/SpatialCanvas/public.ts
  • packages/vis/src/SpatialCanvas/renderStackAdapters.ts
  • packages/vis/src/SpatialCanvas/renderers/imageRenderer.ts
  • packages/vis/src/SpatialCanvas/renderers/index.ts
  • packages/vis/src/SpatialCanvas/renderers/labelsRenderer.ts
  • packages/vis/src/SpatialCanvas/renderers/shapesRenderer.ts
  • packages/vis/src/SpatialCanvas/stores.ts
  • packages/vis/src/SpatialCanvas/types.ts
  • packages/vis/src/SpatialCanvas/useLayerData.ts
  • packages/vis/src/SpatialCanvas/vivImagePassthrough.ts
  • packages/vis/src/Table/index.tsx
  • packages/vis/src/Tree/index.tsx
  • packages/vis/src/index.ts
  • packages/vis/tsconfig.json
  • packages/zarrextra/src/chunkDecode.ts
  • packages/zarrextra/src/index.ts
  • packages/zarrextra/src/omeZarr.ts
  • packages/zarrextra/src/workers/codec-worker-init.ts
  • packages/zarrextra/src/workers/workerControls.ts
  • packages/zarrextra/tsconfig.json
💤 Files with no reviewable changes (1)
  • packages/layers/src/geoArrowStrategies.ts

Comment on lines +75 to +82
// biome-ignore lint/a11y/noStaticElementInteractions: drag-reorder drop target; keyboard a11y not yet implemented for this dev panel.
<div
key={id}
style={rowStyle(selectedLayerId === id)}
onDragOver={onDragOver}
onDrop={(e) => onDrop(e, id)}
>
{/* biome-ignore lint/a11y/noStaticElementInteractions: drag handle; keyboard a11y not yet implemented for this dev panel. */}

@coderabbitai coderabbitai Bot Jul 13, 2026

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.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Do not suppress the accessibility error without adding keyboard reordering.

The layer-order control is operable only through drag events on a div/span; keyboard and assistive-technology users cannot reorder layers. Add keyboard-accessible move controls or equivalent key handling, then remove these suppressions.

🤖 Prompt for 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.

In `@packages/vis/src/SpatialCanvas/LayerOrderList.tsx` around lines 75 - 82, Add
keyboard-accessible reordering to the layer-order control around the drag
handlers in LayerOrderList, using focusable semantic controls or equivalent key
handling to move the targeted layer and update the same ordering state as
onDrop. Ensure assistive technologies can identify and operate the reorder
action, then remove both biome-ignore accessibility suppressions for the drop
target and drag handle.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The general quality of these UI components is somewhat basic, this is out-of-scope to fix here.

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.

@xinaesthete, understood — makes sense to keep this out of scope for this lint-cleanup PR. Let me know if you'd like me to open a follow-up issue to track adding keyboard-accessible reordering to LayerOrderList, and I'll file it with a link back to this discussion.

Comment on lines +1140 to +1165
const reloadElement = useCallback(
(type: string, key: string) => {
const loaded = loadedDataRef.current;
if (type === 'shapes') {
loaded.shapes.delete(key);
loaded.worldBounds.delete(`shapes:${key}`);
// Clear prebuilt data for every layer that maps to this element key.
for (const [layerId, config] of Object.entries(layersRef.current)) {
if (config.type === 'shapes' && config.elementKey === key) {
loaded.shapePrebuiltData.delete(layerId);
loaded.shapeFillColorData.delete(layerId);
}
}
} else if (type === 'points') {
pointsEngine.evict(key);
loaded.worldBounds.delete(`points:${key}`);
} else if (type === 'image') {
loaded.images.delete(key);
loaded.worldBounds.delete(`image:${key}`);
} else if (type === 'labels') {
loaded.labels.delete(key);
loaded.worldBounds.delete(`labels:${key}`);
}
} else if (type === 'points') {
pointsEngine.evict(key);
loaded.worldBounds.delete(`points:${key}`);
} else if (type === 'image') {
loaded.images.delete(key);
loaded.worldBounds.delete(`image:${key}`);
} else if (type === 'labels') {
loaded.labels.delete(key);
loaded.worldBounds.delete(`labels:${key}`);
}
// The useEffect will pick up the missing data and reload
}, [pointsEngine]);
// The useEffect will pick up the missing data and reload
},
[pointsEngine]

@coderabbitai coderabbitai Bot Jul 13, 2026

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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="packages/vis/src/SpatialCanvas/useLayerData.ts"

echo "== file size =="
wc -l "$file"

echo "== outline =="
ast-grep outline "$file" --view expanded || true

echo "== search for resolveLayerElement and reload-related symbols =="
rg -n "resolveLayerElement|reloadRevision|reloadElement|useEffect\\(|loadedDataRef|shapePrebuiltData|shapeFillColorData|elementKey" "$file"

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 10960


🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="packages/vis/src/SpatialCanvas/useLayerData.ts"

echo "== lines 560-680 =="
sed -n '560,680p' "$file" | cat -n

echo "== lines 1136-1160 =="
sed -n '1136,1160p' "$file" | cat -n

echo "== lines 340-370 =="
sed -n '340,370p' "$file" | cat -n

echo "== lines 1210-1235 =="
sed -n '1210,1235p' "$file" | cat -n

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 9801


🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="packages/vis/src/SpatialCanvas/useLayerData.ts"

echo "== lines 680-910 =="
sed -n '680,910p' "$file" | cat -n

echo "== lines 910-1120 =="
sed -n '910,1120p' "$file" | cat -n

echo "== lines 1120-1188 =="
sed -n '1120,1188p' "$file" | cat -n

echo "== call sites of reloadElement =="
rg -n "reloadElement\\(" -g '!**/node_modules/**' .

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 26528


🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="packages/vis/src/SpatialCanvas/useLayerData.ts"

echo "== lines 680-910 =="
sed -n '680,910p' "$file" | cat -n

echo "== lines 910-1120 =="
sed -n '910,1120p' "$file" | cat -n

echo "== lines 1120-1188 =="
sed -n '1120,1188p' "$file" | cat -n

echo "== call sites of reloadElement =="
rg -n "reloadElement\\(" .

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 26528


🏁 Script executed:

#!/bin/bash
set -euo pipefail
file="packages/vis/src/SpatialCanvas/useLayerData.ts"

echo "== lines 475-560 =="
sed -n '475,560p' "$file" | cat -n

echo "== elementMap assignments =="
rg -n "elementMap\\.(current|set|delete)|getElementMapKey\\(" "$file"

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 5750


Schedule a rerender after clearing caches, and resolve shapes before deleting layer caches. reloadElement only mutates refs, so the load effect won’t see a changed dependency and the cleared data can stay unloaded until some unrelated update. The config.elementKey check also skips layers that fall back to resolveLayerElement via layerId, leaving stale shape prebuilt/fill-color entries behind.

🤖 Prompt for 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.

In `@packages/vis/src/SpatialCanvas/useLayerData.ts` around lines 1140 - 1165,
Update reloadElement to trigger a rerender after clearing caches, and use
resolveLayerElement to determine the element associated with each shapes layer
before deleting its shapePrebuiltData and shapeFillColorData. Preserve the
existing cache invalidation for all element types while ensuring the load effect
runs immediately after reloadElement.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This should be addressed in a subsequent pass where we'll be substantially reviewing and refactoring things around useLayerData.

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.

@xinaesthete, understood — thanks for the context. I'll leave this as-is for now and defer to the upcoming refactor pass on useLayerData. Feel free to ping this thread again if it's still relevant once that work is underway.

xinaesthete and others added 3 commits July 13, 2026 18:03
Reverts the biome.json disable of useExhaustiveDependencies and
useHookAtTopLevel. Instead of turning the rules off repo-wide (which
loses signal for future code), the 6 intentional-violation sites now
carry per-site `// biome-ignore` comments with rationale.

Biome and eslint don't share a suppression syntax, so sites linted by
both tools need both comments:
- avivatorish/* (biome-only; not in lint:react scope): biome-ignore
  added alongside the existing eslint-disable comments.
  - layerChannelState.ts useMemo (deps = [layerId] by design)
  - state.tsx useMetadata (hooks called in try/catch, out-of-Viv guard)
  - useChannelSelectionStats.ts useLayoutEffect + useEffect (stable-key deps)
- vis/SpatialCanvasViewer.tsx useMemo (linted by both): biome-ignore
  added; the eslint-disable was already present.

biome ci packages/*/src stays at 0 errors with the rules active.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`codec-worker.js` is emitted by a second vite pass (--mode codec-worker),
but the default pass had emptyOutDir: true. So `dev` (vite build --watch)
emptied dist and never ran the codec-worker pass, deleting codec-worker.js
while `workers.js` still references it via
`new URL('./codec-worker.js', import.meta.url)`.

Anything consuming zarrextra/dist after a `dev` run then failed to resolve
it — e.g. the docs dev server:
  ERROR in ../packages/zarrextra/dist/workers.js
  Module not found: Can't resolve './codec-worker.js'

- vite.config.ts: emptyOutDir: false in both modes.
- build: explicit `rm -rf dist` so clean builds stay stale-free.
- dev: build the codec worker once, then watch.

Verified: clean build emits codec-worker.js + .d.ts (27 files); the watch
pass and subsequent rebuilds no longer delete it; docs dev server renders
with no module-resolution error.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Three pre-existing bugs that broke consuming zarrextra/core from a
bundler (surfaced by the docs Docusaurus/webpack build).

1. parquetWasmLoader gated its Node-only WASM init on
   `import.meta.url.startsWith('file:')`. Bundlers rewrite import.meta.url
   to a file:// URL inside a *browser* bundle, so this was true in the
   browser too, sending us into the Node branch where vite had replaced
   node:fs/node:url/node:path with empty stubs:
     TypeError: t is not a function   (t = fileURLToPath, undefined)
   Detect the Node runtime itself instead (process.versions.node + no
   window). Correctly false in browser and in web workers.

2. Package exports maps declared only "types" + "import" with no
   "default"/"require" fallback. Any resolver using a non-import
   condition — e.g. Docusaurus' SSR/server webpack pass — got
   ERR_PACKAGE_PATH_NOT_EXPORTED:
     Can't resolve 'zarrextra/workers' / '@spatialdata/core'
   ESM-only packages (zarrextra, avivatorish, layers, react, vis) now use
   "default"; core, which really does emit CJS, gets a proper
   "require": "./dist/index.cjs".

3. core's "./workers" export pointed at ./dist/workers/index.js, which is
   never emitted (that dir holds only .d.ts) — the JS is ./dist/workers.js.
   Latent: nothing imports that subpath yet.

Verified: all 9 subpaths resolve under both import and require conditions
with existing targets; `docusaurus build` compiles clean (Server + Client);
root build, unit tests, and lint:biome all pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@xinaesthete
xinaesthete merged commit d8fa44b into main Jul 14, 2026
4 checks passed
@xinaesthete
xinaesthete deleted the claude/confident-tereshkova-57fc60 branch July 14, 2026 09:22
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.

1 participant