Export the points feature-state API from the vis entry point - #144
Merged
Conversation
`SpatialCanvas/public.ts` exports `PointsFeatureStateProvider`, `usePointsFeatureState` and the engine types, and its comment tells you to take `pointsEngine` off the renderer hook and wrap a subtree in the provider. But `src/index.ts` never re-exported any of it, and the package publishes only a "." export, so there was no deep-import route either — the documented path was reachable from inside this repo and nowhere else. The demo panels work because they import by relative path, which is why nothing caught it. Found while building the equivalent points UI in MDV, where the engine is reachable through the renderer's return type but the React state layer is not, leaving an embedder to reimplement the engine subscription instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe package entry now exports the points feature-state provider, hook, and related engine and state types. A changeset documents these public exports. No behavior changes are introduced. ChangesPoints feature-state API
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Merged
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
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.
Re-exports the points feature-state API from
packages/vis/src/index.ts.The gap
SpatialCanvas/public.tsalready exportsPointsFeatureStateProvider,usePointsFeatureStateand the points engine types, and the comment above them describes the intended integration:But
src/index.tsnever re-exported any of it, and the package publishes only a"."export, so there is no deep-import route either. In published0.6.0the sole occurrence ofusePointsFeatureStateindist/index.jsis inside an error message string.So the documented path is reachable from inside this repo and nowhere else. The demo panels work because they import by relative path, which is why nothing caught it.
What this adds
Values:
PointsFeatureStateProvider,usePointsFeatureState.Types:
PointsDataEngine,PointsLoadTarget,PointsFeatureState,PointsFeatureSelection,PointsFeatureStateProviderProps.No behaviour change — this is purely the surface an embedding application needs in order to build its own points feature UI instead of reimplementing the engine subscription.
Why it came up
Building the equivalent points layer UI in MDV. The engine itself is reachable, structurally, through
useSpatialCanvasRendererFromLayerInputs's return type — so MDV can bridge it from the chart to its layer dialog, which is what it now does. What is not reachable is the React state layer, leaving an embedder to hand-roll theuseSyncExternalStoresubscription and its derived reads. That is around 40 lines of duplicated logic plus the React Compiler'use no memo'hatch these hooks need, which is the kind of thing worth not having two of.Verification
pnpm --filter @spatialdata/vis build, then imported the builtdist/index.js: both values resolve as functions, and the types appear indist/index.d.ts.pnpm --filter @spatialdata/vis test— 149 tests, 16 files, passing.Minor changeset included, so this can go out in the next release.
Summary by CodeRabbit
New Features
Documentation