Skip to content

Commit

Permalink
fix(js): prevent id incrementation when toggling detached mode (#489)
Browse files Browse the repository at this point in the history
* prevent id incrementation when toggling detached mode
  • Loading branch information
shortcuts committed Mar 5, 2021
1 parent f83d3ec commit fe2bf13
Show file tree
Hide file tree
Showing 10 changed files with 124 additions and 7 deletions.
9 changes: 6 additions & 3 deletions packages/autocomplete-core/src/getDefaultProps.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { getItemsCount } from '@algolia/autocomplete-shared';
import {
getItemsCount,
generateAutocompleteId,
} from '@algolia/autocomplete-shared';

import {
AutocompleteEnvironment,
Expand All @@ -7,7 +10,7 @@ import {
BaseItem,
InternalAutocompleteOptions,
} from './types';
import { generateAutocompleteId, getNormalizedSources, flatten } from './utils';
import { getNormalizedSources, flatten } from './utils';

export function getDefaultProps<TItem extends BaseItem>(
props: AutocompleteOptions<TItem>,
Expand All @@ -29,7 +32,7 @@ export function getDefaultProps<TItem extends BaseItem>(
shouldPanelOpen: ({ state }) => getItemsCount(state) > 0,
...props,
// Since `generateAutocompleteId` triggers a side effect (it increments
// and internal counter), we don't want to execute it if unnecessary.
// an internal counter), we don't want to execute it if unnecessary.
id: props.id ?? generateAutocompleteId(),
plugins,
// The following props need to be deeply defaulted.
Expand Down
1 change: 0 additions & 1 deletion packages/autocomplete-core/src/utils/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
export * from './createConcurrentSafePromise';
export * from './flatten';
export * from './generateAutocompleteId';
export * from './getNextActiveItemId';
export * from './getNormalizedSources';
export * from './getActiveItem';
Expand Down
18 changes: 18 additions & 0 deletions packages/autocomplete-js/src/__tests__/api.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { createAutocomplete } from '@algolia/autocomplete-core';
import { waitFor } from '@testing-library/dom';

import { createCollection } from '../../../../test/utils';
import { autocomplete } from '../autocomplete';
Expand Down Expand Up @@ -253,6 +254,23 @@ describe('api', () => {
})
);
});

test('overrides the default id', async () => {
const container = document.createElement('div');

document.body.appendChild(container);
const { update } = autocomplete<{ label: string }>({
container,
});

update({ id: 'bestSearchExperience' });

await waitFor(() => {
expect(
document.body.querySelector('#bestSearchExperience-label')
).toBeInTheDocument();
});
});
});

describe('destroy', () => {
Expand Down
87 changes: 87 additions & 0 deletions packages/autocomplete-js/src/__tests__/autocomplete.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,18 @@
import * as autocompleteShared from '@algolia/autocomplete-shared';
import { fireEvent, waitFor } from '@testing-library/dom';

import { castToJestMock } from '../../../../test/utils';
import { autocomplete } from '../autocomplete';

jest.mock('@algolia/autocomplete-shared', () => {
const module = jest.requireActual('@algolia/autocomplete-shared');

return {
...module,
generateAutocompleteId: jest.fn(() => `autocomplete-test`),
};
});

describe('autocomplete-js', () => {
test('renders with default options', () => {
const container = document.createElement('div');
Expand Down Expand Up @@ -151,6 +162,82 @@ describe('autocomplete-js', () => {
`);
});

test("renders with an auto-incremented id if there's multiple instances", () => {
const container = document.createElement('div');
const initialNbCalls = castToJestMock(
autocompleteShared.generateAutocompleteId
).mock.calls.length;

document.body.appendChild(container);
autocomplete({
container,
});

expect(autocompleteShared.generateAutocompleteId).toHaveBeenCalledTimes(
initialNbCalls + 1
);

autocomplete({
container,
});

expect(autocompleteShared.generateAutocompleteId).toHaveBeenCalledTimes(
initialNbCalls + 2
);

autocomplete({
container,
});

expect(autocompleteShared.generateAutocompleteId).toHaveBeenCalledTimes(
initialNbCalls + 3
);
});

test("doesn't increment the id when toggling detached mode", () => {
const container = document.createElement('div');

document.body.appendChild(container);
const { update } = autocomplete<{ label: string }>({
container,
});

const initialNbCalls = castToJestMock(
autocompleteShared.generateAutocompleteId
).mock.calls.length;

expect(autocompleteShared.generateAutocompleteId).toHaveBeenCalledTimes(
initialNbCalls
);

const originalMatchMedia = window.matchMedia;

Object.defineProperty(window, 'matchMedia', {
writable: true,
value: jest.fn((query) => ({
matches: true,
media: query,
onchange: null,
addListener: jest.fn(),
removeListener: jest.fn(),
addEventListener: jest.fn(),
removeEventListener: jest.fn(),
dispatchEvent: jest.fn(),
})),
});

update({ detachedMediaQuery: '' });

Object.defineProperty(window, 'matchMedia', {
writable: true,
value: originalMatchMedia,
});

expect(autocompleteShared.generateAutocompleteId).toHaveBeenCalledTimes(
initialNbCalls
);
});

test('renders noResults template on no results', async () => {
const container = document.createElement('div');
const panelContainer = document.createElement('div');
Expand Down
5 changes: 3 additions & 2 deletions packages/autocomplete-js/src/autocomplete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export function autocomplete<TItem extends BaseItem>(
props.value.renderer.detachedMediaQuery
).matches
);

const autocomplete = reactive(() =>
createAutocomplete<TItem>({
...props.value.core,
Expand Down Expand Up @@ -246,12 +247,12 @@ export function autocomplete<TItem extends BaseItem>(

runEffect(() => {
const onResize = debounce<Event>(() => {
const previousisDetached = isDetached.value;
const previousIsDetached = isDetached.value;
isDetached.value = props.value.core.environment.matchMedia(
props.value.renderer.detachedMediaQuery
).matches;

if (previousisDetached !== isDetached.value) {
if (previousIsDetached !== isDetached.value) {
update({});
} else {
requestAnimationFrame(setPanelPosition);
Expand Down
6 changes: 5 additions & 1 deletion packages/autocomplete-js/src/getDefaultOptions.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { BaseItem } from '@algolia/autocomplete-core';
import { invariant } from '@algolia/autocomplete-shared';
import {
generateAutocompleteId,
invariant,
} from '@algolia/autocomplete-shared';
import {
createElement as preactCreateElement,
Fragment as PreactFragment,
Expand Down Expand Up @@ -115,6 +118,7 @@ export function getDefaultOptions<TItem extends BaseItem>(
},
core: {
...core,
id: core.id ?? generateAutocompleteId(),
environment,
},
};
Expand Down
1 change: 1 addition & 0 deletions packages/autocomplete-shared/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export * from './createRef';
export * from './debounce';
export * from './generateAutocompleteId';
export * from './getAttributeValueByPath';
export * from './getItemsCount';
export * from './invariant';
Expand Down
3 changes: 3 additions & 0 deletions test/utils/castToJestMock.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const castToJestMock = <TFunction extends (...args: any[]) => any>(
func: TFunction
) => func as jest.MockedFunction<typeof func>;
1 change: 1 addition & 0 deletions test/utils/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export * from './castToJestMock';
export * from './createApiResponse';
export * from './createCollection';
export * from './createNavigator';
Expand Down

0 comments on commit fe2bf13

Please sign in to comment.