feat(core): track controlled props to give widgets signal on user intent#10075
Draft
chrisgervang wants to merge 11 commits intomasterfrom
Draft
feat(core): track controlled props to give widgets signal on user intent#10075chrisgervang wants to merge 11 commits intomasterfrom
chrisgervang wants to merge 11 commits intomasterfrom
Conversation
Adds `_controlledProps` tracking to `Deck` so widgets can distinguish
props the app explicitly owns from props they are free to manage.
Two entry points with different semantics:
- `setProps` (imperative): accumulates controlled keys across calls —
once a prop is set by the user it stays controlled, matching the
existing patch-update contract.
- `setPropsFromReact` (React wrapper): replaces the controlled set on
every render so only keys the user actually wrote in JSX are treated
as controlled, not defaultProps or wrapper-owned overrides.
The React wrapper now calls `setPropsFromReact(explicitProps, allProps)`,
where `explicitProps` is `props` minus `WRAPPER_OWNED_KEYS` (style,
width, height, parent, canvas, _customRender). `allProps` (forwardProps)
continues to drive rendering unchanged.
Widgets can check `deck.isControlled('viewState')` before writing a prop,
enabling safe uncontrolled-prop management without stomping on app state.
https://claude.ai/code/session_014gG6b28kG6h5H1VfUXT9zX
Makes the name generic (works for any framework wrapper, not just React) and signals internal-use via the underscore convention. https://claude.ai/code/session_014gG6b28kG6h5H1VfUXT9zX
setProps accumulates _controlledProps, so widgets calling it would incorrectly mark props as user-controlled. _setWidgetProps applies a prop patch without touching _controlledProps, keeping isControlled an accurate reflection of user intent only. Widget base class exposes updateDeckProps() as the protected helper widgets should use instead of deck.setProps. https://claude.ai/code/session_014gG6b28kG6h5H1VfUXT9zX
…iews _setPropsSnapshot no longer takes a separate allProps argument. It only receives what the user declared, and _applyProps leaves everything else in this.props untouched — so widget-managed props survive re-renders. The React wrapper now omits layers and views from the snapshot unless the user explicitly passed them as a prop or via JSX children. extractJSXLayers gains hasJSXLayers/hasJSXViews to signal when children contributed them. https://claude.ai/code/session_014gG6b28kG6h5H1VfUXT9zX
…pshot Previously, omitting a prop from JSX after having declared it would leave this.props unchanged — wrong declarative behavior. Now _setPropsSnapshot detects keys that leave _controlledProps between renders and resets them to their defaultProps values. This preserves both contracts: - Removing a prop from JSX resets it to its default (declarative) - A prop never declared by the user is left alone for widgets to manage https://claude.ai/code/session_014gG6b28kG6h5H1VfUXT9zX
deck.spec.ts: - isControlled reflects setProps declarations - _setWidgetProps does not mark props controlled - _setPropsSnapshot replaces the controlled set - dropped controlled props are reset to their default value deckgl.spec.ts: - widget-managed views survive React re-renders when user never declared views - user-declared views are not overwritten by a widget https://claude.ai/code/session_014gG6b28kG6h5H1VfUXT9zX
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
…lled
`this.setProps(props)` at line 414 was called after `props` was
reassigned to `{...defaultProps, ...props}`, so every key from
defaultProps (views, layers, widgets, viewState, controller, …) was
permanently added to `_controlledProps`, making `isControlled()` return
true for all default props even when the user never supplied them.
Fix: capture the original user-supplied props before the merge and use
`_applyProps` directly, iterating only over those original keys to seed
`_controlledProps`.
The new `Deck#isControlled` unit test covers this: it constructs Deck
without `views` and asserts `isControlled('views')` is false.
https://claude.ai/code/session_014gG6b28kG6h5H1VfUXT9zX
`_setPropsSnapshot` was called with the full `explicitProps` object, which always contains wrapper-injected keys (style, width, height, parent, canvas, onViewStateChange, onInteractionStateChange). Every one of those ended up in `_controlledProps`, causing `isControlled()` to return true for them even though the user never declared them. Fixes: - Add `onViewStateChange` and `onInteractionStateChange` to `WRAPPER_OWNED_KEYS` (they were already excluded from the user-prop filter but not from the snapshot tracking path). - Split `_setPropsSnapshot(userProps, allProps?)` so the controlled-set is built only from `userProps` (user intent), while `allProps` (including wrapper-owned keys) is what actually gets applied. - The React wrapper now strips `WRAPPER_OWNED_KEYS` before building the `userProps` argument, then passes the full `explicitProps` as `allProps` so wrapper keys are still applied on every render. Regression test: DeckGL#wrapper-owned keys are not user-controlled verifies that style/width/height/parent/canvas/onViewStateChange/ onInteractionStateChange are all non-controlled after mount. https://claude.ai/code/session_014gG6b28kG6h5H1VfUXT9zX
Three root-cause fixes for 22 failing CI tests: 1. `deck.ts` – `_setDevice` was calling `this.setProps(this.props)` which marked every merged prop (including all `defaultProps` keys) as user-controlled. Change to `this._applyProps(this.props)` so initialization does not pollute `_controlledProps`. 2. `deck.ts` – Add `_setControlledProps(keys)` so framework wrappers can replace the controlled-props set immediately after constructing a Deck instance, correcting the window where wrapper-owned keys (width, height, parent, canvas …) were erroneously counted as user-declared. 3. `deckgl.ts` (React wrapper) – After `createDeckInstance`, call `_setControlledProps` with only the user-declared keys (WRAPPER_OWNED_KEYS excluded). This ensures `isControlled` correctly returns `false` for wrapper-owned props before `onLoad` fires, letting widgets safely manage un-declared props on initial mount. 4. `deck-utils.ts` (Mapbox) – `getViewport` / `view.makeViewport` returns `null` when canvas dimensions are 0 (JSDOM test environment). Guard both `drawLayer` and `drawLayerGroup` with an early return when `currentViewport` is null, preventing a `TypeError: Cannot read properties of null (reading 'id')` in `layers-pass.ts`. https://claude.ai/code/session_014gG6b28kG6h5H1VfUXT9zX
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.
Summary
This PR introduces a controlled-props tracking mechanism so that widgets can safely manage props the application has left unset, without stomping on props the user has explicitly declared.
Problem
The React wrapper called
deck.setProps(forwardProps)on every render, passing a fully merged snapshot that included framework defaults and wrapper-owned overrides (e.g.views: null,layers: []). This meant any prop a widget wrote would be overwritten on the next re-render — even if the user never declared that prop in JSX.Separately, there was no way for a widget to know whether a given prop was user-controlled or available for the widget to own.
Solution
Deck._controlledProps: Set<string>Tracks which props were explicitly declared by the user.
Deck.setProps(patch — unchanged contract)Accumulates into
_controlledProps. Once a prop is declared imperatively, it stays controlled.Deck._setPropsSnapshot(userProps, allProps?)(framework wrappers)Replaces
_controlledPropson every call so it always reflects the current declaration. Props the user never declared are left untouched, allowing widgets to manage them. Props that were declared last render but are absent this render are reset to theirdefaultPropsvalue - so removing a prop from JSX behaves identically to before.userPropsis what drives_controlledPropsallProps(defaults touserProps) is what gets passed to_applyPropsDeck.isControlled(key)Public query for widgets. Returns
trueonly for props the user declared.Deck._setWidgetProps(props)Applies a prop patch without touching
_controlledProps. Widgets must use this (viaWidget.updateDeckProps) instead ofdeck.setPropsto avoid incorrectly marking props as user-controlled.Widget.updateDeckProps(props)Protected helper on the
Widgetbase class. The correct way for any widget to write deck props.React wrapper (
deckgl.ts)Now calls
deck._setPropsSnapshot(userProps, explicitProps)instead ofdeck.setProps(forwardProps).explicitPropscontains wrapper-owned props (always applied) plus only what the user actually wrote in JSX;userPropsisexplicitPropsminusWRAPPER_OWNED_KEYS(style, width, height, parent, canvas, onViewStateChange, onInteractionStateChange), so_controlledPropsreflects only what the user declared.layersandviewsare included in both only when the user declared them or JSX children contributed them —extractJSXLayersgainshasJSXLayers/hasJSXViewsfor this. The reset-on-drop logic in_setPropsSnapshotensures that if the user stops declaring one of these, it is still reset to its default.Widget usage pattern (e.g. SplitterWidget)
On re-render, if the user never wrote
viewsin JSX,_setPropsSnapshotleavesthis.props.viewsuntouched. The widget’s views survive._controlledPropseffectsetProps_setPropsSnapshotupdateDeckProps→_setWidgetPropsNote
Medium Risk
Changes Deck/DeckGL prop-application semantics and introduces new internal update paths; regressions could surface in React wrappers or widget-driven props if controlled/uncontrolled detection is wrong. Scope is moderate and covered by new unit tests, with minimal security impact.
Overview
Adds controlled-props tracking to
Deckso widgets can detect whether a prop was explicitly set by the app via newdeck.isControlled(key).Updates prop application to distinguish user patch updates (
setPropsaccumulates controlled keys) vs framework snapshot updates (_setPropsSnapshotreplaces the controlled set and resets dropped keys todefaultProps), plus a widget-safe path (_setWidgetProps, exposed asWidget.updateDeckProps) that won’t mark props as user-controlled.Adjusts the React
DeckGLwrapper to snapshot only user-declared keys (excluding wrapper-owned overrides and omittinglayers/viewsunless explicitly provided/derived from JSX), and adds guards in Mapbox interleaved rendering to skip a render cycle when viewport dimensions are 0. Adds tests covering controlled-props behavior and widget-managed props surviving React re-renders.Written by Cursor Bugbot for commit f7f93bd. This will update automatically on new commits. Configure here.