From bf8894ff13f64a3cec2fd48e811aac2514687a0e Mon Sep 17 00:00:00 2001 From: Dhaya <154633+dhayab@users.noreply.github.com> Date: Mon, 13 Mar 2023 16:17:59 +0100 Subject: [PATCH 1/4] fix(autocomplete-js): display warning when there are more than one instance of autocomplete --- .../src/__tests__/autocomplete.test.ts | 18 ++++++++++++++++++ packages/autocomplete-js/src/autocomplete.ts | 11 +++++++++++ 2 files changed, 29 insertions(+) diff --git a/packages/autocomplete-js/src/__tests__/autocomplete.test.ts b/packages/autocomplete-js/src/__tests__/autocomplete.test.ts index 39269e595..5be846793 100644 --- a/packages/autocomplete-js/src/__tests__/autocomplete.test.ts +++ b/packages/autocomplete-js/src/__tests__/autocomplete.test.ts @@ -23,6 +23,24 @@ 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] Multiple instances of Autocomplete are not currently supported and can introduce unwanted behavior during user interaction. Please destroy the previous instance before creating a new one.' + ); + }); + test('renders with default options', () => { const container = document.createElement('div'); autocomplete<{ label: string }>({ diff --git a/packages/autocomplete-js/src/autocomplete.ts b/packages/autocomplete-js/src/autocomplete.ts index ce0ef2e9e..b22be863a 100644 --- a/packages/autocomplete-js/src/autocomplete.ts +++ b/packages/autocomplete-js/src/autocomplete.ts @@ -7,6 +7,7 @@ import { createRef, debounce, getItemsCount, + warn, } from '@algolia/autocomplete-shared'; import htm from 'htm'; @@ -27,6 +28,8 @@ import { import { userAgents } from './userAgents'; import { mergeDeep, pickBy, setProperties } from './utils'; +let instancesCount = 0; + export function autocomplete( options: AutocompleteOptions ): AutocompleteApi { @@ -336,6 +339,7 @@ export function autocomplete( }); function destroy() { + instancesCount--; cleanupEffects(); } @@ -397,6 +401,13 @@ export function autocomplete( }); } + warn( + instancesCount === 0, + 'Multiple instances of Autocomplete are not currently supported and can introduce unwanted behavior during user interaction. Please destroy the previous instance before creating a new one.' + ); + + instancesCount++; + return { ...autocompleteScopeApi, update, From c7807e6cfbe6451a6f1dfe70e296f0cd4f965ca4 Mon Sep 17 00:00:00 2001 From: Dhaya <154633+dhayab@users.noreply.github.com> Date: Mon, 13 Mar 2023 16:44:06 +0100 Subject: [PATCH 2/4] fix tests --- .../src/__tests__/renderer.test.ts | 51 ++++++++++++++----- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/packages/autocomplete-js/src/__tests__/renderer.test.ts b/packages/autocomplete-js/src/__tests__/renderer.test.ts index eb3f9b95e..cca9ed416 100644 --- a/packages/autocomplete-js/src/__tests__/renderer.test.ts +++ b/packages/autocomplete-js/src/__tests__/renderer.test.ts @@ -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: { @@ -53,6 +53,8 @@ describe('renderer', () => { render(createElement(Fragment, null, 'testSource'), root); }, }); + + destroy(); }); test('accepts a custom renderer', () => { @@ -66,7 +68,7 @@ describe('renderer', () => { document.body.appendChild(panelContainer); - autocomplete<{ label: string }>({ + const { destroy } = autocomplete<{ label: string }>({ container, panelContainer, initialState: { @@ -110,6 +112,8 @@ describe('renderer', () => { render: mockRender, }, }); + + destroy(); }); test('defaults `render` when not specified in the renderer', () => { @@ -122,7 +126,7 @@ describe('renderer', () => { document.body.appendChild(panelContainer); - autocomplete<{ label: string }>({ + const { destroy } = autocomplete<{ label: string }>({ container, panelContainer, initialState: { @@ -153,6 +157,8 @@ describe('renderer', () => { Fragment: CustomFragment, }, }); + + destroy(); }); test('uses a custom `render` via `renderer`', async () => { @@ -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', @@ -238,6 +244,8 @@ describe('renderer', () => { `); }); + + destroy(); }); test('warns about renderer mismatch when specifying an incomplete renderer', () => { @@ -249,8 +257,10 @@ describe('renderer', () => { document.body.appendChild(panelContainer); + let instance; + expect(() => { - autocomplete<{ label: string }>({ + instance = autocomplete<{ label: string }>({ container, panelContainer, initialState: { @@ -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: { @@ -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: { @@ -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: { @@ -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', () => { @@ -394,8 +408,9 @@ describe('renderer', () => { document.body.appendChild(panelContainer); + let instance; function startAutocomplete() { - autocomplete<{ label: string }>({ + instance = autocomplete<{ label: string }>({ container, panelContainer, initialState: { @@ -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', () => { @@ -447,8 +464,9 @@ describe('renderer', () => { document.body.appendChild(panelContainer); + let instance; expect(() => { - autocomplete<{ label: string }>({ + instance = autocomplete<{ label: string }>({ container, panelContainer, initialState: { @@ -474,6 +492,7 @@ describe('renderer', () => { }, }); }).not.toWarnDev(); + instance.destroy(); }); test('does not warn at all when passing an empty `renderer.render` function', () => { @@ -484,8 +503,9 @@ describe('renderer', () => { document.body.appendChild(panelContainer); + let instance; expect(() => { - autocomplete<{ label: string }>({ + instance = autocomplete<{ label: string }>({ container, panelContainer, initialState: { @@ -513,6 +533,7 @@ describe('renderer', () => { }, }); }).not.toWarnDev(); + instance.destroy(); }); test('does not warn at all when not passing a custom renderer', () => { @@ -521,8 +542,9 @@ describe('renderer', () => { document.body.appendChild(panelContainer); + let instance; expect(() => { - autocomplete<{ label: string }>({ + instance = autocomplete<{ label: string }>({ container, panelContainer, initialState: { @@ -545,6 +567,7 @@ describe('renderer', () => { }, }); }).not.toWarnDev(); + instance.destroy(); }); test('does not warn at all when passing a full custom renderer', () => { @@ -556,8 +579,9 @@ describe('renderer', () => { document.body.appendChild(panelContainer); + let instance; expect(() => { - autocomplete<{ label: string }>({ + instance = autocomplete<{ label: string }>({ container, panelContainer, initialState: { @@ -585,5 +609,6 @@ describe('renderer', () => { }, }); }).not.toWarnDev(); + instance.destroy(); }); }); From e0f95914569d628103239d0f498b8af247797705 Mon Sep 17 00:00:00 2001 From: Dhaya <154633+dhayab@users.noreply.github.com> Date: Mon, 13 Mar 2023 16:44:29 +0100 Subject: [PATCH 3/4] apply suggestions Co-authored-by: Sarah Dayan <5370675+sarahdayan@users.noreply.github.com> --- packages/autocomplete-js/src/__tests__/autocomplete.test.ts | 4 +++- packages/autocomplete-js/src/autocomplete.ts | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/autocomplete-js/src/__tests__/autocomplete.test.ts b/packages/autocomplete-js/src/__tests__/autocomplete.test.ts index 5be846793..77356c486 100644 --- a/packages/autocomplete-js/src/__tests__/autocomplete.test.ts +++ b/packages/autocomplete-js/src/__tests__/autocomplete.test.ts @@ -37,7 +37,9 @@ describe('autocomplete-js', () => { container: secondContainer, }) ).toWarnDev( - '[Autocomplete] Multiple instances of Autocomplete are not currently supported and can introduce unwanted behavior during user interaction. Please destroy the previous instance before creating a new one.' + `[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/#returns` ); }); diff --git a/packages/autocomplete-js/src/autocomplete.ts b/packages/autocomplete-js/src/autocomplete.ts index b22be863a..a618cb2d3 100644 --- a/packages/autocomplete-js/src/autocomplete.ts +++ b/packages/autocomplete-js/src/autocomplete.ts @@ -403,7 +403,9 @@ export function autocomplete( warn( instancesCount === 0, - 'Multiple instances of Autocomplete are not currently supported and can introduce unwanted behavior during user interaction. Please destroy the previous instance before creating a new one.' + `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/#returns` ); instancesCount++; From 2c6e81f983b316260159210a79e6c90f9b3fce18 Mon Sep 17 00:00:00 2001 From: Dhaya <154633+dhayab@users.noreply.github.com> Date: Mon, 13 Mar 2023 17:10:36 +0100 Subject: [PATCH 4/4] direct link to destroy param --- packages/autocomplete-js/src/__tests__/autocomplete.test.ts | 2 +- packages/autocomplete-js/src/autocomplete.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/autocomplete-js/src/__tests__/autocomplete.test.ts b/packages/autocomplete-js/src/__tests__/autocomplete.test.ts index 77356c486..ca04cbf75 100644 --- a/packages/autocomplete-js/src/__tests__/autocomplete.test.ts +++ b/packages/autocomplete-js/src/__tests__/autocomplete.test.ts @@ -39,7 +39,7 @@ describe('autocomplete-js', () => { ).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/#returns` +See: https://www.algolia.com/doc/ui-libraries/autocomplete/api-reference/autocomplete-js/autocomplete/#param-destroy` ); }); diff --git a/packages/autocomplete-js/src/autocomplete.ts b/packages/autocomplete-js/src/autocomplete.ts index a618cb2d3..e4ef89f31 100644 --- a/packages/autocomplete-js/src/autocomplete.ts +++ b/packages/autocomplete-js/src/autocomplete.ts @@ -405,7 +405,7 @@ export function autocomplete( 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/#returns` +See: https://www.algolia.com/doc/ui-libraries/autocomplete/api-reference/autocomplete-js/autocomplete/#param-destroy` ); instancesCount++;