Skip to content

Commit a7782a6

Browse files
committed
Fix updating Element options
Before this fix, a call to update() like this: update({panelContainer: document.createElement('div')}) would throw the following error: TypeError: panelContainer.contains is not a function The reason lies in an incorrect object deep merge implementation where object entry values of Element type are merged like plain objects instead of being replaced, resulting in broken non-Element entries. This change fixes the issue with HTML Elements (Nodes of any kind to be more precise) but it might be worth considering replacing the current mergeDeep implementation with something more robust and future proof. Deep merging of objects surprisingly hard to get right, see the implementation in Lodash for example: https://github.com/lodash/lodash/blob/4.17.15/lodash.js#L3633
1 parent d6d6ba9 commit a7782a6

3 files changed

Lines changed: 121 additions & 1 deletion

File tree

packages/autocomplete-js/src/__tests__/api.test.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,79 @@ describe('api', () => {
259259
);
260260
});
261261

262+
test('correctly merges DOM Element options', async () => {
263+
let inputElement: HTMLInputElement;
264+
const onStateChange = jest.fn();
265+
const container = document.createElement('div');
266+
document.body.appendChild(container);
267+
const panelContainer = document.createElement('div');
268+
document.body.appendChild(panelContainer);
269+
const panelContainer2 = document.createElement('div');
270+
document.body.appendChild(panelContainer2);
271+
272+
const { update } = autocomplete<{ label: string }>({
273+
container,
274+
onStateChange,
275+
panelContainer,
276+
shouldPanelOpen: () => true,
277+
openOnFocus: true,
278+
getSources() {
279+
return [
280+
{
281+
sourceId: 'testSource',
282+
getItems() {
283+
return [];
284+
},
285+
templates: {
286+
item({ item }) {
287+
return item.label;
288+
},
289+
noResults() {
290+
return 'No results template';
291+
},
292+
},
293+
},
294+
];
295+
},
296+
});
297+
298+
// Focusing the input should render the panel
299+
inputElement = container.querySelector('.aa-Input');
300+
inputElement.focus();
301+
302+
// Focusing the input should open the panel
303+
expect(onStateChange).toHaveBeenLastCalledWith(
304+
expect.objectContaining({
305+
state: expect.objectContaining({
306+
isOpen: true,
307+
}),
308+
})
309+
);
310+
311+
// Panel is rendered into the original container
312+
await waitFor(() => {
313+
expect(
314+
panelContainer.querySelector<HTMLElement>('.aa-Panel')
315+
).toHaveTextContent('No results template');
316+
});
317+
318+
// Update options (set `panelContainer` to a different element)
319+
update({
320+
panelContainer: panelContainer2,
321+
});
322+
323+
// Focusing the input should render the panel
324+
inputElement = container.querySelector('.aa-Input');
325+
inputElement.focus();
326+
327+
// Panel is rendered into the new container
328+
await waitFor(() => {
329+
expect(
330+
panelContainer2.querySelector<HTMLElement>('.aa-Panel')
331+
).toHaveTextContent('No results template');
332+
});
333+
});
334+
262335
test('overrides the default id', async () => {
263336
const container = document.createElement('div');
264337

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import { mergeDeep } from '../mergeDeep';
2+
3+
describe('mergeDeep', () => {
4+
test('arrays', () => {
5+
expect(mergeDeep({ key: ['test1'] }, { key: ['test2'] })).toEqual({
6+
key: ['test1', 'test2'],
7+
});
8+
});
9+
10+
test('plain objects', () => {
11+
expect(
12+
mergeDeep({ key: { test1: 'value1' } }, { key: { test2: 'value2' } })
13+
).toEqual({
14+
key: { test1: 'value1', test2: 'value2' },
15+
});
16+
});
17+
18+
test('HTML Elements', () => {
19+
expect(
20+
mergeDeep(
21+
{ key: document.createElement('div') },
22+
{ key: document.createElement('span') }
23+
)
24+
).toEqual({
25+
key: document.createElement('span'),
26+
});
27+
});
28+
29+
test('primitives', () => {
30+
expect(mergeDeep({ key: 1 }, { key: 2 })).toEqual({
31+
key: 2,
32+
});
33+
});
34+
35+
test('null', () => {
36+
expect(mergeDeep({ key: 1 }, { key: null })).toEqual({
37+
key: null,
38+
});
39+
});
40+
41+
test('undefined', () => {
42+
expect(mergeDeep({ key: 1 }, { key: undefined })).toEqual({
43+
key: undefined,
44+
});
45+
});
46+
});

packages/autocomplete-js/src/utils/mergeDeep.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
const isObject = (value: unknown) => value && typeof value === 'object';
1+
const isObject = (value: unknown) =>
2+
value && typeof value === 'object' && !(value instanceof Node);
23

34
export function mergeDeep(...values: any[]) {
45
return values.reduce((acc, current) => {

0 commit comments

Comments
 (0)