Skip to content

Commit 91e64b3

Browse files
authored
perf(ui): avoid unnecessary remounting of DraggableSortable (#14463)
The `DraggableSortable` (rendered in the list view when sorting columns) was unmounted and re-mounted multiple times when loading a page. This happened because the `useMemo` we used in `PillSelector` was returning a new function reference on each recomputation => every time it ran, `DraggableSortable` remounted. This means the ID generated by `useId` in `DraggableSortable` changed multiple times during load. This made the hydration issues in my [Next.js 16 PR](#14456) more difficult to debug. This PR removes the unnecessary re-mounting, improving performance and potentially reducing the risk of hydration issues.
1 parent 0231a8d commit 91e64b3

File tree

1 file changed

+45
-44
lines changed
  • packages/ui/src/elements/PillSelector

1 file changed

+45
-44
lines changed

packages/ui/src/elements/PillSelector/index.tsx

Lines changed: 45 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -30,52 +30,53 @@ export type Props = {
3030
* If `draggable` is true, the pills can be reordered by dragging.
3131
*/
3232
export const PillSelector: React.FC<Props> = ({ draggable, onClick, pills }) => {
33-
const Wrapper = React.useMemo(() => {
34-
if (draggable) {
35-
return ({ children }) => (
36-
<DraggableSortable
37-
className={baseClass}
38-
ids={pills.map((pill) => pill.name)}
39-
onDragEnd={({ moveFromIndex, moveToIndex }) => {
40-
draggable.onDragEnd({
41-
moveFromIndex,
42-
moveToIndex,
43-
})
33+
// IMPORTANT: Do NOT wrap DraggableSortable in a dynamic component function using useMemo.
34+
// BAD: useMemo(() => ({ children }) => <DraggableSortable>...</DraggableSortable>, [deps])
35+
// This creates a new function reference on each recomputation, causing React to treat it as a
36+
// different component type, triggering unmount/mount cycles instead of just updating props.
37+
// GOOD: Use conditional rendering directly: draggable ? <DraggableSortable /> : <div />
38+
const pillElements = React.useMemo(() => {
39+
return pills.map((pill, i) => {
40+
return (
41+
<Pill
42+
alignIcon="left"
43+
aria-checked={pill.selected}
44+
className={[`${baseClass}__pill`, pill.selected && `${baseClass}__pill--selected`]
45+
.filter(Boolean)
46+
.join(' ')}
47+
draggable={Boolean(draggable)}
48+
icon={pill.selected ? <XIcon /> : <PlusIcon />}
49+
id={pill.name}
50+
key={pill.key ?? `${pill.name}-${i}`}
51+
onClick={() => {
52+
if (onClick) {
53+
void onClick({ pill })
54+
}
4455
}}
56+
size="small"
4557
>
46-
{children}
47-
</DraggableSortable>
58+
{pill.Label ?? <span className={`${baseClass}__pill-label`}>{pill.name}</span>}
59+
</Pill>
4860
)
49-
} else {
50-
return ({ children }) => <div className={baseClass}>{children}</div>
51-
}
52-
}, [draggable, pills])
61+
})
62+
}, [pills, onClick, draggable])
5363

54-
return (
55-
<Wrapper>
56-
{pills.map((pill, i) => {
57-
return (
58-
<Pill
59-
alignIcon="left"
60-
aria-checked={pill.selected}
61-
className={[`${baseClass}__pill`, pill.selected && `${baseClass}__pill--selected`]
62-
.filter(Boolean)
63-
.join(' ')}
64-
draggable={Boolean(draggable)}
65-
icon={pill.selected ? <XIcon /> : <PlusIcon />}
66-
id={pill.name}
67-
key={pill.key ?? `${pill.name}-${i}`}
68-
onClick={() => {
69-
if (onClick) {
70-
void onClick({ pill })
71-
}
72-
}}
73-
size="small"
74-
>
75-
{pill.Label ?? <span className={`${baseClass}__pill-label`}>{pill.name}</span>}
76-
</Pill>
77-
)
78-
})}
79-
</Wrapper>
80-
)
64+
if (draggable) {
65+
return (
66+
<DraggableSortable
67+
className={baseClass}
68+
ids={pills.map((pill) => pill.name)}
69+
onDragEnd={({ moveFromIndex, moveToIndex }) => {
70+
draggable.onDragEnd({
71+
moveFromIndex,
72+
moveToIndex,
73+
})
74+
}}
75+
>
76+
{pillElements}
77+
</DraggableSortable>
78+
)
79+
}
80+
81+
return <div className={baseClass}>{pillElements}</div>
8182
}

0 commit comments

Comments
 (0)