Skip to content

Commit 6cc5451

Browse files
sarahdayansarahdayan
committed
fix: handle late resolving promises with promise cancelation
1 parent 47b5982 commit 6cc5451

13 files changed

Lines changed: 708 additions & 516 deletions

‎packages/autocomplete-core/package.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@
3131
"watch": "watch \"yarn on:change\" --ignoreDirectoryPattern \"/dist/\""
3232
},
3333
"dependencies": {
34-
"@algolia/autocomplete-shared": "1.5.1"
34+
"@algolia/autocomplete-shared": "1.5.1",
35+
"cancelable-promise": "4.2.1"
3536
},
3637
"devDependencies": {
3738
"@algolia/autocomplete-preset-algolia": "1.5.1",

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

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

‎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: 9 additions & 9 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 === true || !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
},
@@ -212,8 +212,8 @@ export function getPropGetters<
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
},

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

Lines changed: 47 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import CancelablePromise, { cancelable } from 'cancelable-promise';
2+
13
import { reshape } from './reshape';
24
import { preResolve, resolve, postResolve } from './resolve';
35
import {
@@ -37,7 +39,7 @@ export function onInput<TItem extends BaseItem>({
3739
refresh,
3840
store,
3941
...setters
40-
}: OnInputParams<TItem>): Promise<void> {
42+
}: OnInputParams<TItem>): CancelablePromise<void> {
4143
if (lastStalledId) {
4244
props.environment.clearTimeout(lastStalledId);
4345
}
@@ -69,7 +71,13 @@ export function onInput<TItem extends BaseItem>({
6971
// promises to keep late resolving promises from "cancelling" the state
7072
// updates performed in this code path.
7173
// We chain with a void promise to respect `onInput`'s expected return type.
72-
return runConcurrentSafePromise(collections).then(() => Promise.resolve());
74+
const request = cancelable(
75+
runConcurrentSafePromise(collections).then(() => Promise.resolve())
76+
);
77+
78+
store.pendingRequests.add(request);
79+
80+
return request;
7381
}
7482

7583
setStatus('loading');
@@ -84,35 +92,37 @@ export function onInput<TItem extends BaseItem>({
8492
// We don't track nested promises and only rely on the full chain resolution,
8593
// meaning we should only ever manipulate the state once this concurrent-safe
8694
// promise is resolved.
87-
return runConcurrentSafePromise(
88-
props
89-
.getSources({
90-
query,
91-
refresh,
92-
state: store.getState(),
93-
...setters,
94-
})
95-
.then((sources) => {
96-
return Promise.all(
97-
sources.map((source) => {
98-
return Promise.resolve(
99-
source.getItems({
100-
query,
101-
refresh,
102-
state: store.getState(),
103-
...setters,
104-
})
105-
).then((itemsOrDescription) =>
106-
preResolve<TItem>(itemsOrDescription, source.sourceId)
95+
const request = cancelable(
96+
runConcurrentSafePromise(
97+
props
98+
.getSources({
99+
query,
100+
refresh,
101+
state: store.getState(),
102+
...setters,
103+
})
104+
.then((sources) => {
105+
return Promise.all(
106+
sources.map((source) => {
107+
return Promise.resolve(
108+
source.getItems({
109+
query,
110+
refresh,
111+
state: store.getState(),
112+
...setters,
113+
})
114+
).then((itemsOrDescription) =>
115+
preResolve<TItem>(itemsOrDescription, source.sourceId)
116+
);
117+
})
118+
)
119+
.then(resolve)
120+
.then((responses) => postResolve(responses, sources))
121+
.then((collections) =>
122+
reshape({ collections, props, state: store.getState() })
107123
);
108-
})
109-
)
110-
.then(resolve)
111-
.then((responses) => postResolve(responses, sources))
112-
.then((collections) =>
113-
reshape({ collections, props, state: store.getState() })
114-
);
115-
})
124+
})
125+
)
116126
)
117127
.then((collections) => {
118128
// Parameters passed to `onInput` could be stale when the following code
@@ -122,14 +132,6 @@ export function onInput<TItem extends BaseItem>({
122132

123133
setStatus('idle');
124134

125-
if (store.shouldSkipPendingUpdate) {
126-
if (!runConcurrentSafePromise.isRunning()) {
127-
store.shouldSkipPendingUpdate = false;
128-
}
129-
130-
return;
131-
}
132-
133135
setCollections(collections as any);
134136

135137
const isPanelOpen = props.shouldPanelOpen({ state: store.getState() });
@@ -157,10 +159,14 @@ export function onInput<TItem extends BaseItem>({
157159
}
158160
})
159161
.finally(() => {
162+
setStatus('idle');
163+
160164
if (lastStalledId) {
161165
props.environment.clearTimeout(lastStalledId);
162166
}
163-
});
164-
}
167+
}, true);
168+
169+
store.pendingRequests.add(request);
165170

166-
onInput.isRunning = runConcurrentSafePromise.isRunning;
171+
return request;
172+
}

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,7 @@ export function onKeyDown<TItem extends BaseItem>({
104104
// autocomplete. At this point, we should ignore any requests that are still
105105
// running and could reopen the panel once they resolve, because that would
106106
// result in an unsolicited UI behavior.
107-
if (onInput.isRunning()) {
108-
store.shouldSkipPendingUpdate = true;
109-
}
107+
store.pendingRequests.cancelAll();
110108
} else if (event.key === 'Enter') {
111109
// No active item, so we let the browser handle the native `onSubmit` form
112110
// event.

‎packages/autocomplete-core/src/types/AutocompleteStore.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1+
import { CancelablePromiseQueue } from '../utils';
2+
13
import { BaseItem } from './AutocompleteApi';
24
import { InternalAutocompleteOptions } from './AutocompleteOptions';
35
import { AutocompleteState } from './AutocompleteState';
46

57
export interface AutocompleteStore<TItem extends BaseItem> {
68
getState(): AutocompleteState<TItem>;
79
dispatch(action: ActionType, payload: any): void;
8-
shouldSkipPendingUpdate: boolean;
10+
pendingRequests: CancelablePromiseQueue;
911
}
1012

1113
export type Reducer = <TItem extends BaseItem>(

0 commit comments

Comments
 (0)