Skip to content

Commit 34a9e62

Browse files
authored
fix: Select and Combobox infinite loop on item select (#925)
1 parent b00ddf9 commit 34a9e62

File tree

55 files changed

+1236
-1440
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

55 files changed

+1236
-1440
lines changed

.changeset/wise-pumas-help.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"bits-ui": patch
3+
---
4+
5+
fix: `Select` and `Combobox` infinite loop on item selection

package.json

+8-8
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,18 @@
1919
"author": "Hunter Johnston <https://github.com/huntabyte>",
2020
"license": "MIT",
2121
"devDependencies": {
22-
"@changesets/cli": "^2.27.1",
22+
"@changesets/cli": "^2.27.9",
2323
"@huntabyte/eslint-config": "^0.3.2",
2424
"@huntabyte/eslint-plugin": "^0.1.0",
25-
"@svitejs/changesets-changelog-github-compact": "^1.1.0",
26-
"eslint": "^9.0.0",
27-
"eslint-plugin-svelte": "^2.44.0",
28-
"prettier": "^3.2.5",
29-
"prettier-plugin-svelte": "^3.2.2",
25+
"@svitejs/changesets-changelog-github-compact": "^1.2.0",
26+
"eslint": "^9.14.0",
27+
"eslint-plugin-svelte": "^2.46.0",
28+
"prettier": "^3.3.3",
29+
"prettier-plugin-svelte": "^3.2.8",
3030
"prettier-plugin-tailwindcss": "0.5.13",
31-
"svelte": "^5.1.0",
31+
"svelte": "^5.1.16",
3232
"svelte-eslint-parser": "^0.41.1",
33-
"wrangler": "^3.44.0"
33+
"wrangler": "^3.87.0"
3434
},
3535
"type": "module",
3636
"engines": {

packages/bits-ui/package.json

+11-11
Original file line numberDiff line numberDiff line change
@@ -27,21 +27,21 @@
2727
"!dist/**/*.spec.*"
2828
],
2929
"devDependencies": {
30-
"@sveltejs/kit": "^2.5.28",
31-
"@sveltejs/package": "^2.3.5",
30+
"@sveltejs/kit": "^2.8.1",
31+
"@sveltejs/package": "^2.3.7",
3232
"@sveltejs/vite-plugin-svelte": "4.0.0-next.7",
33-
"@types/node": "^20.14.10",
33+
"@types/node": "^20.17.6",
3434
"@types/resize-observer-browser": "^0.1.11",
3535
"csstype": "^3.1.3",
3636
"jest-axe": "^9.0.0",
37-
"jsdom": "^24.1.0",
38-
"publint": "^0.2.11",
39-
"svelte": "^5.1.0",
40-
"svelte-check": "4.0.3",
41-
"tslib": "^2.7.0",
42-
"typescript": "^5.6.2",
43-
"vite": "^5.4.6",
44-
"vitest": "^2.1.1"
37+
"jsdom": "^24.1.3",
38+
"publint": "^0.2.12",
39+
"svelte": "^5.1.16",
40+
"svelte-check": "^4.0.7",
41+
"tslib": "^2.8.1",
42+
"typescript": "^5.6.3",
43+
"vite": "^5.4.11",
44+
"vitest": "^2.1.5"
4545
},
4646
"svelte": "./dist/index.js",
4747
"types": "./dist/index.d.ts",

packages/bits-ui/src/lib/bits/select/select.svelte.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -842,7 +842,7 @@ class SelectItemState {
842842

843843
$effect(() => {
844844
if (!this.mounted) return;
845-
this.root.setInitialHighlightedNode();
845+
untrack(() => this.root.setInitialHighlightedNode());
846846
});
847847
}
848848

packages/tests/package.json

+12-12
Original file line numberDiff line numberDiff line change
@@ -10,25 +10,25 @@
1010
"test": "pnpm sync && vitest"
1111
},
1212
"devDependencies": {
13-
"@sveltejs/adapter-auto": "^3.0.0",
14-
"@sveltejs/kit": "^2.0.0",
13+
"@sveltejs/adapter-auto": "^3.3.1",
14+
"@sveltejs/kit": "^2.8.1",
1515
"@sveltejs/vite-plugin-svelte": "4.0.0-next.7",
16-
"@testing-library/dom": "^10.3.1",
17-
"@testing-library/jest-dom": "^6.4.6",
18-
"@testing-library/svelte": "^5.2.1",
16+
"@testing-library/dom": "^10.4.0",
17+
"@testing-library/jest-dom": "^6.6.3",
18+
"@testing-library/svelte": "^5.2.4",
1919
"@testing-library/user-event": "^14.5.2",
2020
"@types/jest-axe": "^3.5.9",
21-
"@types/node": "^20.14.10",
21+
"@types/node": "^20.17.6",
2222
"@types/resize-observer-browser": "^0.1.11",
2323
"@types/testing-library__jest-dom": "^5.14.9",
2424
"jest-axe": "^9.0.0",
25-
"jsdom": "^24.1.0",
25+
"jsdom": "^24.1.3",
2626
"resize-observer-polyfill": "^1.5.1",
27-
"svelte": "^5.1.0",
28-
"svelte-check": "^4.0.3",
29-
"typescript": "^5.6.2",
30-
"vite": "^5.4.6",
31-
"vitest": "^2.1.1"
27+
"svelte": "^5.1.16",
28+
"svelte-check": "^4.0.7",
29+
"typescript": "^5.6.3",
30+
"vite": "^5.4.11",
31+
"vitest": "^2.1.5"
3232
},
3333
"dependencies": {
3434
"@internationalized/date": "^3.5.6",

packages/tests/src/tests/accordion/accordion-single-force-mount-test.svelte

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
</Accordion.Content>
4444
{:else}
4545
<Accordion.Content data-testid="{value}-content" forceMount>
46-
{#snippet child({ props })}
46+
{#snippet child({ props, open: _open })}
4747
<div {...props}>
4848
{content}
4949
</div>

packages/tests/src/tests/alert-dialog/alert-dialog-force-mount-test.svelte

+2-2
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
data-testid="overlay"
4444
class="fixed inset-0 h-[100vh] w-[100vw] bg-black"
4545
>
46-
{#snippet child({ props })}
46+
{#snippet child({ props, open: _open })}
4747
<div {...props}></div>
4848
{/snippet}
4949
</AlertDialog.Overlay>
@@ -80,7 +80,7 @@
8080
data-testid="content"
8181
class="tranlate-x-[50%] fixed left-[50%] top-[50%] translate-y-[50%] bg-white p-1"
8282
>
83-
{#snippet child({ props })}
83+
{#snippet child({ props, open: _open })}
8484
<div {...props}>
8585
<AlertDialog.Title {...titleProps} data-testid="title"
8686
>title</AlertDialog.Title

packages/tests/src/tests/alert-dialog/alert-dialog.test.ts

-2
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ async function expectIsOpen(
3131

3232
function setup(props: AlertDialogTestProps = {}, component: Component = AlertDialogTest) {
3333
const user = userEvent.setup({ pointerEventsCheck: 0 });
34-
// @ts-expect-error - testing lib needs to update their generic types
3534
const returned = render(component, { ...props });
3635
const trigger = returned.getByTestId("trigger");
3736

@@ -56,7 +55,6 @@ async function open(props: AlertDialogTestProps = {}) {
5655

5756
describe("alert dialog", () => {
5857
it("should have no accessibility violations", async () => {
59-
// @ts-expect-error - testing lib needs to update their generic types
6058
const { container } = render(AlertDialogTest);
6159
expect(await axe(container)).toHaveNoViolations();
6260
});

packages/tests/src/tests/avatar/avatar.test.ts

-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import AvatarTest from "./avatar-test.svelte";
77
const src = "https://github.com/huntabyte.png";
88

99
function setup(props: { src: string }) {
10-
// @ts-expect-error - testing lib needs to update their generic types
1110
return render(AvatarTest, { props });
1211
}
1312

packages/tests/src/tests/calendar/calendar.test.ts

-3
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ const months = ["January", "February", "March", "April", "May", "June", "July",
2222

2323
function setup(props: Partial<CalendarSingleTestProps> = {}) {
2424
const user = userEvent.setup();
25-
// @ts-expect-error - testing lib needs to update their generic types
2625
const returned = render(CalendarTest, { ...props, type: "single" });
2726
const calendar = returned.getByTestId("calendar");
2827
expect(calendar).toBeVisible();
@@ -31,7 +30,6 @@ function setup(props: Partial<CalendarSingleTestProps> = {}) {
3130

3231
function setupMulti(props: Partial<CalendarMultiTestProps> = {}) {
3332
const user = userEvent.setup();
34-
// @ts-expect-error - testing lib needs to update their generic types
3533
const returned = render(CalendarMultiTest, { ...props, type: "multiple" });
3634
const calendar = returned.getByTestId("calendar");
3735
expect(calendar).toBeVisible();
@@ -40,7 +38,6 @@ function setupMulti(props: Partial<CalendarMultiTestProps> = {}) {
4038

4139
describe("calendar", () => {
4240
it("should have no accessibility violations", async () => {
43-
// @ts-expect-error - testing lib needs to update their generic types
4441
const { container } = render(CalendarTest);
4542
expect(await axe(container)).toHaveNoViolations();
4643
});

packages/tests/src/tests/checkbox/checkbox.test.ts

-2
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ const kbd = getTestKbd();
1010

1111
function setup(props?: Checkbox.RootProps) {
1212
const user = setupUserEvents();
13-
// @ts-expect-error - testing lib needs to update their generic types
1413
const returned = render(CheckboxTest, props);
1514
const root = returned.getByTestId("root");
1615
const input = document.querySelector("input") as HTMLInputElement;
@@ -24,7 +23,6 @@ function setup(props?: Checkbox.RootProps) {
2423

2524
describe("checkbox", () => {
2625
it("should have no accessibility violations", async () => {
27-
// @ts-expect-error - testing lib needs to update their generic types
2826
const { container } = render(CheckboxTest);
2927
expect(await axe(container)).toHaveNoViolations();
3028
});

packages/tests/src/tests/collapsible/collapsible-force-mount-test.svelte

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
</Collapsible.Content>
2525
{:else}
2626
<Collapsible.Content data-testid="content" forceMount>
27-
{#snippet child({ props })}
27+
{#snippet child({ props, open: _open })}
2828
<div {...props}>Content</div>
2929
{/snippet}
3030
</Collapsible.Content>

packages/tests/src/tests/collapsible/collapsible.test.ts

-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ function setup(
1414
component: Component = CollapsibleTest
1515
) {
1616
const user = setupUserEvents();
17-
// @ts-expect-error - testing lib needs to update their generic types
1817
const returned = render(component, props);
1918
const root = returned.getByTestId("root");
2019
const trigger = returned.getByTestId("trigger");
@@ -32,7 +31,6 @@ function setup(
3231

3332
describe("collapsible", () => {
3433
it("should have no accessibility violations", async () => {
35-
// @ts-expect-error - testing lib needs to update their generic types
3634
const { container } = render(CollapsibleTest);
3735
expect(await axe(container)).toHaveNoViolations();
3836
});

packages/tests/src/tests/combobox/combobox-force-mount-test.svelte

+6-3
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,10 @@
8080
{value}
8181
{label}
8282
>
83-
{#snippet children({ selected })}
83+
{#snippet children({
84+
selected,
85+
highlighted: _highlighted,
86+
})}
8487
{#if selected}
8588
<span data-testid="{value}-indicator">x</span>
8689
{/if}
@@ -95,15 +98,15 @@
9598
</Combobox.Content>
9699
{:else}
97100
<Combobox.Content data-testid="content" {...contentProps} forceMount>
98-
{#snippet child({ props })}
101+
{#snippet child({ props, open: _open })}
99102
<div {...props}>
100103
<Combobox.Group data-testid="group">
101104
<Combobox.GroupHeading data-testid="group-label"
102105
>Options</Combobox.GroupHeading
103106
>
104107
{#each filteredItems as { value, label, disabled }}
105108
<Combobox.Item data-testid={value} {disabled} {value} {label}>
106-
{#snippet children({ selected })}
109+
{#snippet children({ selected, highlighted: _highlighted })}
107110
{#if selected}
108111
<span data-testid="{value}-indicator">x</span>
109112
{/if}

packages/tests/src/tests/combobox/combobox-multi-test.svelte

+1-1
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@
6868
<Combobox.GroupHeading data-testid="group-label">Options</Combobox.GroupHeading>
6969
{#each filteredItems as { value, label, disabled }}
7070
<Combobox.Item data-testid={value} {disabled} {value} {label}>
71-
{#snippet children({ selected })}
71+
{#snippet children({ selected, highlighted: _highlighted })}
7272
{#if selected}
7373
<span data-testid="{value}-indicator">x</span>
7474
{/if}

packages/tests/src/tests/combobox/combobox-test.svelte

+1-1
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@
6464
<Combobox.GroupHeading data-testid="group-label">Options</Combobox.GroupHeading>
6565
{#each filteredItems as { value, label, disabled }}
6666
<Combobox.Item data-testid={value} {disabled} {value} {label}>
67-
{#snippet children({ selected })}
67+
{#snippet children({ selected, highlighted: _highlighted })}
6868
{#if selected}
6969
<span data-testid="{value}-indicator">x</span>
7070
{/if}

packages/tests/src/tests/combobox/combobox.test.ts

+1-5
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ function setupSingle(
3939
component: Component<any, any, any> = ComboboxTest
4040
) {
4141
const user = setupUserEvents();
42-
// @ts-expect-error - testing lib needs to update their generic types
4342
const returned = render(component, { name: "test", ...props, items });
4443
const input = returned.getByTestId("input");
4544
const trigger = returned.getByTestId("trigger");
@@ -70,7 +69,6 @@ function setupSingle(
7069

7170
function setupMultiple(props: Partial<ComboboxMultipleTestProps> = {}, items: Item[] = testItems) {
7271
const user = setupUserEvents();
73-
// @ts-expect-error - testing lib needs to update their generic types
7472
const returned = render(ComboboxMultiTest, { name: "test", ...props, items });
7573
const input = returned.getByTestId("input");
7674
const trigger = returned.getByTestId("trigger");
@@ -155,8 +153,7 @@ async function openMultiple(
155153
const OPEN_KEYS = [kbd.ARROW_DOWN, kbd.ARROW_UP];
156154

157155
describe("combobox - single", () => {
158-
it("should have noaccessibility violations", async () => {
159-
// @ts-expect-error - testing lib needs to update their generic types
156+
it("should have no accessibility violations", async () => {
160157
const { container } = render(ComboboxTest);
161158
expect(await axe(container)).toHaveNoViolations();
162159
});
@@ -450,7 +447,6 @@ describe("combobox - single", () => {
450447
////////////////////////////////////
451448
describe("combobox - multiple", () => {
452449
it("should have no accessibility violations", async () => {
453-
// @ts-expect-error - testing lib needs to update their generic types
454450
const { container } = render(ComboboxMultiTest);
455451
expect(await axe(container)).toHaveNoViolations();
456452
});

packages/tests/src/tests/context-menu/context-menu-force-mount-test.svelte

+14-5
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,10 @@
6666
bind:checked={subChecked}
6767
data-testid="sub-checkbox-item"
6868
>
69-
{#snippet children({ checked })}
69+
{#snippet children({
70+
checked,
71+
indeterminate: _indeterminate,
72+
})}
7073
<span data-testid="sub-checkbox-indicator">
7174
{checked}
7275
</span>
@@ -85,7 +88,10 @@
8588
bind:checked
8689
data-testid="checkbox-item"
8790
>
88-
{#snippet children({ checked })}
91+
{#snippet children({
92+
checked,
93+
indeterminate: _indeterminate,
94+
})}
8995
<span data-testid="checkbox-indicator">
9096
{checked}
9197
</span>
@@ -120,7 +126,7 @@
120126
</ContextMenu.Content>
121127
{:else}
122128
<ContextMenu.Content {...contentProps} data-testid="content" forceMount>
123-
{#snippet child({ props })}
129+
{#snippet child({ props, open: _open })}
124130
<div {...props}>
125131
<ContextMenu.Separator data-testid="separator" />
126132
<ContextMenu.Group data-testid="group">
@@ -144,7 +150,10 @@
144150
bind:checked={subChecked}
145151
data-testid="sub-checkbox-item"
146152
>
147-
{#snippet children({ checked })}
153+
{#snippet children({
154+
checked,
155+
indeterminate: _indeterminate,
156+
})}
148157
<span data-testid="sub-checkbox-indicator">
149158
{checked}
150159
</span>
@@ -160,7 +169,7 @@
160169
>disabled item 2</ContextMenu.Item
161170
>
162171
<ContextMenu.CheckboxItem bind:checked data-testid="checkbox-item">
163-
{#snippet children({ checked })}
172+
{#snippet children({ checked, indeterminate: _indeterminate })}
164173
<span data-testid="checkbox-indicator">
165174
{checked}
166175
</span>

packages/tests/src/tests/context-menu/context-menu-test.svelte

+2-2
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@
6060
bind:checked={subChecked}
6161
data-testid="sub-checkbox-item"
6262
>
63-
{#snippet children({ checked })}
63+
{#snippet children({ checked, indeterminate: _indeterminate })}
6464
<span data-testid="sub-checkbox-indicator">
6565
{checked}
6666
</span>
@@ -76,7 +76,7 @@
7676
>disabled item 2</ContextMenu.Item
7777
>
7878
<ContextMenu.CheckboxItem bind:checked data-testid="checkbox-item">
79-
{#snippet children({ checked })}
79+
{#snippet children({ checked, indeterminate: _indeterminate })}
8080
<span data-testid="checkbox-indicator">
8181
{checked}
8282
</span>

packages/tests/src/tests/context-menu/context-menu.test.ts

-2
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ function setup(
1818
component: Component = ContextMenuTest
1919
) {
2020
const user = setupUserEvents();
21-
// @ts-expect-error - testing lib needs to update their generic types
2221
const returned = render(component, { ...props });
2322
const trigger = returned.getByTestId("trigger");
2423
function getContent() {
@@ -68,7 +67,6 @@ async function openSubmenu(props: Awaited<ReturnType<typeof open>>) {
6867

6968
describe("context menu", () => {
7069
it("should have no accessibility violations", async () => {
71-
// @ts-expect-error - testing lib needs to update their generic types
7270
const { container } = render(ContextMenuTest);
7371
expect(await axe(container)).toHaveNoViolations();
7472
});

0 commit comments

Comments
 (0)