Skip to content

Commit fad89d5

Browse files
chrisgervangclaude
andcommitted
fix(react): Fix StrictMode widget cleanup with deferred removal
- 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>
1 parent 3d8cbb3 commit fad89d5

1 file changed

Lines changed: 46 additions & 12 deletions

File tree

modules/react/src/utils/use-widget.ts

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,13 @@
44

55
import {useContext, useRef, useEffect} from 'react';
66
import {DeckGlContext} from './deckgl-context';
7-
import {log, Widget, type WidgetProps, _deepEqual as deepEqual} from '@deck.gl/core';
7+
import {log, Widget, type WidgetProps} from '@deck.gl/core';
8+
9+
// Track pending removals by widget ID so they can be cancelled on remount
10+
const pendingRemovals = new Map<string, ReturnType<typeof setTimeout>>();
811

912
export function useWidget<WidgetT extends Widget, WidgetPropsT extends WidgetProps>(
10-
WidgetClass: {new (props_: WidgetPropsT): WidgetT},
13+
WidgetClass: {new (props_: WidgetPropsT): WidgetT} & {defaultProps?: {id?: string}},
1114
props: WidgetPropsT
1215
): WidgetT {
1316
const context = useContext(DeckGlContext);
@@ -18,9 +21,14 @@ export function useWidget<WidgetT extends Widget, WidgetPropsT extends WidgetPro
1821
// Instead, find existing widgets by ID to handle StrictMode's double-mount.
1922
const widgetRef = useRef<WidgetT | null>(null);
2023

24+
// Determine the widget ID: use props.id if provided, otherwise fall back to the class default
25+
const widgetId = props.id ?? WidgetClass.defaultProps?.id;
26+
2127
if (!widgetRef.current) {
2228
// Check if a widget with this ID already exists (from StrictMode's first mount)
23-
const existingWidget = widgets?.find(w => w.id === props.id) as WidgetT | undefined;
29+
const existingWidget = widgetId
30+
? (widgets?.find(w => w.id === widgetId) as WidgetT | undefined)
31+
: undefined;
2432
if (existingWidget) {
2533
// Reuse the existing widget instance
2634
widgetRef.current = existingWidget;
@@ -31,6 +39,13 @@ export function useWidget<WidgetT extends Widget, WidgetPropsT extends WidgetPro
3139
}
3240
const widget = widgetRef.current;
3341

42+
// Cancel any pending removal for this widget (handles StrictMode remount)
43+
const pendingRemoval = pendingRemovals.get(widget.id);
44+
if (pendingRemoval) {
45+
clearTimeout(pendingRemoval);
46+
pendingRemovals.delete(widget.id);
47+
}
48+
3449
// Register widget during render for immediate availability (needed for onLoad callbacks)
3550
// Check by ID to prevent duplicates in StrictMode where refs are recreated
3651
const existingInArray = widgets?.find(w => w.id === widget.id);
@@ -42,28 +57,47 @@ export function useWidget<WidgetT extends Widget, WidgetPropsT extends WidgetPro
4257
widget.setProps(props);
4358

4459
useEffect(() => {
60+
// Cancel any pending removal for this widget (handles StrictMode effect remount).
61+
// This must be in useEffect because StrictMode re-runs effects without re-rendering,
62+
// so the render-phase cancellation won't run between cleanup and effect re-run.
63+
const pendingRemovalInEffect = pendingRemovals.get(widget.id);
64+
if (pendingRemovalInEffect) {
65+
clearTimeout(pendingRemovalInEffect);
66+
pendingRemovals.delete(widget.id);
67+
}
68+
4569
// Re-register widget if cleanup removed it (handles StrictMode remount after cleanup)
4670
if (widgets && !widgets.includes(widget)) {
4771
widgets.push(widget);
4872
}
4973

50-
// Warn if the user supplied a pure-js widget, since it will be ignored
51-
// Check BEFORE setProps so we can compare the user's widgets with our React widgets
74+
// Warn if the user supplied a pure-js widget via the widgets prop, since it will be ignored.
75+
// Only warn if there are widgets in deck.props.widgets that aren't in our managed array.
76+
// This avoids false positives from normal React sync timing differences.
5277
const internalWidgets = deck?.props.widgets;
53-
if (widgets?.length && internalWidgets?.length && !deepEqual(internalWidgets, widgets, 1)) {
78+
const hasUserSuppliedWidgets = internalWidgets?.some(w => !widgets?.includes(w));
79+
if (widgets?.length && hasUserSuppliedWidgets) {
5480
log.warn('"widgets" prop will be ignored because React widgets are in use.')();
5581
}
5682

5783
// Sync widgets to deck
5884
deck?.setProps({widgets: widgets ? [...widgets] : []});
5985

6086
return () => {
61-
// Remove widget from context when it is unmounted
62-
const index = widgets?.indexOf(widget);
63-
if (typeof index === 'number' && index !== -1) {
64-
widgets?.splice(index, 1);
65-
deck?.setProps({widgets: widgets ? [...widgets] : []});
66-
}
87+
// Defer widget removal to allow StrictMode remount to cancel it.
88+
// In StrictMode, React unmounts and remounts effects to detect side effects.
89+
// If we remove immediately, the remounted component can't find the widget.
90+
// By deferring, we give the remount a chance to cancel the removal.
91+
const id = widget.id;
92+
const timeoutId = setTimeout(() => {
93+
pendingRemovals.delete(id);
94+
const index = widgets?.findIndex(w => w.id === id);
95+
if (typeof index === 'number' && index !== -1) {
96+
widgets?.splice(index, 1);
97+
deck?.setProps({widgets: widgets ? [...widgets] : []});
98+
}
99+
}, 0);
100+
pendingRemovals.set(id, timeoutId);
67101
};
68102
}, [widgets, deck, widget]);
69103

0 commit comments

Comments
 (0)