Skip to content

Commit 17b2d34

Browse files
RobinMalfaitRobinMalfait
authored
Improve performance of Combobox component (#2574)
* add countries data and combobox example * directly push to the options array No need to spread here because this will slow things down tremendously. If you add 5 options, then this will happen: ```js let options = [] options = [...options, 1] options = [...options, 2] options = [...options, 3] options = [...options, 4] options = [...options, 5] ``` It's making a lot of copies and has to spread everything it just copied again. Instead, let's just push to the array: ```js let options = [] options.push(1) options.push(2) options.push(3) options.push(4) options.push(5) ``` * only sort options by DOM node once When registering 50 options, we will re-sort the options based on the DOM node position for every option that gets registered. This is overkill and unnecessary. The re-sorting is useful if an option is injected between 2 options. When we sort the full list for _every_ `registerOption` call then the result after the last `registerOption` will be the same if we only sorted after the last `registerOption` call only. This will ensure that we are skipping a ton of work and only do the work once at the end where it matters. * debounce the `goToOption` call until the next frame This will allow us to perform the task of `goToOption` once instead of `n` times if they happen really fast after eachother. This can happen when you are leaving option B and going to option A. Then the following steps happen: - Leaving Option B `goToOption(Nothing)` - Entering Option A `goToOption(A)` Both will re-render everything because the internal active option index would be changed. But only the last `goToOption(A)` is the interesting version here. We could also move the `goToOption(Nothing)` to the `ComboboxOptions` (wrapper) itself instead of adding these listeners on each individual `ComboboxOption`. However, if you add an additional visual piece of DOM above or between the options, hovering that would not leave the `ComboboxOptions` and therefore the last `ComboboxOption` would still be active which we don't want. * update changelog
1 parent a9e8563 commit 17b2d34

File tree

6 files changed

+813
-36
lines changed

6 files changed

+813
-36
lines changed

‎packages/@headlessui-vue/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1111

1212
- Ensure the caret is in a consistent position when syncing the `Combobox.Input` value ([#2568](https://github.com/tailwindlabs/headlessui/pull/2568))
1313
- Improve "outside click" behaviour in combination with 3rd party libraries ([#2572](https://github.com/tailwindlabs/headlessui/pull/2572))
14+
- Improve performance of `Combobox` component ([#2574](https://github.com/tailwindlabs/headlessui/pull/2574))
1415

1516
## [1.7.14] - 2023-06-01
1617

‎packages/@headlessui-vue/src/components/combobox/combobox.ts‎

Lines changed: 51 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,9 @@ export let Combobox = defineComponent({
202202
computed(() => props.defaultValue)
203203
)
204204

205+
let goToOptionRaf: ReturnType<typeof requestAnimationFrame> | null = null
206+
let orderOptionsRaf: ReturnType<typeof requestAnimationFrame> | null = null
207+
205208
let api = {
206209
comboboxState,
207210
value,
@@ -275,47 +278,53 @@ export let Combobox = defineComponent({
275278
comboboxState.value = ComboboxStates.Open
276279
},
277280
goToOption(focus: Focus, id?: string, trigger?: ActivationTrigger) {
278-
defaultToFirstOption.value = false
279-
280-
if (props.disabled) return
281-
if (
282-
optionsRef.value &&
283-
!optionsPropsRef.value.static &&
284-
comboboxState.value === ComboboxStates.Closed
285-
) {
286-
return
281+
if (goToOptionRaf !== null) {
282+
cancelAnimationFrame(goToOptionRaf)
287283
}
288284

289-
let adjustedState = adjustOrderedState()
285+
goToOptionRaf = requestAnimationFrame(() => {
286+
defaultToFirstOption.value = false
290287

291-
// It's possible that the activeOptionIndex is set to `null` internally, but
292-
// this means that we will fallback to the first non-disabled option by default.
293-
// We have to take this into account.
294-
if (adjustedState.activeOptionIndex === null) {
295-
let localActiveOptionIndex = adjustedState.options.findIndex(
296-
(option) => !option.dataRef.disabled
297-
)
298-
299-
if (localActiveOptionIndex !== -1) {
300-
adjustedState.activeOptionIndex = localActiveOptionIndex
288+
if (props.disabled) return
289+
if (
290+
optionsRef.value &&
291+
!optionsPropsRef.value.static &&
292+
comboboxState.value === ComboboxStates.Closed
293+
) {
294+
return
301295
}
302-
}
303296

304-
let nextActiveOptionIndex = calculateActiveIndex(
305-
focus === Focus.Specific
306-
? { focus: Focus.Specific, id: id! }
307-
: { focus: focus as Exclude<Focus, Focus.Specific> },
308-
{
309-
resolveItems: () => adjustedState.options,
310-
resolveActiveIndex: () => adjustedState.activeOptionIndex,
311-
resolveId: (option) => option.id,
312-
resolveDisabled: (option) => option.dataRef.disabled,
297+
let adjustedState = adjustOrderedState()
298+
299+
// It's possible that the activeOptionIndex is set to `null` internally, but
300+
// this means that we will fallback to the first non-disabled option by default.
301+
// We have to take this into account.
302+
if (adjustedState.activeOptionIndex === null) {
303+
let localActiveOptionIndex = adjustedState.options.findIndex(
304+
(option) => !option.dataRef.disabled
305+
)
306+
307+
if (localActiveOptionIndex !== -1) {
308+
adjustedState.activeOptionIndex = localActiveOptionIndex
309+
}
313310
}
314-
)
315311

316-
activeOptionIndex.value = nextActiveOptionIndex
317-
activationTrigger.value = trigger ?? ActivationTrigger.Other
318-
options.value = adjustedState.options
312+
let nextActiveOptionIndex = calculateActiveIndex(
313+
focus === Focus.Specific
314+
? { focus: Focus.Specific, id: id! }
315+
: { focus: focus as Exclude<Focus, Focus.Specific> },
316+
{
317+
resolveItems: () => adjustedState.options,
318+
resolveActiveIndex: () => adjustedState.activeOptionIndex,
319+
resolveId: (option) => option.id,
320+
resolveDisabled: (option) => option.dataRef.disabled,
321+
}
322+
)
323+
324+
activeOptionIndex.value = nextActiveOptionIndex
325+
activationTrigger.value = trigger ?? ActivationTrigger.Other
326+
options.value = adjustedState.options
327+
})
319328
},
320329
selectOption(id: string) {
321330
let option = options.value.find((item) => item.id === id)
@@ -369,8 +378,14 @@ export let Combobox = defineComponent({
369378
api.goToOption(Focus.Specific, id)
370379
},
371380
registerOption(id: string, dataRef: ComboboxOptionData) {
381+
if (orderOptionsRaf) cancelAnimationFrame(orderOptionsRaf)
382+
372383
let option = { id, dataRef }
373-
let adjustedState = adjustOrderedState((options) => [...options, option])
384+
385+
let adjustedState = adjustOrderedState((options) => {
386+
options.push(option)
387+
return options
388+
})
374389

375390
// Check if we have a selected value that we can make active.
376391
if (activeOptionIndex.value === null) {
@@ -394,7 +409,7 @@ export let Combobox = defineComponent({
394409

395410
// If some of the DOM elements aren't ready yet, then we can retry in the next tick.
396411
if (adjustedState.options.some((option) => !dom(option.dataRef.domRef))) {
397-
requestAnimationFrame(() => {
412+
orderOptionsRaf = requestAnimationFrame(() => {
398413
let adjustedState = adjustOrderedState()
399414
options.value = adjustedState.options
400415
activeOptionIndex.value = adjustedState.activeOptionIndex

‎packages/playground-react/data.ts‎

Lines changed: 251 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,251 @@
1+
export let countries = [
2+
'Afghanistan',
3+
'Albania',
4+
'Algeria',
5+
'American Samoa',
6+
'Andorra',
7+
'Angola',
8+
'Anguilla',
9+
'Antarctica',
10+
'Antigua and Barbuda',
11+
'Argentina',
12+
'Armenia',
13+
'Aruba',
14+
'Australia',
15+
'Austria',
16+
'Azerbaijan',
17+
'Bahamas (the)',
18+
'Bahrain',
19+
'Bangladesh',
20+
'Barbados',
21+
'Belarus',
22+
'Belgium',
23+
'Belize',
24+
'Benin',
25+
'Bermuda',
26+
'Bhutan',
27+
'Bolivia (Plurinational State of)',
28+
'Bonaire, Sint Eustatius and Saba',
29+
'Bosnia and Herzegovina',
30+
'Botswana',
31+
'Bouvet Island',
32+
'Brazil',
33+
'British Indian Ocean Territory (the)',
34+
'Brunei Darussalam',
35+
'Bulgaria',
36+
'Burkina Faso',
37+
'Burundi',
38+
'Cabo Verde',
39+
'Cambodia',
40+
'Cameroon',
41+
'Canada',
42+
'Cayman Islands (the)',
43+
'Central African Republic (the)',
44+
'Chad',
45+
'Chile',
46+
'China',
47+
'Christmas Island',
48+
'Cocos (Keeling) Islands (the)',
49+
'Colombia',
50+
'Comoros (the)',
51+
'Congo (the Democratic Republic of the)',
52+
'Congo (the)',
53+
'Cook Islands (the)',
54+
'Costa Rica',
55+
'Croatia',
56+
'Cuba',
57+
'Curaçao',
58+
'Cyprus',
59+
'Czechia',
60+
"Côte d'Ivoire",
61+
'Denmark',
62+
'Djibouti',
63+
'Dominica',
64+
'Dominican Republic (the)',
65+
'Ecuador',
66+
'Egypt',
67+
'El Salvador',
68+
'Equatorial Guinea',
69+
'Eritrea',
70+
'Estonia',
71+
'Eswatini',
72+
'Ethiopia',
73+
'Falkland Islands (the) [Malvinas]',
74+
'Faroe Islands (the)',
75+
'Fiji',
76+
'Finland',
77+
'France',
78+
'French Guiana',
79+
'French Polynesia',
80+
'French Southern Territories (the)',
81+
'Gabon',
82+
'Gambia (the)',
83+
'Georgia',
84+
'Germany',
85+
'Ghana',
86+
'Gibraltar',
87+
'Greece',
88+
'Greenland',
89+
'Grenada',
90+
'Guadeloupe',
91+
'Guam',
92+
'Guatemala',
93+
'Guernsey',
94+
'Guinea',
95+
'Guinea-Bissau',
96+
'Guyana',
97+
'Haiti',
98+
'Heard Island and McDonald Islands',
99+
'Holy See (the)',
100+
'Honduras',
101+
'Hong Kong',
102+
'Hungary',
103+
'Iceland',
104+
'India',
105+
'Indonesia',
106+
'Iran (Islamic Republic of)',
107+
'Iraq',
108+
'Ireland',
109+
'Isle of Man',
110+
'Israel',
111+
'Italy',
112+
'Jamaica',
113+
'Japan',
114+
'Jersey',
115+
'Jordan',
116+
'Kazakhstan',
117+
'Kenya',
118+
'Kiribati',
119+
"Korea (the Democratic People's Republic of)",
120+
'Korea (the Republic of)',
121+
'Kuwait',
122+
'Kyrgyzstan',
123+
"Lao People's Democratic Republic (the)",
124+
'Latvia',
125+
'Lebanon',
126+
'Lesotho',
127+
'Liberia',
128+
'Libya',
129+
'Liechtenstein',
130+
'Lithuania',
131+
'Luxembourg',
132+
'Macao',
133+
'Madagascar',
134+
'Malawi',
135+
'Malaysia',
136+
'Maldives',
137+
'Mali',
138+
'Malta',
139+
'Marshall Islands (the)',
140+
'Martinique',
141+
'Mauritania',
142+
'Mauritius',
143+
'Mayotte',
144+
'Mexico',
145+
'Micronesia (Federated States of)',
146+
'Moldova (the Republic of)',
147+
'Monaco',
148+
'Mongolia',
149+
'Montenegro',
150+
'Montserrat',
151+
'Morocco',
152+
'Mozambique',
153+
'Myanmar',
154+
'Namibia',
155+
'Nauru',
156+
'Nepal',
157+
'Netherlands (the)',
158+
'New Caledonia',
159+
'New Zealand',
160+
'Nicaragua',
161+
'Niger (the)',
162+
'Nigeria',
163+
'Niue',
164+
'Norfolk Island',
165+
'Northern Mariana Islands (the)',
166+
'Norway',
167+
'Oman',
168+
'Pakistan',
169+
'Palau',
170+
'Palestine, State of',
171+
'Panama',
172+
'Papua New Guinea',
173+
'Paraguay',
174+
'Peru',
175+
'Philippines (the)',
176+
'Pitcairn',
177+
'Poland',
178+
'Portugal',
179+
'Puerto Rico',
180+
'Qatar',
181+
'Republic of North Macedonia',
182+
'Romania',
183+
'Russian Federation (the)',
184+
'Rwanda',
185+
'Réunion',
186+
'Saint Barthélemy',
187+
'Saint Helena, Ascension and Tristan da Cunha',
188+
'Saint Kitts and Nevis',
189+
'Saint Lucia',
190+
'Saint Martin (French part)',
191+
'Saint Pierre and Miquelon',
192+
'Saint Vincent and the Grenadines',
193+
'Samoa',
194+
'San Marino',
195+
'Sao Tome and Principe',
196+
'Saudi Arabia',
197+
'Senegal',
198+
'Serbia',
199+
'Seychelles',
200+
'Sierra Leone',
201+
'Singapore',
202+
'Sint Maarten (Dutch part)',
203+
'Slovakia',
204+
'Slovenia',
205+
'Solomon Islands',
206+
'Somalia',
207+
'South Africa',
208+
'South Georgia and the South Sandwich Islands',
209+
'South Sudan',
210+
'Spain',
211+
'Sri Lanka',
212+
'Sudan (the)',
213+
'Suriname',
214+
'Svalbard and Jan Mayen',
215+
'Sweden',
216+
'Switzerland',
217+
'Syrian Arab Republic',
218+
'Taiwan',
219+
'Tajikistan',
220+
'Tanzania, United Republic of',
221+
'Thailand',
222+
'Timor-Leste',
223+
'Togo',
224+
'Tokelau',
225+
'Tonga',
226+
'Trinidad and Tobago',
227+
'Tunisia',
228+
'Turkey',
229+
'Turkmenistan',
230+
'Turks and Caicos Islands (the)',
231+
'Tuvalu',
232+
'Uganda',
233+
'Ukraine',
234+
'United Arab Emirates (the)',
235+
'United Kingdom of Great Britain and Northern Ireland (the)',
236+
'United States Minor Outlying Islands (the)',
237+
'United States of America (the)',
238+
'Uruguay',
239+
'Uzbekistan',
240+
'Vanuatu',
241+
'Venezuela (Bolivarian Republic of)',
242+
'Viet Nam',
243+
'Virgin Islands (British)',
244+
'Virgin Islands (U.S.)',
245+
'Wallis and Futuna',
246+
'Western Sahara',
247+
'Yemen',
248+
'Zambia',
249+
'Zimbabwe',
250+
'Åland Islands',
251+
]

0 commit comments

Comments
 (0)