Viv viewstate - #2
Merged
Merged
Conversation
Contributor
Author
|
Lost momentum here a bit - possibly want to more drastically change how state management works. |
xinaesthete
force-pushed
the
viv_viewstate
branch
from
December 5, 2025 18:39
52a8462 to
4a3ed3e
Compare
xinaesthete
added a commit
that referenced
this pull request
Jul 16, 2026
* Consume Resource Resolvers in useLayerData (Step 1, all 3 increments) Make useLayerData *consume* the resolvers #85 landed unconsumed, per docs/plans/step1-consumption-tactics.md. The 17-member public surface is unchanged; useLayerData.spec.tsx stays green throughout. Net -724/+394 lines. - Inc 1 (shapes): ShapesResolver drives geometry/tooltip/fill-colour-row loads; vis-side projection memos handle the tooltip->geometry patch (coupling #1) and keep prebuilt/fill-colour lazy. Fill-colour entry is withheld until rows load so the feature-state runtime rebuilds and fill colours actually appear. - Inc 2 (images + labels): ImagesResolver/LabelsResolver consumed via getLoadedData; LabelsLoaderData retyped to LabelsChannelDefaults (tooltip is now a separate resource). Physical-size world-bounds compute kept in the hook (coupling #2). - Inc 3 (store): one SpatialEntryStore + one reconcile() commit-effect replace the per-kind driving effects (hook now has two useEffects total). Points is wrapped in a non-owning proxy so the stable PointsDataEngine the panels subscribe to survives a store rebuild on dataset swap; points row-codes/matching stay on the render-phase engine calls in getLayers (Track A). Verified: vis typecheck + build clean, full suite 549 passing, Biome gate clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * disable tsgo extension in this workspace, pending upgrade to TS7 * Document the non-owning points-resolver lifecycle + guard it with a test The store is designed to own its resolvers (subscribe + dispose), but points is owned by the stable PointsDataEngine the panels subscribe to — so the store borrows it through createNonOwningResolver (no-op dispose) to avoid clearing the engine's cache on a dataset-swap rebuild. Expand the rationale where it goes against the store's ownership grain: the two-owner problem, why the no-op is correct not just safe, why it doesn't reintroduce "points is special" in the store, alternatives rejected, and the exit condition. Add the ownership model at the construction site and note the StrictMode useMemo-subscribe caveat. Add a lifecycle test: a spatialData swap rebuilds the raster resolvers and the store, and the points cache/render-resource identity must survive it — the test that fails if the proxy ever regresses to a real dispose. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: reflect Resource Resolver Step 1 as landed + add changeset - resource-resolver-handoff.md: flip Status off "ready for implementation", add a Progress section and mark Step 0/1 landed (contracts, four resolvers, useLayerData consumption via SpatialEntryStore.reconcile); note the non-owning points proxy and the deferred render-phase points calls (Track A). - spatial-canvas-status.mdx: replace the stale "minimal ScatterplotLayer" points description with the PointsDataEngine reality; retitle useLayerData; add a Resource Resolver entry to "Recently landed"; refresh the feature/table roadmap item (tooltip/pick routing done, ping-pong remains). - Add a changeset for the vis-side resolver consumption. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Extract non-owning proxy + shapes projection out of useLayerData Intermediate decomposition pass, no behaviour change. Moves two cohesive, kind-local chunks out of the 1.6kloc hook: - resolvers/nonOwningResolver.ts — `createNonOwningResolver` + its lifecycle rationale (the store-ownership exception for points). - shapesProjection.ts — the shapes feature-state / fill-colour projection helpers and their cache-entry types (`ShapePrebuiltEntry`, `ShapeFillColorEntry`, `getStableShapeFeatureStateRuntime`, signature/serialise helpers). This is the `project()` half of ADR 0004 §4; a vis-local waypoint before Step 3 relocates it into @spatialdata/layers. useLayerData.ts drops from 1623 to 1446 lines and imports both. The shapes read path now lives in a small dedicated module, so Track B / Step 3 touch it rather than the hook. Behaviour-preserving: control-char signature separators kept byte-identical; full suite (550 tests) green, vis typecheck + build clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Review fixes: preserve explicit shape stroke; replan on element-map change Two code-review findings, verified against current code: - shapesProjection: mergeShapeFeatureStateForRender no longer clobbers an explicit per-feature strokeColorByFeatureId when a fill-by-column encoding is active. It now mirrors the fill map only when the caller has NOT set an explicit stroke override (the schema allows both together, e.g. via SpatialLayerProps). Signature unchanged and deliberately so: it already hashes the explicit stroke, which is what drives the render; dropping that term would stale the runtime when the stroke changes, and in the mirroring case the term is already empty. - useLayerData: the reconcile effect now depends on elementMapValue, so it replans when element resolution changes without layers/store changing — e.g. a coordinate system switch that makes a previously unavailable element resolvable. The map is memoised on availableElements, so no per-render churn. Skipped: wiring reconcile into reloadElement (the finding's other suggestion) — reloadElement has zero runtime callers (dead surface, per the Step 3 punchlist), so it would fix nothing observable. Verified: vis typecheck + build clean, Biome gate clean, full suite 550 passing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <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.
Make sure there's a sensible initial view state for image component when first loading/changing selection.