Skip to content

Commit 515f476

Browse files
VasilyStrelyaevVasilyStrelyaev
authored
fix: Multiple root elements are not supported in React elements (T1300588) (DevExpress#30790)
1 parent ba105c5 commit 515f476

File tree

3 files changed

+237
-39
lines changed

3 files changed

+237
-39
lines changed

‎packages/devextreme-react/src/core/__tests__/integration/integration.test.tsx‎

Lines changed: 132 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,13 @@ import {
1313
import Validator from '../../../validator';
1414
import ValidationSummary from '../../../validation-summary';
1515
import DataGrid from '../../../data-grid';
16+
import Scheduler, { View, Resource } from '../../../scheduler';
1617
import { ContextMenu, Item as ContextMenuItem } from '../../../context-menu';
1718
import Button from '../../../button';
1819

1920
jest.useFakeTimers();
2021

2122
describe('integration tests', () => {
22-
let consoleWarnSpy;
23-
2423
afterEach(() => {
2524
jest.clearAllMocks();
2625
testingLib.cleanup();
@@ -155,7 +154,7 @@ describe('integration tests', () => {
155154

156155
it('ref callback is not triggered when not needed', async () => {
157156
const user = userEvent.setup({ delay: null });
158-
consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
157+
const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
159158

160159
const DataGridWithSelectButton = () => {
161160
const [selectedRowKeys, setSelectedRowKeys] = React.useState([] as any);
@@ -226,4 +225,134 @@ describe('integration tests', () => {
226225
expect(consoleWarnSpy).toHaveBeenCalledTimes(1);
227226
consoleWarnSpy.mockRestore();
228227
});
228+
229+
it('must not fail if a template with two root elements is unmounted (T1300588)', async () => {
230+
try {
231+
jest.useRealTimers();
232+
expect.assertions(1);
233+
234+
const user = userEvent.setup({ delay: null });
235+
236+
const currentDate = new Date(2021, 3, 27);
237+
const dayOfWeekNames = ['Sun', 'Mon', 'Tue', 'Wed', 'Thu', 'Fri', 'Sat'];
238+
const typeGroups = ['typeId'];
239+
const priorityGroups = ['priorityId'];
240+
241+
const data = [
242+
{
243+
text: 'Walking a dog',
244+
priorityId: 1,
245+
typeId: 1,
246+
startDate: new Date('2021-04-26T15:00:00.000Z'),
247+
endDate: new Date('2021-04-26T15:30:00.000Z'),
248+
recurrenceRule: 'FREQ=DAILY;BYDAY=MO,TU,WE,TH,FR;UNTIL=20210502',
249+
},
250+
{
251+
text: 'Website Re-Design Plan',
252+
priorityId: 2,
253+
typeId: 2,
254+
startDate: new Date('2021-04-26T16:00:00.000Z'),
255+
endDate: new Date('2021-04-26T18:30:00.000Z'),
256+
},
257+
{
258+
text: 'Book Flights to San Fran for Sales Trip',
259+
priorityId: 2,
260+
typeId: 2,
261+
startDate: new Date('2021-04-26T19:00:00.000Z'),
262+
endDate: new Date('2021-04-26T20:00:00.000Z'),
263+
}
264+
];
265+
266+
const priorityData = [{
267+
text: 'Low Priority',
268+
id: 1,
269+
color: '#fcb65e',
270+
}, {
271+
text: 'High Priority',
272+
id: 2,
273+
color: '#e18e92',
274+
}];
275+
276+
const typeData = [{
277+
text: 'Home',
278+
id: 1,
279+
color: '#b6d623',
280+
}, {
281+
text: 'Work',
282+
id: 2,
283+
color: '#679ec5',
284+
}];
285+
286+
const DateCell = ({ data: cellData }) => (
287+
<React.Fragment>
288+
<div className="name">{dayOfWeekNames[cellData.date.getDay()]}</div>
289+
<div className="number">{cellData.date.getDate()}</div>
290+
</React.Fragment>
291+
);
292+
293+
let clicked = false;
294+
let resolve = () => {};
295+
296+
const onContentReady = async () => {
297+
if (clicked)
298+
return;
299+
300+
clicked = true;
301+
await expect(user.click(document.querySelector('.dx-icon-chevronnext')!)).resolves.toBe(void 0);
302+
resolve();
303+
}
304+
305+
const SchedulerWithTemplates = () => (
306+
<Scheduler
307+
timeZone="America/Los_Angeles"
308+
dataSource={data}
309+
defaultCurrentView="workWeek"
310+
showAllDayPanel={false}
311+
defaultCurrentDate={currentDate}
312+
height={730}
313+
startDayHour={7}
314+
endDayHour={23}
315+
onContentReady={onContentReady}
316+
>
317+
<View type="day" />
318+
<View type="week" groups={typeGroups} dateCellComponent={DateCell} />
319+
<View
320+
type="workWeek"
321+
groups={priorityGroups}
322+
startDayHour={9}
323+
endDayHour={18}
324+
dateCellComponent={DateCell}
325+
/>
326+
<View type="month" />
327+
<Resource
328+
dataSource={priorityData}
329+
fieldExpr="priorityId"
330+
label="Priority"
331+
allowMultiple={false}
332+
/>
333+
<Resource
334+
dataSource={typeData}
335+
fieldExpr="typeId"
336+
label="Type"
337+
allowMultiple={false}
338+
/>
339+
</Scheduler>
340+
);
341+
342+
testingLib.render(
343+
<React.Fragment>
344+
<SchedulerWithTemplates />
345+
</React.Fragment>
346+
);
347+
348+
await testingLib.act(async () => {
349+
return new Promise((r) => {
350+
resolve = r;
351+
});
352+
});
353+
}
354+
finally {
355+
jest.useFakeTimers();
356+
}
357+
});
229358
});

‎packages/devextreme-react/src/core/__tests__/template-wrapper.test.tsx‎

Lines changed: 67 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,42 @@
11
import * as React from 'react';
2+
import { default as dxRender } from 'devextreme/core/renderer';
23
import { useEffect, useContext } from 'react';
34
import { TemplateWrapper } from '../template-wrapper';
45
import { cleanup, render } from '@testing-library/react';
56
import * as events from 'devextreme/events';
67
import { RemovalLockerContext, UpdateLocker } from '../contexts';
78
import { TemplateFunc } from '../types';
89

9-
function TemplateComponent(args: { data, index, onRendered?, effect? }) {
10-
const { data, index, onRendered, effect } = args;
10+
function TemplateComponent(args: { data, index, onRendered?, effect?, multipleRoots }) {
11+
const { data, index, onRendered, effect, multipleRoots } = args;
1112

1213
effect?.();
1314

1415
useEffect(() => {
1516
onRendered?.();
1617
}, [onRendered]);
1718

18-
return (
19-
<div className='template-element'>{`${data.text} - ${index}`}</div>
20-
);
19+
return multipleRoots
20+
? (
21+
<>
22+
<div className='template-element-1'>{`${data.text} - ${index}`}</div>
23+
<div className='template-element-2'>{`${data.text} - ${index}`}</div>
24+
</>
25+
)
26+
: (
27+
<div className='template-element'>{`${data.text} - ${index}`}</div>
28+
);
2129
}
2230

23-
function getComponentTemplateFunction(effect?) {
31+
function getComponentTemplateFunction(effect?, multipleRoots = false) {
2432
return ({ data, index, onRendered }) => {
2533
return (
2634
<TemplateComponent
2735
data={data}
2836
index={index}
2937
onRendered={onRendered}
3038
effect={effect}
39+
multipleRoots={multipleRoots}
3140
/>
3241
);
3342
};
@@ -43,7 +52,7 @@ describe('Template Wrapper', () => {
4352
cleanup();
4453
});
4554

46-
it('works with locker in the context correctly', () => {
55+
it('works with locker in the context correctly', async () => {
4756
let onRemovedFired = false;
4857
let removalLocker: UpdateLocker | undefined = undefined;
4958

@@ -86,6 +95,7 @@ describe('Template Wrapper', () => {
8695

8796
events.triggerHandler(document.querySelector('.template-element')!, 'dxremove');
8897

98+
await Promise.resolve();
8999
expect(onRemovedFired).toBeTruthy();
90100

91101
onRemovedFired = false;
@@ -107,6 +117,7 @@ describe('Template Wrapper', () => {
107117
removalLocker.lock();
108118
events.triggerHandler(document.querySelector('.template-element')!, 'dxremove');
109119

120+
await Promise.resolve();
110121
expect(onRemovedFired).toBeFalsy();
111122

112123
rerender(
@@ -127,6 +138,7 @@ describe('Template Wrapper', () => {
127138
removalLocker.lock();
128139
events.triggerHandler(document.querySelector('.template-element')!, 'dxremove');
129140

141+
await Promise.resolve();
130142
expect(onRemovedFired).toBeFalsy();
131143

132144
rerender(
@@ -147,10 +159,11 @@ describe('Template Wrapper', () => {
147159
removalLocker.unlock();
148160
events.triggerHandler(document.querySelector('.template-element')!, 'dxremove');
149161

162+
await Promise.resolve();
150163
expect(onRemovedFired).toBeTruthy();
151164
});
152165

153-
it('does not fire onRemove when the event comes from wrappers', () => {
166+
it('does not fire onRemove when the event comes from wrappers', async () => {
154167
let onRemovedFired = false;
155168

156169
const onRemoved = () => {
@@ -182,6 +195,7 @@ describe('Template Wrapper', () => {
182195

183196
events.triggerHandler(document.querySelector('.template-element')!, 'dxremove', { isUnmounting: true });
184197

198+
await Promise.resolve();
185199
expect(onRemovedFired).toBeFalsy();
186200

187201
rerender(
@@ -201,6 +215,7 @@ describe('Template Wrapper', () => {
201215

202216
events.triggerHandler(document.querySelector('.template-element')!, 'dxremove');
203217

218+
await Promise.resolve();
204219
expect(onRemovedFired).toBeTruthy();
205220
});
206221

@@ -355,7 +370,7 @@ describe('Template Wrapper', () => {
355370
.toBe('<div class="template-container">My template - 1<div style="display: none;"></div><span style="display: none;"></span></div>');
356371
});
357372

358-
it('triggers onRemove when the element is removed', () => {
373+
it('triggers onRemove when the element is removed', async () => {
359374
let onRemovedFired = false;
360375

361376
const onRemoved = () => {
@@ -383,15 +398,17 @@ describe('Template Wrapper', () => {
383398
/>
384399
</>
385400
);
386-
401+
402+
await Promise.resolve();
387403
expect(onRemovedFired).toBeFalsy();
388404

389405
events.triggerHandler(document.querySelector('.template-element')!, 'dxremove');
390406

407+
await Promise.resolve();
391408
expect(onRemovedFired).toBeTruthy();
392409
});
393410

394-
it('removes text templates when the removal listener is removed', () => {
411+
it('removes text templates when the removal listener is removed', async () => {
395412
let onRemovedFired = false;
396413

397414
const onRemoved = () => {
@@ -420,6 +437,7 @@ describe('Template Wrapper', () => {
420437
</>
421438
);
422439

440+
await Promise.resolve();
423441
expect(onRemovedFired).toBeFalsy();
424442

425443
const containerChildren = document.querySelector('.template-container')?.children!;
@@ -428,10 +446,11 @@ describe('Template Wrapper', () => {
428446
events.triggerHandler(containerChildren[i], 'dxremove');
429447
}
430448

449+
await Promise.resolve();
431450
expect(onRemovedFired).toBeTruthy();
432451
});
433452

434-
it('returns all the elements to DOM on unmount to avoid upsetting React', () => {
453+
it('returns the element and hidden nodes to DOM on unmount to avoid upsetting React', () => {
435454
const templateFunction: TemplateFunc = getComponentTemplateFunction();
436455

437456
const { rerender, unmount } = render(
@@ -458,10 +477,45 @@ describe('Template Wrapper', () => {
458477
const children = container.children;
459478

460479
for(var i = 0; i < children.length; i++) {
461-
container.removeChild(children[i]);
480+
dxRender(children[i]).remove();
481+
}
482+
483+
expect(() => unmount()).not.toThrow();
484+
expect(container.children.length).toBe(0);
485+
});
486+
487+
it('returns multiple template root elements to DOM on unmount to avoid upsetting React', () => {
488+
const templateFunction: TemplateFunc = getComponentTemplateFunction(void 0, true);
489+
490+
const { rerender, unmount } = render(
491+
<>
492+
<div className='template-container' />
493+
</>
494+
);
495+
496+
rerender(
497+
<>
498+
<div className='template-container' />
499+
<TemplateWrapper
500+
templateFactory={templateFunction}
501+
data={{ text: 'My template' }}
502+
index={1}
503+
onRendered={() => undefined}
504+
container={document.querySelector('.template-container')!}
505+
onRemoved={() => undefined}
506+
/>
507+
</>
508+
);
509+
510+
const container = document.querySelector('.template-container')!;
511+
const children = container.children;
512+
513+
for(var i = 0; i < children.length; i++) {
514+
dxRender(children[i]).remove();
462515
}
463516

464517
expect(() => unmount()).not.toThrow();
518+
expect(container.children.length).toBe(0);
465519
});
466520

467521
it('portals its component to the specified container', () => {

0 commit comments

Comments
 (0)