Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(concurrency): ensure panel stays closed after blur #829

Merged
merged 30 commits into from Dec 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
8d57e22
fix(onInput): ensure used values in callbacks aren't stale
sarahdayan Nov 24, 2021
5fdbdf1
fix(concurrency): ensure panel stays closed after blur
sarahdayan Nov 24, 2021
071d912
style: lint
sarahdayan Nov 25, 2021
58f5144
fix(concurrency): schedule panel closing on blur when not debug
sarahdayan Nov 25, 2021
3e38754
fix(onInput): skip query when next panel state is closed and requests…
sarahdayan Nov 25, 2021
20b4d02
test: fix flaky test
sarahdayan Nov 25, 2021
0659502
chore: increase bundle size
sarahdayan Nov 25, 2021
e75feb6
fix(onKeyDown): reproduce blur behavior on escape
sarahdayan Nov 26, 2021
61ff898
test(concurrency): test concurrent closing behaviors
sarahdayan Nov 26, 2021
ce95062
fix(getPropGetters): better infer the need for ignoring touchstart event
sarahdayan Nov 26, 2021
57645e0
test(concurrency): better isolate tests to avoid false negatives
sarahdayan Nov 26, 2021
117eeee
fix(onInput): better infer the need to ignore a request
sarahdayan Nov 26, 2021
4c83f3a
fix(concurrency): ensure no extra request is triggered
sarahdayan Nov 26, 2021
c33c3a2
chore(bundlesize): increase bundle size
sarahdayan Nov 26, 2021
e2a701a
refactor(concurrency): rely on running state rather than idleness
sarahdayan Nov 26, 2021
7e7b4fe
fix(createConcurrentSafePromise): better infer whether promises are r…
sarahdayan Nov 26, 2021
f365e06
style: lint
sarahdayan Nov 26, 2021
962e27a
refactor: rename variables
sarahdayan Dec 3, 2021
6cf2fb9
refactor: rename variables
sarahdayan Dec 3, 2021
f24741e
style: disable ESLint rule
sarahdayan Dec 3, 2021
b47de3f
chore: add comments
sarahdayan Dec 3, 2021
9348a63
fix: apply suggestions from code review
sarahdayan Dec 6, 2021
d5f903a
fix(concurrency): ensure panel stays closed after blur (patch) (#831)
francoischalifour Dec 6, 2021
6fdf279
refactor: rmeove no longer needed logic
sarahdayan Dec 7, 2021
daa4705
refactor: remove code we don't yet need
sarahdayan Dec 7, 2021
3dd7947
chore: add link to sandbox
sarahdayan Dec 7, 2021
d663200
refactor: rename internal property
sarahdayan Dec 8, 2021
8280fb6
chore: fix comments
sarahdayan Dec 8, 2021
43107e5
refactor: clarify branch$
sarahdayan Dec 8, 2021
ac2b33f
fix: ensure panel stays closed with many pending requests
sarahdayan Dec 8, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions bundlesize.config.json
Expand Up @@ -2,11 +2,11 @@
"files": [
{
"path": "packages/autocomplete-core/dist/umd/index.production.js",
"maxSize": "5.75 kB"
"maxSize": "6 kB"
},
{
"path": "packages/autocomplete-js/dist/umd/index.production.js",
"maxSize": "16.25 kB"
"maxSize": "16.5 kB"
},
{
"path": "packages/autocomplete-preset-algolia/dist/umd/index.production.js",
Expand Down
294 changes: 265 additions & 29 deletions packages/autocomplete-core/src/__tests__/concurrency.test.ts
@@ -1,39 +1,25 @@
import userEvent from '@testing-library/user-event';

import { AutocompleteState } from '..';
import { createSource, defer } from '../../../../test/utils';
import { createPlayground, createSource, defer } from '../../../../test/utils';
import { createAutocomplete } from '../createAutocomplete';

type Item = {
label: string;
};

beforeEach(() => {
document.body.innerHTML = '';
});

describe('concurrency', () => {
test('resolves the responses in order from getSources', async () => {
// These delays make the second query come back after the third one.
const sourcesDelays = [100, 150, 200];
const itemsDelays = [0, 150, 0];
let deferSourcesCount = -1;
let deferItemsCount = -1;

const getSources = ({ query }) => {
deferSourcesCount++;

return defer(() => {
return [
createSource({
getItems() {
deferItemsCount++;

return defer(
() => [{ label: query }],
itemsDelays[deferItemsCount]
);
},
}),
];
}, sourcesDelays[deferSourcesCount]);
};
const { timeout, delayedGetSources: getSources } = createDelayedGetSources({
// These delays make the second query come back after the third one.
sources: [100, 150, 200],
items: [0, 150, 0],
});

const onStateChange = jest.fn();
const autocomplete = createAutocomplete({ getSources, onStateChange });
const { onChange } = autocomplete.getInputProps({ inputElement: null });
Expand All @@ -45,10 +31,6 @@ describe('concurrency', () => {
userEvent.type(input, 'b');
userEvent.type(input, 'c');

const timeout = Math.max(
...sourcesDelays.map((delay, index) => delay + itemsDelays[index])
);

await defer(() => {}, timeout);

let stateHistory: Array<
Expand Down Expand Up @@ -91,4 +73,258 @@ describe('concurrency', () => {

document.body.removeChild(input);
});

describe('closing the panel with pending requests', () => {
describe('without debug mode', () => {
test('keeps the panel closed on Escape', async () => {
const onStateChange = jest.fn();
const { timeout, delayedGetSources } = createDelayedGetSources({
sources: [100, 200],
});
const getSources = jest.fn(delayedGetSources);

const { inputElement } = createPlayground(createAutocomplete, {
onStateChange,
getSources,
});

userEvent.type(inputElement, 'ab{esc}');

await defer(() => {}, timeout);

expect(onStateChange).toHaveBeenLastCalledWith(
expect.objectContaining({
state: expect.objectContaining({
isOpen: false,
status: 'idle',
}),
})
);
expect(getSources).toHaveBeenCalledTimes(2);
});

test('keeps the panel closed on blur', async () => {
const onStateChange = jest.fn();
const { timeout, delayedGetSources } = createDelayedGetSources({
sources: [100, 200],
});
const getSources = jest.fn(delayedGetSources);

const { inputElement } = createPlayground(createAutocomplete, {
onStateChange,
getSources,
});

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

await defer(() => {}, timeout);

expect(onStateChange).toHaveBeenLastCalledWith(
expect.objectContaining({
state: expect.objectContaining({
isOpen: false,
status: 'idle',
}),
})
);
expect(getSources).toHaveBeenCalledTimes(1);
});

test('keeps the panel closed on touchstart blur', async () => {
const onStateChange = jest.fn();
const { timeout, delayedGetSources } = createDelayedGetSources({
sources: [100, 200],
});
const getSources = jest.fn(delayedGetSources);

const {
getEnvironmentProps,
inputElement,
formElement,
} = createPlayground(createAutocomplete, {
onStateChange,
getSources,
});

const panelElement = document.createElement('div');

const { onTouchStart } = getEnvironmentProps({
inputElement,
formElement,
panelElement,
});
window.addEventListener('touchstart', onTouchStart);

userEvent.type(inputElement, 'a');
const customEvent = new CustomEvent('touchstart', { bubbles: true });
window.document.dispatchEvent(customEvent);

await defer(() => {}, timeout);

expect(onStateChange).toHaveBeenLastCalledWith(
expect.objectContaining({
state: expect.objectContaining({
isOpen: false,
status: 'idle',
}),
})
);
expect(getSources).toHaveBeenCalledTimes(1);

window.removeEventListener('touchstart', onTouchStart);
});
});

describe('with debug mode', () => {
const delay = 300;

test('keeps the panel closed on Escape', async () => {
const onStateChange = jest.fn();
const getSources = jest.fn(() => {
return defer(() => {
return [
createSource({
getItems: () => [{ label: '1' }, { label: '2' }],
}),
];
}, delay);
});
const { inputElement } = createPlayground(createAutocomplete, {
debug: true,
onStateChange,
getSources,
});

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

await defer(() => {}, delay);

expect(onStateChange).toHaveBeenLastCalledWith(
expect.objectContaining({
state: expect.objectContaining({
isOpen: false,
status: 'idle',
}),
})
);
expect(getSources).toHaveBeenCalledTimes(1);
});

test('keeps the panel open on blur', async () => {
const onStateChange = jest.fn();
const getSources = jest.fn(() => {
return defer(() => {
return [
createSource({
getItems: () => [{ label: '1' }, { label: '2' }],
}),
];
}, delay);
});
const { inputElement } = createPlayground(createAutocomplete, {
debug: true,
onStateChange,
getSources,
});

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

await defer(() => {}, delay);

expect(onStateChange).toHaveBeenLastCalledWith(
expect.objectContaining({
state: expect.objectContaining({
isOpen: true,
status: 'idle',
}),
})
);
expect(getSources).toHaveBeenCalledTimes(1);
});

test('keeps the panel open on touchstart blur', async () => {
const onStateChange = jest.fn();
const getSources = jest.fn(() => {
return defer(() => {
return [
createSource({
getItems: () => [{ label: '1' }, { label: '2' }],
}),
];
}, delay);
});
const {
getEnvironmentProps,
inputElement,
formElement,
} = createPlayground(createAutocomplete, {
debug: true,
onStateChange,
getSources,
});

const panelElement = document.createElement('div');

const { onTouchStart } = getEnvironmentProps({
inputElement,
formElement,
panelElement,
});
window.addEventListener('touchstart', onTouchStart);

userEvent.type(inputElement, 'a');
const customEvent = new CustomEvent('touchstart', { bubbles: true });
window.document.dispatchEvent(customEvent);

await defer(() => {}, delay);

expect(onStateChange).toHaveBeenLastCalledWith(
expect.objectContaining({
state: expect.objectContaining({
isOpen: true,
status: 'idle',
}),
})
);
expect(getSources).toHaveBeenCalledTimes(1);

window.removeEventListener('touchstart', onTouchStart);
});
});
});
});

function createDelayedGetSources(delays: {
sources: number[];
items?: number[];
}) {
let deferSourcesCount = -1;
let deferItemsCount = -1;

const itemsDelays = delays.items || delays.sources.map(() => 0);

const timeout = Math.max(
...delays.sources.map((delay, index) => delay + itemsDelays[index])
);

function delayedGetSources({ query }) {
deferSourcesCount++;

return defer(() => {
return [
createSource({
getItems() {
deferItemsCount++;

return defer(
() => [{ label: query }],
itemsDelays[deferItemsCount]
);
},
}),
];
}, delays.sources[deferSourcesCount]);
}

return { timeout, delayedGetSources };
}
Expand Up @@ -30,7 +30,7 @@ describe('getEnvironmentProps', () => {
});

describe('onTouchStart', () => {
test('is a noop when panel is not open', () => {
test('is a noop when panel is not open and status is idle', () => {
const onStateChange = jest.fn();
const {
getEnvironmentProps,
Expand Down
Expand Up @@ -1894,7 +1894,7 @@ describe('getInputProps', () => {
});

describe('onBlur', () => {
test('resets activeItemId and isOpen', () => {
test('resets activeItemId and isOpen', async () => {
const onStateChange = jest.fn();
const { inputElement } = createPlayground(createAutocomplete, {
onStateChange,
Expand All @@ -1905,6 +1905,8 @@ describe('getInputProps', () => {
inputElement.focus();
inputElement.blur();

await runAllMicroTasks();
sarahdayan marked this conversation as resolved.
Show resolved Hide resolved

expect(onStateChange).toHaveBeenLastCalledWith(
expect.objectContaining({
state: expect.objectContaining({
Expand Down
12 changes: 11 additions & 1 deletion packages/autocomplete-core/src/__tests__/getSources.test.ts
Expand Up @@ -8,6 +8,10 @@ import {
import { createAutocomplete } from '../createAutocomplete';
import * as handlers from '../onInput';

beforeEach(() => {
document.body.innerHTML = '';
});

describe('getSources', () => {
test('gets calls on input', () => {
const getSources = jest.fn((..._args: any[]) => {
Expand Down Expand Up @@ -140,7 +144,13 @@ describe('getSources', () => {

const { inputElement } = createPlayground(createAutocomplete, {
getSources() {
return [createSource({ sourceId: 'source1', getItems: () => {} })];
return [
createSource({
sourceId: 'source1',
// @ts-expect-error
getItems: () => {},
}),
];
},
});

Expand Down
1 change: 1 addition & 0 deletions packages/autocomplete-core/src/createStore.ts
Expand Up @@ -35,5 +35,6 @@ export function createStore<TItem extends BaseItem>(

onStoreStateChange({ state, prevState });
},
shouldSkipPendingUpdate: false,
};
}