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.

Sign up for GitHub

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(autocomplete-js): display warning when there are more than one instances of autocomplete #1108

Merged
merged 4 commits into from
Mar 16, 2023
Merged

fix(autocomplete-js): display warning when there are more than one instances of autocomplete #1108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bf8894f
fix(autocomplete-js): display warning when there are more than one in…
dhayab Mar 13, 2023
c7807e6
fix tests
dhayab Mar 13, 2023
e0f9591
apply suggestions
dhayab Mar 13, 2023
2c6e81f
direct link to destroy param
dhayab Mar 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions packages/autocomplete-js/src/__tests__/autocomplete.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,26 @@ beforeEach(() => {
});

describe('autocomplete-js', () => {
test('warns when more than one instance is detected in a document', () => {
const firstContainer = document.createElement('div');
expect(() =>
autocomplete({
container: firstContainer,
})
).not.toWarnDev();

const secondContainer = document.createElement('div');
expect(() =>
autocomplete({
container: secondContainer,
})
).toWarnDev(
`[Autocomplete] Autocomplete doesn't support multiple instances running at the same time. Make sure to destroy the previous instance before creating a new one.

See: https://www.algolia.com/doc/ui-libraries/autocomplete/api-reference/autocomplete-js/autocomplete/#param-destroy`
);
});

test('renders with default options', () => {
const container = document.createElement('div');
autocomplete<{ label: string }>({
Expand Down
51 changes: 38 additions & 13 deletions packages/autocomplete-js/src/__tests__/renderer.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ describe('renderer', () => {
const panelContainer = document.createElement('div');

document.body.appendChild(panelContainer);
autocomplete<{ label: string }>({
const { destroy } = autocomplete<{ label: string }>({
container,
panelContainer,
initialState: {
Expand Down Expand Up @@ -53,6 +53,8 @@ describe('renderer', () => {
render(createElement(Fragment, null, 'testSource'), root);
},
});

destroy();
});

test('accepts a custom renderer', () => {
Expand All @@ -66,7 +68,7 @@ describe('renderer', () => {

document.body.appendChild(panelContainer);

autocomplete<{ label: string }>({
const { destroy } = autocomplete<{ label: string }>({
container,
panelContainer,
initialState: {
Expand Down Expand Up @@ -110,6 +112,8 @@ describe('renderer', () => {
render: mockRender,
},
});

destroy();
});

test('defaults `render` when not specified in the renderer', () => {
Expand All @@ -122,7 +126,7 @@ describe('renderer', () => {

document.body.appendChild(panelContainer);

autocomplete<{ label: string }>({
const { destroy } = autocomplete<{ label: string }>({
container,
panelContainer,
initialState: {
Expand Down Expand Up @@ -153,6 +157,8 @@ describe('renderer', () => {
Fragment: CustomFragment,
},
});

destroy();
});

test('uses a custom `render` via `renderer`', async () => {
Expand All @@ -165,7 +171,7 @@ describe('renderer', () => {
const mockCreateElement = jest.fn(preactCreateElement);
const mockRender = jest.fn().mockImplementation(preactRender);

autocomplete<{ label: string }>({
const { destroy } = autocomplete<{ label: string }>({
container,
panelContainer,
id: 'autocomplete-0',
Expand Down Expand Up @@ -238,6 +244,8 @@ describe('renderer', () => {
</div>
`);
});

destroy();
});

test('warns about renderer mismatch when specifying an incomplete renderer', () => {
Expand All @@ -249,8 +257,10 @@ describe('renderer', () => {

document.body.appendChild(panelContainer);

let instance;

expect(() => {
autocomplete<{ label: string }>({
instance = autocomplete<{ label: string }>({
container,
panelContainer,
initialState: {
Expand Down Expand Up @@ -280,9 +290,10 @@ describe('renderer', () => {
'[Autocomplete] You provided an incomplete `renderer` (missing: `renderer.render`). This can cause rendering issues.' +
'\nSee https://www.algolia.com/doc/ui-libraries/autocomplete/api-reference/autocomplete-js/autocomplete/#param-renderer'
);
instance.destroy();

expect(() => {
autocomplete<{ label: string }>({
instance = autocomplete<{ label: string }>({
container,
panelContainer,
initialState: {
Expand Down Expand Up @@ -315,9 +326,10 @@ describe('renderer', () => {
'[Autocomplete] You provided an incomplete `renderer` (missing: `renderer.createElement`). This can cause rendering issues.' +
'\nSee https://www.algolia.com/doc/ui-libraries/autocomplete/api-reference/autocomplete-js/autocomplete/#param-renderer'
);
instance.destroy();

expect(() => {
autocomplete<{ label: string }>({
instance = autocomplete<{ label: string }>({
container,
panelContainer,
initialState: {
Expand Down Expand Up @@ -350,9 +362,10 @@ describe('renderer', () => {
'[Autocomplete] You provided an incomplete `renderer` (missing: `renderer.Fragment`). This can cause rendering issues.' +
'\nSee https://www.algolia.com/doc/ui-libraries/autocomplete/api-reference/autocomplete-js/autocomplete/#param-renderer'
);
instance.destroy();

expect(() => {
autocomplete<{ label: string }>({
instance = autocomplete<{ label: string }>({
container,
panelContainer,
initialState: {
Expand Down Expand Up @@ -384,6 +397,7 @@ describe('renderer', () => {
'[Autocomplete] You provided an incomplete `renderer` (missing: `renderer.Fragment`, `renderer.render`). This can cause rendering issues.' +
'\nSee https://www.algolia.com/doc/ui-libraries/autocomplete/api-reference/autocomplete-js/autocomplete/#param-renderer'
);
instance.destroy();
});

test('warns about new `renderer.render` option when specifying an incomplete renderer and a `render` option', () => {
Expand All @@ -394,8 +408,9 @@ describe('renderer', () => {

document.body.appendChild(panelContainer);

let instance;
function startAutocomplete() {
autocomplete<{ label: string }>({
instance = autocomplete<{ label: string }>({
container,
panelContainer,
initialState: {
Expand Down Expand Up @@ -434,11 +449,13 @@ describe('renderer', () => {
'\n- If you are using the `render` option to work with React 18, pass an empty `render` function into `renderer`.' +
'\nSee https://www.algolia.com/doc/ui-libraries/autocomplete/api-reference/autocomplete-js/autocomplete/#param-render'
);
instance.destroy();

expect(startAutocomplete).not.toWarnDev(
'[Autocomplete] You provided an incomplete `renderer` (missing: `renderer.Fragment`, `renderer.render`). This can cause rendering issues.' +
'\nSee https://www.algolia.com/doc/ui-libraries/autocomplete/api-reference/autocomplete-js/autocomplete/#param-renderer'
);
instance.destroy();
});

test('does not warn at all when only passing a `render` option', () => {
Expand All @@ -447,8 +464,9 @@ describe('renderer', () => {

document.body.appendChild(panelContainer);

let instance;
expect(() => {
autocomplete<{ label: string }>({
instance = autocomplete<{ label: string }>({
container,
panelContainer,
initialState: {
Expand All @@ -474,6 +492,7 @@ describe('renderer', () => {
},
});
}).not.toWarnDev();
instance.destroy();
});

test('does not warn at all when passing an empty `renderer.render` function', () => {
Expand All @@ -484,8 +503,9 @@ describe('renderer', () => {

document.body.appendChild(panelContainer);

let instance;
expect(() => {
autocomplete<{ label: string }>({
instance = autocomplete<{ label: string }>({
container,
panelContainer,
initialState: {
Expand Down Expand Up @@ -513,6 +533,7 @@ describe('renderer', () => {
},
});
}).not.toWarnDev();
instance.destroy();
});

test('does not warn at all when not passing a custom renderer', () => {
Expand All @@ -521,8 +542,9 @@ describe('renderer', () => {

document.body.appendChild(panelContainer);

let instance;
expect(() => {
autocomplete<{ label: string }>({
instance = autocomplete<{ label: string }>({
container,
panelContainer,
initialState: {
Expand All @@ -545,6 +567,7 @@ describe('renderer', () => {
},
});
}).not.toWarnDev();
instance.destroy();
});

test('does not warn at all when passing a full custom renderer', () => {
Expand All @@ -556,8 +579,9 @@ describe('renderer', () => {

document.body.appendChild(panelContainer);

let instance;
expect(() => {
autocomplete<{ label: string }>({
instance = autocomplete<{ label: string }>({
container,
panelContainer,
initialState: {
Expand Down Expand Up @@ -585,5 +609,6 @@ describe('renderer', () => {
},
});
}).not.toWarnDev();
instance.destroy();
});
});
13 changes: 13 additions & 0 deletions packages/autocomplete-js/src/autocomplete.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
createRef,
debounce,
getItemsCount,
warn,
} from '@algolia/autocomplete-shared';
import htm from 'htm';

Expand All @@ -27,6 +28,8 @@ import {
import { userAgents } from './userAgents';
import { mergeDeep, pickBy, setProperties } from './utils';

let instancesCount = 0;

export function autocomplete<TItem extends BaseItem>(
options: AutocompleteOptions<TItem>
): AutocompleteApi<TItem> {
Expand Down Expand Up @@ -336,6 +339,7 @@ export function autocomplete<TItem extends BaseItem>(
});

function destroy() {
instancesCount--;
cleanupEffects();
}

Expand Down Expand Up @@ -397,6 +401,15 @@ export function autocomplete<TItem extends BaseItem>(
});
}

warn(
instancesCount === 0,
`Autocomplete doesn't support multiple instances running at the same time. Make sure to destroy the previous instance before creating a new one.

See: https://www.algolia.com/doc/ui-libraries/autocomplete/api-reference/autocomplete-js/autocomplete/#param-destroy`
);

instancesCount++;

return {
...autocompleteScopeApi,
update,
Expand Down