Skip to content

Commit 8b2ad21

Browse files
paustintpaustint
authored
Merge pull request #1847 from jetstreamapp/fix/state-update-issue
fix: maximum depth exceeded state update
2 parents 3513a1b + ba00a92 commit 8b2ad21

6 files changed

Lines changed: 45 additions & 172 deletions

File tree

‎libs/features/anon-apex/src/AnonymousApex.tsx‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,6 @@ export const AnonymousApex: FunctionComponent<AnonymousApexProps> = () => {
118118
}
119119
}, [historyItems]);
120120

121-
useEffect(() => {
122-
setVisibleResults(results || '');
123-
}, [results]);
124-
125121
useEffect(() => {
126122
let currResults = results.split('\n');
127123
// remove non user-debug lines

‎libs/ui/src/lib/form/combobox/ComboboxWithGroupedItems.tsx‎

Lines changed: 7 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { focusElementFromRefWhenAvailable, getFlattenedListItemsById, menuItemSe
22
import { multiWordObjectFilter, NOOP } from '@jetstream/shared/utils';
33
import { ListItem, ListItemGroup, Maybe } from '@jetstream/types';
44
import isNumber from 'lodash/isNumber';
5-
import { createRef, forwardRef, useCallback, useEffect, useImperativeHandle, useRef, useState } from 'react';
5+
import { createRef, forwardRef, useCallback, useEffect, useImperativeHandle, useMemo, useRef, useState } from 'react';
66
import { Combobox, ComboboxPropsRef, ComboboxSharedProps } from './Combobox';
77
import { ComboboxListItem, ComboboxListItemProps } from './ComboboxListItem';
88
import { ComboboxListItemGroup } from './ComboboxListItemGroup';
@@ -65,24 +65,13 @@ export const ComboboxWithGroupedItems = forwardRef<ComboboxWithGroupedItemsRef,
6565
const comboboxRef = useRef<ComboboxPropsRef>(null);
6666
const [filterTextNonDebounced, setFilterText] = useState<string>('');
6767
const filterText = useDebounce(filterTextNonDebounced, 300);
68-
const [selectedItem, setSelectedItem] = useState<Maybe<ListItem>>(() =>
69-
selectedItemId ? groups.flatMap((group) => group.items).find((item) => item.id === selectedItemId) : null,
70-
);
7168
const [visibleItems, setVisibleItems] = useState(groups);
72-
const [selectedItemLabel, setSelectedItemLabel] = useState<string | null>(() => {
73-
if (selectedItem) {
74-
const selectedItem = groups.flatMap((group) => group.items).find((item) => item.id === selectedItemId);
75-
return selectedItem ? selectedItemLabelFn(selectedItem) : null;
76-
}
77-
return null;
78-
});
79-
const [selectedItemTitle, setSelectedItemTitle] = useState<string | null>(() => {
80-
if (selectedItem) {
81-
const selectedItem = groups.flatMap((group) => group.items).find((item) => item.id === selectedItemId);
82-
return selectedItem ? selectedItemLabelFn(selectedItem) : null;
83-
}
84-
return null;
85-
});
69+
// Derived during render rather than mirrored into state with effects — see ComboboxWithItems
70+
// for why the effect-synced version risked "Maximum update depth exceeded".
71+
const itemsById = useMemo(() => getFlattenedListItemsById(groups.flatMap((group) => group.items)), [groups]);
72+
const selectedItem: Maybe<ListItem> = selectedItemId ? itemsById[selectedItemId] : null;
73+
const selectedItemLabel = selectedItem ? selectedItemLabelFn(selectedItem) : null;
74+
const selectedItemTitle = selectedItem ? selectedItemTitleFn(selectedItem) || '' : null;
8675
const [focusedIndex, setFocusedIndex] = useState<number | null>(null);
8776
const refs = visibleItems.flatMap((group) => group.items).map((value) => createRef<HTMLLIElement>());
8877

@@ -97,24 +86,6 @@ export const ComboboxWithGroupedItems = forwardRef<ComboboxWithGroupedItemsRef,
9786
[],
9887
);
9988

100-
useEffect(() => {
101-
if (selectedItemId) {
102-
setSelectedItem(getFlattenedListItemsById(groups.flatMap((group) => group.items))[selectedItemId]);
103-
} else {
104-
setSelectedItem(null);
105-
}
106-
}, [groups, selectedItemId]);
107-
108-
useEffect(() => {
109-
if (selectedItem) {
110-
setSelectedItemLabel(selectedItemLabelFn(selectedItem));
111-
setSelectedItemTitle(selectedItemTitleFn(selectedItem) || '');
112-
} else {
113-
setSelectedItemLabel(null);
114-
setSelectedItemTitle(null);
115-
}
116-
}, [selectedItem, selectedItemLabelFn, selectedItemTitleFn]);
117-
11889
useEffect(() => {
11990
if (!filterText) {
12091
setVisibleItems(groups);

‎libs/ui/src/lib/form/combobox/ComboboxWithItems.tsx‎

Lines changed: 10 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import {
77
import { NOOP } from '@jetstream/shared/utils';
88
import { ListItem, Maybe } from '@jetstream/types';
99
import isNumber from 'lodash/isNumber';
10-
import { createRef, forwardRef, useCallback, useEffect, useImperativeHandle, useRef, useState } from 'react';
10+
import { createRef, forwardRef, useCallback, useImperativeHandle, useMemo, useRef, useState } from 'react';
1111
import { Combobox, ComboboxPropsRef, ComboboxSharedProps } from './Combobox';
1212
import { ComboboxListItem, ComboboxListItemProps } from './ComboboxListItem';
1313
import { ComboboxListItemHeading } from './ComboboxListItemHeading';
@@ -61,24 +61,16 @@ export const ComboboxWithItems = forwardRef<ComboboxWithItemsRef, ComboboxWithIt
6161
) => {
6262
const comboboxRef = useRef<ComboboxPropsRef>(null);
6363
const [filterText, setFilterText] = useState<string>('');
64-
const [selectedItem, setSelectedItem] = useState<Maybe<ListItem>>(() =>
65-
selectedItemId ? items.find((item) => item.id === selectedItemId) : null,
66-
);
6764
const visibleItems = useFuzzySearchFilter(items, filterText);
68-
const [selectedItemLabel, setSelectedItemLabel] = useState<string | null>(() => {
69-
if (selectedItem) {
70-
const selectedItem = items.find((item) => item.id === selectedItemId);
71-
return selectedItem ? selectedItemLabelFn(selectedItem) : null;
72-
}
73-
return null;
74-
});
75-
const [selectedItemTitle, setSelectedItemTitle] = useState<string | null>(() => {
76-
if (selectedItem) {
77-
const selectedItem = items.find((item) => item.id === selectedItemId);
78-
return selectedItem ? selectedItemLabelFn(selectedItem) : null;
79-
}
80-
return null;
81-
});
65+
// Derived during render rather than mirrored into state with effects. The effect-synced version
66+
// had selectedItemLabelFn/selectedItemTitleFn in its dependencies — callers usually pass inline
67+
// functions, so it dispatched a setState after every parent re-render, which counts toward
68+
// React's nested-update limit and escalated to "Maximum update depth exceeded" during long
69+
// update cascades.
70+
const itemsById = useMemo(() => getFlattenedListItemsById(items), [items]);
71+
const selectedItem: Maybe<ListItem> = selectedItemId ? itemsById[selectedItemId] : null;
72+
const selectedItemLabel = selectedItem ? selectedItemLabelFn(selectedItem) : null;
73+
const selectedItemTitle = selectedItem ? selectedItemTitleFn(selectedItem) || '' : null;
8274
const [focusedIndex, setFocusedIndex] = useState<number | null>(null);
8375
const refs = visibleItems.map((value) => createRef<HTMLLIElement>());
8476

@@ -93,24 +85,6 @@ export const ComboboxWithItems = forwardRef<ComboboxWithItemsRef, ComboboxWithIt
9385
[],
9486
);
9587

96-
useEffect(() => {
97-
if (selectedItemId) {
98-
setSelectedItem(getFlattenedListItemsById(items)[selectedItemId]);
99-
} else {
100-
setSelectedItem(null);
101-
}
102-
}, [items, selectedItemId]);
103-
104-
useEffect(() => {
105-
if (selectedItem) {
106-
setSelectedItemLabel(selectedItemLabelFn(selectedItem));
107-
setSelectedItemTitle(selectedItemTitleFn(selectedItem) || '');
108-
} else {
109-
setSelectedItemLabel(null);
110-
setSelectedItemTitle(null);
111-
}
112-
}, [selectedItem, selectedItemLabelFn, selectedItemTitleFn]);
113-
11488
const onInputEnter = useCallback(() => {
11589
if (visibleItems.length > 0) {
11690
onSelected(visibleItems[0]);

‎libs/ui/src/lib/form/combobox/ComboboxWithItemsTypeAhead.tsx‎

Lines changed: 8 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { menuItemSelectScroll, useDebounce } from '@jetstream/shared/ui-utils';
22
import { ListItem, Maybe } from '@jetstream/types';
33
import isNumber from 'lodash/isNumber';
4-
import { createRef, FunctionComponent, useCallback, useEffect, useRef, useState } from 'react';
4+
import { createRef, FunctionComponent, useCallback, useEffect, useMemo, useRef, useState } from 'react';
55
import { Combobox, ComboboxPropsRef, ComboboxSharedProps } from './Combobox';
66
import { ComboboxListItem } from './ComboboxListItem';
77

@@ -35,44 +35,17 @@ export const ComboboxWithItemsTypeAhead: FunctionComponent<ComboboxWithItemsType
3535
const comboboxRef = useRef<ComboboxPropsRef>(null);
3636
const [filterTextNonDebounced, setFilterText] = useState<string>('');
3737
const filterText = useDebounce(filterTextNonDebounced, 300);
38-
const [selectedItem, setSelectedItem] = useState<Maybe<ListItem>>(() =>
39-
selectedItemId ? items.find((item) => item.id === selectedItemId) : null,
38+
// Derived during render rather than mirrored into state with effects — see ComboboxWithItems
39+
// for why the effect-synced version risked "Maximum update depth exceeded".
40+
const selectedItem: Maybe<ListItem> = useMemo(
41+
() => (selectedItemId ? items.find((item) => item.id === selectedItemId) : null),
42+
[items, selectedItemId],
4043
);
41-
const [selectedItemLabel, setSelectedItemLabel] = useState<string | null>(() => {
42-
if (selectedItem) {
43-
const selectedItem = items.find((item) => item.id === selectedItemId);
44-
return selectedItem ? selectedItemLabelFn(selectedItem) : null;
45-
}
46-
return null;
47-
});
48-
const [selectedItemTitle, setSelectedItemTitle] = useState<string | null>(() => {
49-
if (selectedItem) {
50-
const selectedItem = items.find((item) => item.id === selectedItemId);
51-
return selectedItem ? selectedItemLabelFn(selectedItem) : null;
52-
}
53-
return null;
54-
});
44+
const selectedItemLabel = selectedItem ? selectedItemLabelFn(selectedItem) : null;
45+
const selectedItemTitle = selectedItem ? selectedItemTitleFn(selectedItem) || '' : null;
5546
const [focusedIndex, setFocusedIndex] = useState<number | null>(null);
5647
const refs = items.map((value) => createRef<HTMLLIElement>());
5748

58-
useEffect(() => {
59-
if (selectedItemId) {
60-
setSelectedItem(items.find((item) => item.id === selectedItemId));
61-
} else {
62-
setSelectedItem(null);
63-
}
64-
}, [items, selectedItemId]);
65-
66-
useEffect(() => {
67-
if (selectedItem) {
68-
setSelectedItemLabel(selectedItemLabelFn(selectedItem));
69-
setSelectedItemTitle(selectedItemTitleFn(selectedItem) || '');
70-
} else {
71-
setSelectedItemLabel(null);
72-
setSelectedItemTitle(null);
73-
}
74-
}, [selectedItem, selectedItemLabelFn, selectedItemTitleFn]);
75-
7649
useEffect(() => {
7750
setLoading(true);
7851
setFocusedIndex(null);

‎libs/ui/src/lib/form/combobox/ComboboxWithItemsVirtual.tsx‎

Lines changed: 9 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { multiWordObjectFilter, NOOP } from '@jetstream/shared/utils';
44
import { ListItem, Maybe } from '@jetstream/types';
55
import { useVirtualizer } from '@tanstack/react-virtual';
66
import isNumber from 'lodash/isNumber';
7-
import { FunctionComponent, useCallback, useEffect, useRef, useState } from 'react';
7+
import { FunctionComponent, useCallback, useEffect, useMemo, useRef, useState } from 'react';
88
import { Combobox, ComboboxPropsRef, ComboboxSharedProps } from './Combobox';
99
import { ComboboxListItem } from './ComboboxListItem';
1010

@@ -42,25 +42,16 @@ export const ComboboxWithItemsVirtual: FunctionComponent<ComboboxWithItemsVirtua
4242
const comboboxRef = useRef<ComboboxPropsRef>(null);
4343
const [filterTextNonDebounced, setFilterText] = useState<string>('');
4444
const filterText = useDebounce(filterTextNonDebounced, 300);
45-
const [selectedItem, setSelectedItem] = useState<Maybe<ListItem>>(() =>
46-
selectedItemId ? items.find((item) => item.id === selectedItemId) : null,
47-
);
4845
const [visibleItems, setVisibleItems] = useState(items);
4946
const [focusedIndex, setFocusedIndex] = useState<number | null>(null);
50-
const [selectedItemLabel, setSelectedItemLabel] = useState<string | null>(() => {
51-
if (selectedItem) {
52-
const selectedItem = items.find((item) => item.id === selectedItemId);
53-
return selectedItem ? selectedItemLabelFn(selectedItem) : null;
54-
}
55-
return null;
56-
});
57-
const [selectedItemTitle, setSelectedItemTitle] = useState<string | null>(() => {
58-
if (selectedItem) {
59-
const selectedItem = items.find((item) => item.id === selectedItemId);
60-
return selectedItem ? selectedItemLabelFn(selectedItem) : null;
61-
}
62-
return null;
63-
});
47+
// Derived during render rather than mirrored into state with effects — see ComboboxWithItems
48+
// for why the effect-synced version risked "Maximum update depth exceeded".
49+
const selectedItem: Maybe<ListItem> = useMemo(
50+
() => (selectedItemId ? items.find((item) => item.id === selectedItemId) : null),
51+
[items, selectedItemId],
52+
);
53+
const selectedItemLabel = selectedItem ? selectedItemLabelFn(selectedItem) : null;
54+
const selectedItemTitle = selectedItem ? selectedItemTitleFn(selectedItem) || '' : null;
6455

6556
const rowVirtualizer = useVirtualizer({
6657
count: visibleItems.length,
@@ -95,24 +86,6 @@ export const ComboboxWithItemsVirtual: FunctionComponent<ComboboxWithItemsVirtua
9586
setFocusedIndex(null);
9687
}, [filterTextNonDebounced]);
9788

98-
useEffect(() => {
99-
if (selectedItemId) {
100-
setSelectedItem(items.find((item) => item.id === selectedItemId));
101-
} else {
102-
setSelectedItem(null);
103-
}
104-
}, [items, selectedItemId]);
105-
106-
useEffect(() => {
107-
if (selectedItem) {
108-
setSelectedItemLabel(selectedItemLabelFn(selectedItem));
109-
setSelectedItemTitle(selectedItemTitleFn(selectedItem) || '');
110-
} else {
111-
setSelectedItemLabel(null);
112-
setSelectedItemTitle(null);
113-
}
114-
}, [selectedItem, selectedItemLabelFn, selectedItemTitleFn]);
115-
11689
useEffect(() => {
11790
if (!filterText) {
11891
setVisibleItems(items);

‎libs/ui/src/lib/tabs/Tabs.tsx‎

Lines changed: 11 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { multiWordObjectFilter } from '@jetstream/shared/utils';
33
import { HorizontalVertical, UiTabSection } from '@jetstream/types';
44
import classNames from 'classnames';
55
import isNil from 'lodash/isNil';
6-
import React, { ReactNode, forwardRef, useEffect, useImperativeHandle, useState } from 'react';
6+
import React, { ReactNode, forwardRef, useEffect, useImperativeHandle, useMemo, useState } from 'react';
77
import SearchInput from '../form/search-input/SearchInput';
88
import Tab from './Tab';
99

@@ -58,15 +58,17 @@ export const Tabs = forwardRef<unknown, TabsProps>(
5858
return initialActiveId || (tabs && tabs[0] && tabs[0].id);
5959
});
6060

61-
const [activeTab, setActiveTab] = useState<UiTabSection | undefined>(() => {
62-
if (initialActiveId) {
63-
return tabs.find((tab) => tab.id === initialActiveId);
64-
} else if (tabs) {
65-
return tabs[0];
66-
}
67-
});
6861
const [filterValue, setFilterValue] = useState('');
69-
const [filteredTabs, setFilteredTabs] = useState(tabs);
62+
63+
// Derived during render rather than mirrored into state with effects. The effect-synced version
64+
// dispatched a setState after every parent re-render (the `tabs` prop is usually an inline array),
65+
// which counts toward React's nested-update limit and escalated to "Maximum update depth exceeded"
66+
// during long update cascades.
67+
const activeTab = useMemo(() => (tabs && activeId ? tabs.find((tab) => tab.id === activeId) : undefined), [tabs, activeId]);
68+
const filteredTabs = useMemo(
69+
() => (filterValue ? tabs.filter(multiWordObjectFilter(['titleText', 'title', 'id'], filterValue)) : tabs),
70+
[tabs, filterValue],
71+
);
7072

7173
useImperativeHandle<unknown, TabsRef>(ref, () => ({
7274
changeTab: (id: string) => {
@@ -75,31 +77,15 @@ export const Tabs = forwardRef<unknown, TabsProps>(
7577
},
7678
}));
7779

78-
useEffect(() => {
79-
if (tabs && activeId) {
80-
setActiveTab(tabs.find((tab) => tab.id === activeId));
81-
}
82-
}, [tabs, activeId]);
83-
8480
useEffect(() => {
8581
if (initialActiveId && initialActiveId !== activeId) {
8682
setActiveId(initialActiveId);
8783
}
8884
// eslint-disable-next-line react-hooks/exhaustive-deps
8985
}, [initialActiveId]);
9086

91-
useEffect(() => {
92-
if (!filterValue && tabs !== filteredTabs) {
93-
setFilteredTabs(tabs);
94-
} else if (filterValue) {
95-
setFilteredTabs(tabs.filter(multiWordObjectFilter(['titleText', 'title', 'id'], filterValue)));
96-
}
97-
// eslint-disable-next-line react-hooks/exhaustive-deps
98-
}, [tabs, filterValue]);
99-
10087
function handleTabClick(event: React.MouseEvent<HTMLAnchorElement, MouseEvent>, tab: UiTabSection) {
10188
event.preventDefault();
102-
// setActiveTab(tabs.find((currTab) => currTab.id === tab.id));
10389
setActiveId(tab.id);
10490
if (onChange) {
10591
onChange(tab.id);

0 commit comments

Comments
 (0)