Skip to content

Commit 65316af

Browse files
authored
fix(tabs): avoid infinite update loop in Svelte 5 (#2394)
Fixes #2366 Introduce `needsDomSync` flag to gate DOM-to-store synchronization only when tabs/content are added/removed. Prevents `afterUpdate` from triggering cascading re-renders on every update cycle.
1 parent 1c95d6c commit 65316af

File tree

2 files changed

+38
-45
lines changed

2 files changed

+38
-45
lines changed

src/Tabs/TabContent.svelte

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@
1616
});
1717
1818
$: selected = $selectedContent === id;
19-
$: index = $contentById[id].index;
20-
$: tabId = $tabs[index].id;
19+
$: index = $contentById[id]?.index ?? 0;
20+
$: tabId = $tabs[index]?.id;
2121
</script>
2222
2323
<div

src/Tabs/Tabs.svelte

Lines changed: 36 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -69,44 +69,40 @@
6969
let refTabList = null;
7070
let refRoot = null;
7171
72+
// Flag to trigger DOM reordering only when tabs change.
73+
// This is necessary to avoid infinite loops in Svelte 5.
74+
let needsDomSync = false;
75+
7276
/**
7377
* @type {(data: { id: string; label: string; disabled: boolean }) => void}
7478
*/
7579
const add = (data) => {
76-
tabs.update((_) => {
77-
const newTabs = [..._, { ...data, index: _.length }];
78-
return newTabs.map((tab, index) => ({ ...tab, index }));
79-
});
80+
needsDomSync = true;
81+
tabs.update((_) => [..._, { ...data, index: _.length }]);
8082
};
8183
8284
/**
8385
* @type {(id: string) => void}
8486
*/
8587
const remove = (id) => {
86-
tabs.update((_) => {
87-
const filtered = _.filter((tab) => tab.id !== id);
88-
return filtered.map((tab, index) => ({ ...tab, index }));
89-
});
88+
needsDomSync = true;
89+
tabs.update((_) => _.filter((tab) => tab.id !== id));
9090
};
9191
9292
/**
9393
* @type {(data: { id: string }) => void}
9494
*/
9595
const addContent = (data) => {
96-
content.update((_) => {
97-
const newContent = [..._, { ...data, index: _.length }];
98-
return newContent.map((item, index) => ({ ...item, index }));
99-
});
96+
needsDomSync = true;
97+
content.update((_) => [..._, { ...data, index: _.length }]);
10098
};
10199
102100
/**
103101
* @type {(id: string) => void}
104102
*/
105103
const removeContent = (id) => {
106-
content.update((_) => {
107-
const filtered = _.filter((item) => item.id !== id);
108-
return filtered.map((item, index) => ({ ...item, index }));
109-
});
104+
needsDomSync = true;
105+
content.update((_) => _.filter((item) => item.id !== id));
110106
};
111107
112108
/**
@@ -165,44 +161,41 @@
165161
});
166162
167163
afterUpdate(() => {
168-
// We need to use the DOM order of the tabs to update
169-
// the indices of the tabs in the store. This is because
170-
// the current implementation uses an implicit index approach
171-
// where order matters. A more robust solution would be to
172-
// require explicit keys specified by the consumer.
173-
if (refTabList) {
164+
// Sync DOM order with stores only when tabs are added/removed.
165+
// This avoids infinite loops in Svelte 5 by not running on every update.
166+
if (needsDomSync && refTabList) {
167+
needsDomSync = false;
168+
174169
const domTabs = Array.from(refTabList.querySelectorAll("[role='tab']"));
175-
const domIds = domTabs.map((el) => el.id);
170+
const domTabIds = domTabs.map((el) => el.id);
176171
177172
tabs.update((currentTabs) => {
178173
const tabsMap = new Map(currentTabs.map((tab) => [tab.id, tab]));
179-
const reorderedTabs = domIds
174+
return domTabIds
180175
.map((id, index) => {
181176
const tab = tabsMap.get(id);
182177
return tab ? { ...tab, index } : undefined;
183178
})
184179
.filter(Boolean);
185-
186-
return reorderedTabs;
187180
});
188-
}
189-
190-
const contentElements = refRoot?.parentElement
191-
? Array.from(refRoot.parentElement.querySelectorAll("[role='tabpanel']"))
192-
: [];
193-
const contentIds = contentElements.map((el) => el.id);
194181
195-
content.update((currentContent) => {
196-
const contentMap = new Map(currentContent.map((c) => [c.id, c]));
197-
const reorderedContent = contentIds
198-
.map((id, index) => {
199-
const c = contentMap.get(id);
200-
return c ? { ...c, index } : undefined;
201-
})
202-
.filter(Boolean);
182+
const contentElements = refRoot?.parentElement
183+
? Array.from(
184+
refRoot.parentElement.querySelectorAll("[role='tabpanel']"),
185+
)
186+
: [];
187+
const contentIds = contentElements.map((el) => el.id);
203188
204-
return reorderedContent;
205-
});
189+
content.update((currentContent) => {
190+
const contentMap = new Map(currentContent.map((c) => [c.id, c]));
191+
return contentIds
192+
.map((id, index) => {
193+
const c = contentMap.get(id);
194+
return c ? { ...c, index } : undefined;
195+
})
196+
.filter(Boolean);
197+
});
198+
}
206199
207200
if (selected !== currentIndex) {
208201
selected = currentIndex;
@@ -280,4 +273,4 @@
280273
<slot />
281274
</ul>
282275
</div>
283-
<slot name="content" />
276+
<slot name="content" />

0 commit comments

Comments
 (0)