Skip to content

Commit 9640c2d

Browse files
sarahdayanfrancoischalifourHaroenv
authored
fix: handle late resolving promises with promise cancelation (#864)
Co-authored-by: François Chalifour <[email protected]> Co-authored-by: Haroen Viaene <[email protected]>
1 parent 5c29a20 commit 9640c2d

15 files changed

+927
-143
lines changed

‎packages/autocomplete-core/src/__tests__/concurrency.test.ts

Lines changed: 98 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
1+
import { noop } from '@algolia/autocomplete-shared';
12
import userEvent from '@testing-library/user-event';
23

34
import { AutocompleteState } from '..';
4-
import { createPlayground, createSource, defer } from '../../../../test/utils';
5+
import {
6+
createPlayground,
7+
createSource,
8+
defer,
9+
runAllMicroTasks,
10+
} from '../../../../test/utils';
511
import { createAutocomplete } from '../createAutocomplete';
612

713
type Item = {
@@ -31,7 +37,7 @@ describe('concurrency', () => {
3137
userEvent.type(input, 'b');
3238
userEvent.type(input, 'c');
3339

34-
await defer(() => {}, timeout);
40+
await defer(noop, timeout);
3541

3642
let stateHistory: Array<
3743
AutocompleteState<Item>
@@ -57,7 +63,7 @@ describe('concurrency', () => {
5763

5864
userEvent.type(input, '{backspace}'.repeat(3));
5965

60-
await defer(() => {}, timeout);
66+
await defer(noop, timeout);
6167

6268
stateHistory = onStateChange.mock.calls.flatMap((x) => x[0].state);
6369

@@ -88,19 +94,44 @@ describe('concurrency', () => {
8894
getSources,
8995
});
9096

91-
userEvent.type(inputElement, 'ab{esc}');
97+
userEvent.type(inputElement, 'ab');
9298

93-
await defer(() => {}, timeout);
99+
// The search request is triggered
100+
expect(onStateChange).toHaveBeenLastCalledWith(
101+
expect.objectContaining({
102+
state: expect.objectContaining({
103+
status: 'loading',
104+
query: 'ab',
105+
}),
106+
})
107+
);
94108

109+
userEvent.type(inputElement, '{esc}');
110+
111+
// The status is immediately set to "idle" and the panel is closed
95112
expect(onStateChange).toHaveBeenLastCalledWith(
96113
expect.objectContaining({
97114
state: expect.objectContaining({
115+
status: 'idle',
98116
isOpen: false,
117+
query: '',
118+
}),
119+
})
120+
);
121+
122+
await defer(noop, timeout);
123+
124+
// Once the request is settled, the state remains unchanged
125+
expect(onStateChange).toHaveBeenLastCalledWith(
126+
expect.objectContaining({
127+
state: expect.objectContaining({
99128
status: 'idle',
129+
isOpen: false,
100130
}),
101131
})
102132
);
103-
expect(getSources).toHaveBeenCalledTimes(2);
133+
134+
expect(getSources).toHaveBeenCalledTimes(3);
104135
});
105136

106137
test('keeps the panel closed on blur', async () => {
@@ -115,19 +146,46 @@ describe('concurrency', () => {
115146
getSources,
116147
});
117148

118-
userEvent.type(inputElement, 'a{enter}');
149+
userEvent.type(inputElement, 'a');
150+
151+
await runAllMicroTasks();
152+
153+
// The search request is triggered
154+
expect(onStateChange).toHaveBeenLastCalledWith(
155+
expect.objectContaining({
156+
state: expect.objectContaining({
157+
status: 'loading',
158+
query: 'a',
159+
}),
160+
})
161+
);
119162

120-
await defer(() => {}, timeout);
163+
userEvent.type(inputElement, '{enter}');
121164

165+
// The status is immediately set to "idle" and the panel is closed
122166
expect(onStateChange).toHaveBeenLastCalledWith(
123167
expect.objectContaining({
124168
state: expect.objectContaining({
169+
status: 'idle',
125170
isOpen: false,
171+
query: 'a',
172+
}),
173+
})
174+
);
175+
176+
await defer(noop, timeout);
177+
178+
// Once the request is settled, the state remains unchanged
179+
expect(onStateChange).toHaveBeenLastCalledWith(
180+
expect.objectContaining({
181+
state: expect.objectContaining({
126182
status: 'idle',
183+
isOpen: false,
127184
}),
128185
})
129186
);
130-
expect(getSources).toHaveBeenCalledTimes(1);
187+
188+
expect(getSources).toHaveBeenCalledTimes(2);
131189
});
132190

133191
test('keeps the panel closed on touchstart blur', async () => {
@@ -156,19 +214,45 @@ describe('concurrency', () => {
156214
window.addEventListener('touchstart', onTouchStart);
157215

158216
userEvent.type(inputElement, 'a');
217+
218+
await runAllMicroTasks();
219+
220+
// The search request is triggered
221+
expect(onStateChange).toHaveBeenLastCalledWith(
222+
expect.objectContaining({
223+
state: expect.objectContaining({
224+
status: 'loading',
225+
query: 'a',
226+
}),
227+
})
228+
);
229+
159230
const customEvent = new CustomEvent('touchstart', { bubbles: true });
160231
window.document.dispatchEvent(customEvent);
161232

162-
await defer(() => {}, timeout);
163-
233+
// The status is immediately set to "idle" and the panel is closed
164234
expect(onStateChange).toHaveBeenLastCalledWith(
165235
expect.objectContaining({
166236
state: expect.objectContaining({
237+
status: 'idle',
167238
isOpen: false,
239+
query: 'a',
240+
}),
241+
})
242+
);
243+
244+
await defer(noop, timeout);
245+
246+
// Once the request is settled, the state remains unchanged
247+
expect(onStateChange).toHaveBeenLastCalledWith(
248+
expect.objectContaining({
249+
state: expect.objectContaining({
168250
status: 'idle',
251+
isOpen: false,
169252
}),
170253
})
171254
);
255+
172256
expect(getSources).toHaveBeenCalledTimes(1);
173257

174258
window.removeEventListener('touchstart', onTouchStart);
@@ -197,7 +281,7 @@ describe('concurrency', () => {
197281

198282
userEvent.type(inputElement, 'a{esc}');
199283

200-
await defer(() => {}, delay);
284+
await defer(noop, delay);
201285

202286
expect(onStateChange).toHaveBeenLastCalledWith(
203287
expect.objectContaining({
@@ -229,7 +313,7 @@ describe('concurrency', () => {
229313

230314
userEvent.type(inputElement, 'a{enter}');
231315

232-
await defer(() => {}, delay);
316+
await defer(noop, delay);
233317

234318
expect(onStateChange).toHaveBeenLastCalledWith(
235319
expect.objectContaining({
@@ -276,7 +360,7 @@ describe('concurrency', () => {
276360
const customEvent = new CustomEvent('touchstart', { bubbles: true });
277361
window.document.dispatchEvent(customEvent);
278362

279-
await defer(() => {}, delay);
363+
await defer(noop, delay);
280364

281365
expect(onStateChange).toHaveBeenLastCalledWith(
282366
expect.objectContaining({

‎packages/autocomplete-core/src/__tests__/debouncing.test.ts

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
import { noop } from '@algolia/autocomplete-shared';
2+
import userEvent from '@testing-library/user-event';
3+
4+
import { createAutocomplete, InternalAutocompleteSource } from '..';
5+
import { createPlayground, createSource, defer } from '../../../../test/utils';
6+
7+
type Source = InternalAutocompleteSource<{ label: string }>;
8+
9+
const delay = 10;
10+
11+
const debounced = debouncePromise<Source[][], Source[]>(
12+
(items) => Promise.resolve(items),
13+
delay
14+
);
15+
16+
describe('debouncing', () => {
17+
test('only submits the final query', async () => {
18+
const onStateChange = jest.fn();
19+
const getItems = jest.fn(({ query }) => [{ label: query }]);
20+
const { inputElement } = createPlayground(createAutocomplete, {
21+
onStateChange,
22+
getSources: () => debounced([createSource({ getItems })]),
23+
});
24+
25+
userEvent.type(inputElement, 'abc');
26+
27+
await defer(noop, delay);
28+
29+
expect(getItems).toHaveBeenCalledTimes(1);
30+
expect(onStateChange).toHaveBeenLastCalledWith(
31+
expect.objectContaining({
32+
state: expect.objectContaining({
33+
status: 'idle',
34+
isOpen: true,
35+
collections: expect.arrayContaining([
36+
expect.objectContaining({
37+
items: [{ __autocomplete_id: 0, label: 'abc' }],
38+
}),
39+
]),
40+
}),
41+
})
42+
);
43+
});
44+
45+
test('triggers subsequent queries after reopening the panel', async () => {
46+
const onStateChange = jest.fn();
47+
const getItems = jest.fn(({ query }) => [{ label: query }]);
48+
const { inputElement } = createPlayground(createAutocomplete, {
49+
onStateChange,
50+
getSources: () => debounced([createSource({ getItems })]),
51+
});
52+
53+
userEvent.type(inputElement, 'abc{esc}');
54+
55+
expect(onStateChange).toHaveBeenLastCalledWith(
56+
expect.objectContaining({
57+
state: expect.objectContaining({
58+
status: 'idle',
59+
isOpen: false,
60+
}),
61+
})
62+
);
63+
64+
userEvent.type(inputElement, 'def');
65+
66+
await defer(noop, delay);
67+
68+
expect(onStateChange).toHaveBeenLastCalledWith(
69+
expect.objectContaining({
70+
state: expect.objectContaining({
71+
collections: expect.arrayContaining([
72+
expect.objectContaining({
73+
items: [{ __autocomplete_id: 0, label: 'abcdef' }],
74+
}),
75+
]),
76+
status: 'idle',
77+
isOpen: true,
78+
}),
79+
})
80+
);
81+
});
82+
});
83+
84+
function debouncePromise<TParams extends unknown[], TResponse>(
85+
fn: (...params: TParams) => Promise<TResponse>,
86+
time: number
87+
) {
88+
let timerId: ReturnType<typeof setTimeout> | undefined = undefined;
89+
90+
return function (...args: TParams) {
91+
if (timerId) {
92+
clearTimeout(timerId);
93+
}
94+
95+
return new Promise<TResponse>((resolve) => {
96+
timerId = setTimeout(() => resolve(fn(...args)), time);
97+
});
98+
};
99+
}

‎packages/autocomplete-core/src/createStore.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
InternalAutocompleteOptions,
66
Reducer,
77
} from './types';
8+
import { createCancelablePromiseList } from './utils';
89

910
type OnStoreStateChange<TItem extends BaseItem> = ({
1011
prevState,
@@ -35,6 +36,6 @@ export function createStore<TItem extends BaseItem>(
3536

3637
onStoreStateChange({ state, prevState });
3738
},
38-
shouldSkipPendingUpdate: false,
39+
pendingRequests: createCancelablePromiseList(),
3940
};
4041
}

‎packages/autocomplete-core/src/getPropGetters.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,13 @@ export function getPropGetters<
4343
// The `onTouchStart` event shouldn't trigger the `blur` handler when
4444
// it's not an interaction with Autocomplete. We detect it with the
4545
// following heuristics:
46-
// - the panel is closed AND there are no running requests
46+
// - the panel is closed AND there are no pending requests
4747
// (no interaction with the autocomplete, no future state updates)
4848
// - OR the touched target is the input element (should open the panel)
49-
const isNotAutocompleteInteraction =
50-
store.getState().isOpen === false && !onInput.isRunning();
49+
const isAutocompleteInteraction =
50+
store.getState().isOpen || !store.pendingRequests.isEmpty();
5151

52-
if (isNotAutocompleteInteraction || event.target === inputElement) {
52+
if (!isAutocompleteInteraction || event.target === inputElement) {
5353
return;
5454
}
5555

@@ -62,12 +62,12 @@ export function getPropGetters<
6262
if (isTargetWithinAutocomplete === false) {
6363
store.dispatch('blur', null);
6464

65-
// If requests are still running when the user closes the panel, they
65+
// If requests are still pending when the user closes the panel, they
6666
// could reopen the panel once they resolve.
6767
// We want to prevent any subsequent query from reopening the panel
6868
// because it would result in an unsolicited UI behavior.
69-
if (!props.debug && onInput.isRunning()) {
70-
store.shouldSkipPendingUpdate = true;
69+
if (!props.debug) {
70+
store.pendingRequests.cancelAll();
7171
}
7272
}
7373
},
@@ -208,12 +208,12 @@ export function getPropGetters<
208208
if (!isTouchDevice) {
209209
store.dispatch('blur', null);
210210

211-
// If requests are still running when the user closes the panel, they
211+
// If requests are still pending when the user closes the panel, they
212212
// could reopen the panel once they resolve.
213213
// We want to prevent any subsequent query from reopening the panel
214214
// because it would result in an unsolicited UI behavior.
215-
if (!props.debug && onInput.isRunning()) {
216-
store.shouldSkipPendingUpdate = true;
215+
if (!props.debug) {
216+
store.pendingRequests.cancelAll();
217217
}
218218
}
219219
},

0 commit comments

Comments
 (0)