Skip to content

fix(widgets): useWidget cleanup and add StrictMode test#9873

Closed
ibgreen wants to merge 18 commits into
masterfrom
codex/update-usewidget-cleanup-logic-and-add-test
Closed

fix(widgets): useWidget cleanup and add StrictMode test#9873
ibgreen wants to merge 18 commits into
masterfrom
codex/update-usewidget-cleanup-logic-and-add-test

Conversation

@ibgreen
Copy link
Copy Markdown
Collaborator

@ibgreen ibgreen commented Nov 18, 2025

Closes #9957

For https://openjs-foundation.slack.com/archives/C05JCDKHQUE/p1763426667707549

Summary

  • ensure useWidget removes widgets regardless of index so the first widget cleans up correctly
  • add a React StrictMode regression test verifying widgets are not duplicated after remount

Testing

  • yarn test node --filter react/deckgl.spec.ts

Codex Task


Note

Medium Risk
Touches the React DeckGL wrapper and useWidget lifecycle/timing (effects, deferred cleanup, onLoad ordering), which can subtly impact widget registration and app initialization behavior across React modes.

Overview
Fixes React widget lifecycle issues that could duplicate widgets or fail to clean up under React 18 StrictMode and during mount/unmount timing.

The React DeckGL wrapper now keeps a stable widgets array across remounts, passes it through view context, and defers the user onLoad callback until after React widget children have registered (via an effect + setTimeout(0)). useWidget is reworked to reuse widgets by id, avoid duplicate registration, and defer removals so StrictMode remounts can cancel cleanup.

Adds a StrictMode regression test for useWidget plus new standalone React widgets example and a manual test app to exercise JSX-children vs widgets prop modes and runtime toggling.

Written by Cursor Bugbot for commit 36a9cc5. This will update automatically on new commits. Configure here.

@ibgreen ibgreen changed the title Fix useWidget cleanup and add StrictMode test fix(widgets): useWidget cleanup and add StrictMode test Nov 18, 2025
@ibgreen ibgreen requested a review from chrisgervang November 18, 2025 11:05
@chrisgervang chrisgervang force-pushed the codex/update-usewidget-cleanup-logic-and-add-test branch from 5883356 to eed5bea Compare January 29, 2026 05:34
Comment thread modules/react/src/utils/use-widget.ts Outdated
@chrisgervang chrisgervang force-pushed the codex/update-usewidget-cleanup-logic-and-add-test branch 2 times, most recently from b03ce35 to 231db11 Compare February 2, 2026 17:15
@coveralls
Copy link
Copy Markdown

coveralls commented Feb 2, 2026

Coverage Status

coverage: 91.135% (+0.04%) from 91.091%
when pulling 36a9cc5 on codex/update-usewidget-cleanup-logic-and-add-test
into 1510545 on master.

@chrisgervang chrisgervang added this to the v9.3 milestone Feb 2, 2026
@chrisgervang chrisgervang force-pushed the codex/update-usewidget-cleanup-logic-and-add-test branch 2 times, most recently from ebc5de8 to da1600b Compare February 4, 2026 17:26
Comment thread modules/react/src/deckgl.ts
Comment thread modules/react/src/deckgl.ts
Comment thread modules/react/src/utils/use-widget.ts Outdated
Comment thread examples/get-started/react/widgets/app.jsx Outdated
ibgreen and others added 13 commits February 4, 2026 13:25
- Use stable widgetsRef in DeckGL to persist widgets array across renders
- Pass widgets array to positionChildrenUnderViews instead of creating
  new empty array on each render
- Move widget registration from render phase to useEffect for proper
  StrictMode support (mount → cleanup → mount cycle)
- Use useRef instead of useMemo for widget instance stability
- Use widgetsRef.current in deckProps to prevent widgets being reset
  on every props change (view state updates)
- Add React widgets example with StrictMode for testing

Co-Authored-By: Claude (global.anthropic.claude-opus-4-5-20251101-v1:0) <noreply@anthropic.com>
Register widget in render phase with includes check to prevent
duplicates, while keeping useEffect re-registration for StrictMode
cleanup handling. This ensures widgets are available when onLoad fires.

Co-Authored-By: Claude (global.anthropic.claude-opus-4-5-20251101-v1:0) <noreply@anthropic.com>
…support

- Render children with minimal context when deck is not initialized,
  allowing widget components to push to the widgets array during render
- Use current widgetsRef.current when creating Deck to ensure React
  widgets are included when onLoad fires
- This fixes StrictMode test where onLoad checked widgets before they
  were registered

Co-Authored-By: Claude (global.anthropic.claude-opus-4-5-20251101-v1:0) <noreply@anthropic.com>
Co-Authored-By: Claude (global.anthropic.claude-opus-4-5-20251101-v1:0) <noreply@anthropic.com>
- Revert positionChildrenUnderViews to return empty array when deck not
  initialized (fixes existing test expectations)
- Remove widgets override in deck creation to preserve user-provided
  widgets prop
- Change StrictMode test to use onAfterRender instead of onLoad since
  React widget children can only render after deck exists

Co-Authored-By: Claude (global.anthropic.claude-opus-4-5-20251101-v1:0) <noreply@anthropic.com>
Widget children can only render after deck exists and React needs
additional render cycles to position them. The test was failing
because onAfterRender was checking widgets before the widget
component had a chance to render and register.

Added a safety limit to prevent infinite waiting if widget fails
to render due to deck initialization issues.

Co-Authored-By: Claude (global.anthropic.claude-opus-4-5-20251101-v1:0) <noreply@anthropic.com>
Move the warning check for user-supplied pure-js widgets to run
BEFORE setProps so it can actually detect when those widgets will
be ignored. Previously, the check ran after setProps had already
updated deck.props.widgets, making the comparison always pass.

Co-Authored-By: Claude (global.anthropic.claude-opus-4-5-20251101-v1:0) <noreply@anthropic.com>
…dgets by ID

In React 18 StrictMode, components are unmounted and remounted to detect
side effects. This destroys refs between mount cycles, causing useWidget
to create duplicate widget instances.

The fix finds existing widgets by ID instead of relying on ref persistence:
- When the ref is null, first check if a widget with the same ID exists
- If found, reuse that widget instance instead of creating a new one
- Use ID-based check when registering to prevent duplicates in the array

Co-Authored-By: Claude (global.anthropic.claude-opus-4-5-20251101-v1:0) <noreply@anthropic.com>
…ests

- Reuse shared WebGL context from test-utils instead of creating new contexts
  to avoid "Too many active WebGL contexts" errors
- Remove nested act() call in StrictMode test cleanup to avoid
  "Should not already be working" React scheduler errors

Co-Authored-By: Claude (global.anthropic.claude-opus-4-5-20251101-v1:0) <noreply@anthropic.com>
- Intercept onLoad from Deck and defer it until after React widget
  children have registered via useWidget
- Use setTimeout(0) to escape React's commit phase, allowing callbacks
  to safely trigger state updates or nested act() calls
- Update tests to import act from 'react' instead of deprecated
  'react-dom/test-utils'
- Use shared gl context from @deck.gl/test-utils for all React tests

This ensures widgets are available on deck.props.widgets when the
onLoad callback fires, fixing the issue where onLoad fired before
React children had a chance to render and register their widgets.

Co-Authored-By: Claude (global.anthropic.claude-opus-4-5-20251101-v1:0) <noreply@anthropic.com>
- Use widget class defaultProps.id for lookup when props.id is undefined
- Defer cleanup removal with setTimeout(0) to allow StrictMode remount to cancel
- Cancel pending removals in both render and effect phases
- Fix spurious "widgets prop ignored" warning by checking for user-supplied widgets

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@chrisgervang chrisgervang force-pushed the codex/update-usewidget-cleanup-logic-and-add-test branch from 328e50d to fad89d5 Compare February 4, 2026 21:25
Comment thread modules/react/src/utils/use-widget.ts Outdated
…ample

- Add test/apps/react-widgets/ for manual StrictMode and reactive widget testing
- Update examples/get-started/react/widgets to use JSX layers (consistent with other React examples)
- Add widget theme support matching pure-js example

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Comment thread modules/react/src/utils/use-widget.ts
chrisgervang and others added 4 commits February 4, 2026 14:46
Use WeakMap keyed by widgets array instead of global Map to prevent
cross-deck interference when multiple DeckGL instances use widgets
with the same default ID (e.g., 'zoom', 'compass').

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Remove outdated Mapbox token instructions since the example uses MapLibre.
Simplify feature list to focus on the get-started purpose.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Prevents stale callback execution if component unmounts before the
setTimeout fires during fast navigation or in tests.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

timeoutId = setTimeout(() => {
onLoadRef.current?.();
}, 0);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

onLoad callback never fires in StrictMode

High Severity

The onLoadCalledRef.current flag is set to true before the setTimeout callback actually fires. In React StrictMode, effects run twice (mount → cleanup → remount). During cleanup, the timeout is cancelled via clearTimeout, but onLoadCalledRef is never reset. When the effect re-runs, the condition !onLoadCalledRef.current is false, so no new timeout is scheduled. This means the user's onLoad callback is never invoked in StrictMode.

Fix in Cursor Fix in Web

@chrisgervang
Copy link
Copy Markdown
Collaborator

Fixing this issue involves a pretty significant refactor to the core react module and deals with a lot of race conditions. I'm a little wary of merging it when users can just supply the widgets prop instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] widget appears multiple time in react StrictMode

4 participants