Skip to content

Commit 7f65035

Browse files
authored
Scheduler(T1269168): fix appointment popup delay performance issue (DevExpress#29069)
1 parent 2645fb4 commit 7f65035

File tree

10 files changed

+498
-56
lines changed

10 files changed

+498
-56
lines changed

packages/devextreme/js/__internal/scheduler/appointment_popup/m_form.ts

Lines changed: 63 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,13 @@ import $ from '@js/core/renderer';
1111
import dateUtils from '@js/core/utils/date';
1212
import dateSerialization from '@js/core/utils/date_serialization';
1313
import { extend } from '@js/core/utils/extend';
14+
import type AbstractStore from '@js/data/abstract_store';
1415
import Form from '@js/ui/form';
1516
import { current, isFluent } from '@js/ui/themes';
1617
import { ExpressionUtils } from '@ts/scheduler/m_expression_utils';
1718

1819
import { createAppointmentAdapter } from '../m_appointment_adapter';
20+
import type { TimezoneLabel } from '../m_utils_time_zone';
1921
import timeZoneUtils from '../m_utils_time_zone';
2022

2123
const SCREEN_SIZE_OF_SINGLE_COLUMN = 600;
@@ -40,6 +42,11 @@ const E2E_TEST_CLASSES = {
4042
recurrenceSwitch: 'e2e-dx-scheduler-form-recurrence-switch',
4143
};
4244

45+
const DEFAULT_TIMEZONE_EDITOR_DATA_SOURCE_OPTIONS = {
46+
paginate: true,
47+
pageSize: 10,
48+
};
49+
4350
const getStylingModeFunc = (): string | undefined => (isFluent(current()) ? 'filled' : undefined);
4451

4552
const getStartDateWithStartHour = (startDate, startDayHour) => new Date(new Date(startDate).setHours(startDayHour));
@@ -141,40 +148,13 @@ export class AppointmentForm {
141148
onInitialized: (e) => {
142149
this.form = e.component;
143150
},
144-
customizeItem: (e) => {
145-
if (this.form && e.itemType === 'group') {
146-
const dataExprs = this.scheduler.getDataAccessors().expr;
147-
148-
const startDate = new Date(this.formData[dataExprs.startDateExpr]);
149-
const endDate = new Date(this.formData[dataExprs.endDateExpr]);
150-
151-
const startTimeZoneEditor = e.items.find((i) => i.dataField === dataExprs.startDateTimeZoneExpr);
152-
const endTimeZoneEditor = e.items.find((i) => i.dataField === dataExprs.endDateTimeZoneExpr);
153-
154-
if (startTimeZoneEditor) {
155-
startTimeZoneEditor.editorOptions.dataSource = this.createTimeZoneDataSource(startDate);
156-
}
157-
158-
if (endTimeZoneEditor) {
159-
endTimeZoneEditor.editorOptions.dataSource = this.createTimeZoneDataSource(endDate);
160-
}
161-
}
162-
},
163151
screenByWidth: (width) => (width < SCREEN_SIZE_OF_SINGLE_COLUMN || devices.current().deviceType !== 'desktop' ? 'xs' : 'lg'),
164152
elementAttr: {
165153
class: E2E_TEST_CLASSES.form,
166154
},
167155
});
168156
}
169157

170-
createTimeZoneDataSource(date) {
171-
return new DataSource({
172-
store: timeZoneUtils.getTimeZones(date),
173-
paginate: true,
174-
pageSize: 10,
175-
});
176-
}
177-
178158
_createAppointmentAdapter(rawAppointment) {
179159
return createAppointmentAdapter(
180160
rawAppointment,
@@ -449,12 +429,60 @@ export class AppointmentForm {
449429
editor && this.form.itemOption(editorPath, 'editorOptions', extend({}, editor.editorOptions, options));
450430
}
451431

452-
setTimeZoneEditorDataSource(date, name) {
453-
const dataSource = this.createTimeZoneDataSource(date);
454-
this.setEditorOptions(name, 'Main', { dataSource });
432+
private scheduleTimezoneEditorDataSourceUpdate(
433+
editorName: string,
434+
dataSource: { store: () => AbstractStore; reload: () => void },
435+
selectedTimezoneLabel: TimezoneLabel | null,
436+
date: Date,
437+
): void {
438+
timeZoneUtils.getTimeZoneLabelsAsyncBatch(date)
439+
.catch(() => [] as TimezoneLabel[])
440+
.then(async (timezones) => {
441+
const store = dataSource.store();
442+
443+
await store.remove(selectedTimezoneLabel?.id);
444+
445+
// NOTE: Unfortunately, our store not support bulk operations
446+
// So, we update it record-by-record
447+
const insertPromises = timezones.reduce<Promise<void>[]>((result, timezone) => {
448+
result.push(store.insert(timezone));
449+
return result;
450+
}, []);
451+
452+
// NOTE: We should wait for all insertions before reload
453+
await Promise.all(insertPromises);
454+
455+
dataSource.reload();
456+
// NOTE: We should re-assign dataSource to the editor
457+
// to repaint this editor after dataSource update
458+
this.setEditorOptions(editorName, 'Main', { dataSource });
459+
}).catch(() => {});
460+
}
461+
462+
private setupTimezoneEditorDataSource(
463+
editorName: string,
464+
selectedTimezoneId: string | null,
465+
date: Date,
466+
): void {
467+
const selectedTimezoneLabel = selectedTimezoneId
468+
? timeZoneUtils.getTimeZoneLabel(selectedTimezoneId, date)
469+
: null;
470+
471+
const dataSource = new DataSource({
472+
...DEFAULT_TIMEZONE_EDITOR_DATA_SOURCE_OPTIONS,
473+
store: selectedTimezoneLabel ? [selectedTimezoneLabel] : [],
474+
});
475+
476+
this.setEditorOptions(editorName, 'Main', { dataSource });
477+
this.scheduleTimezoneEditorDataSourceUpdate(
478+
editorName,
479+
dataSource,
480+
selectedTimezoneLabel,
481+
date,
482+
);
455483
}
456484

457-
updateFormData(formData) {
485+
updateFormData(formData: Record<string, any>): void {
458486
this.isFormUpdating = true;
459487
this.form.option('formData', formData);
460488

@@ -463,13 +491,15 @@ export class AppointmentForm {
463491

464492
const rawStartDate = ExpressionUtils.getField(dataAccessors, 'startDate', formData);
465493
const rawEndDate = ExpressionUtils.getField(dataAccessors, 'endDate', formData);
494+
const startDateTimezone = ExpressionUtils.getField(dataAccessors, 'startDateTimeZone', formData) ?? null;
495+
const endDateTimezone = ExpressionUtils.getField(dataAccessors, 'endDateTimeZone', formData) ?? null;
466496

467497
const allDay = ExpressionUtils.getField(dataAccessors, 'allDay', formData);
468498
const startDate = new Date(rawStartDate);
469499
const endDate = new Date(rawEndDate);
470500

471-
this.setTimeZoneEditorDataSource(startDate, expr.startDateTimeZoneExpr);
472-
this.setTimeZoneEditorDataSource(endDate, expr.endDateTimeZoneExpr);
501+
this.setupTimezoneEditorDataSource(expr.startDateTimeZoneExpr, startDateTimezone, startDate);
502+
this.setupTimezoneEditorDataSource(expr.endDateTimeZoneExpr, endDateTimezone, endDate);
473503

474504
this.updateRecurrenceEditorStartDate(startDate, expr.recurrenceRuleExpr);
475505

packages/devextreme/js/__internal/scheduler/m_scheduler.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import {
4343
getPreparedDataItems,
4444
isDateAndTimeView, isTimelineView, viewsUtils,
4545
} from '@ts/scheduler/r1/utils/index';
46+
import { macroTaskArray } from '@ts/scheduler/utils/index';
4647

4748
import { AppointmentForm } from './appointment_popup/m_form';
4849
import { ACTION_TO_APPOINTMENT, AppointmentPopup } from './appointment_popup/m_popup';
@@ -1281,6 +1282,9 @@ class Scheduler extends Widget<any> {
12811282
this._asyncTemplatesTimers.forEach(clearTimeout);
12821283
this._asyncTemplatesTimers = [];
12831284

1285+
// NOTE: Stop all scheduled macro tasks
1286+
macroTaskArray.dispose();
1287+
12841288
// @ts-expect-error
12851289
super._dispose();
12861290
}

packages/devextreme/js/__internal/scheduler/m_utils_time_zone.ts

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,29 @@
1+
// TODO(Refactoring): move this module to ./utils directory
12
import errors from '@js/core/errors';
23
import { dateUtilsTs } from '@ts/core/utils/date';
4+
import { macroTaskArray } from '@ts/scheduler/utils/index';
35

46
import dateUtils from '../../core/utils/date';
57
import DateAdapter from './m_date_adapter';
68
import timeZoneDataUtils from './timezones/m_utils_timezones_data';
79
import timeZoneList from './timezones/timezone_list';
810

11+
export interface TimezoneLabel {
12+
/** uniq timezone id, e.g: 'America/Los_Angeles' */
13+
id: string;
14+
/** timezone display string, e.g: '(GMT -08:00) America - Los Angeles' */
15+
title?: string;
16+
}
17+
18+
export interface TimezoneData extends TimezoneLabel {
19+
/** timezone offset in hours */
20+
offset?: number;
21+
}
22+
923
const toMs = dateUtils.dateToMilliseconds;
1024
const MINUTES_IN_HOUR = 60;
1125
const MS_IN_MINUTE = 60000;
26+
const GET_TIMEZONES_BATCH_SIZE = 20;
1227
const GMT = 'GMT';
1328
const offsetFormatRegexp = /^GMT(?:[+-]\d{2}:\d{2})?$/;
1429

@@ -37,12 +52,6 @@ const createDateFromUTCWithLocalOffset = (date) => {
3752
return result.source;
3853
};
3954

40-
const getTimeZones = (date = new Date()) => timeZoneList.value.map((tz) => ({
41-
offset: calculateTimezoneByValue(tz, date),
42-
title: getTimezoneTitle(tz, date),
43-
id: tz,
44-
}));
45-
4655
const createUTCDate = (date) => new Date(Date.UTC(
4756
date.getUTCFullYear(),
4857
date.getUTCMonth(),
@@ -326,6 +335,33 @@ const addOffsetsWithoutDST = (date: Date, ...offsets: number[]): Date => {
326335
: newDate;
327336
};
328337

338+
const getTimeZoneLabelsAsyncBatch = (
339+
date = new Date(),
340+
): Promise<TimezoneLabel[]> => macroTaskArray.map(
341+
timeZoneList.value,
342+
(timezoneId) => ({
343+
id: timezoneId,
344+
title: getTimezoneTitle(timezoneId, date),
345+
}),
346+
GET_TIMEZONES_BATCH_SIZE,
347+
);
348+
349+
const getTimeZoneLabel = (
350+
timezoneId: string,
351+
date = new Date(),
352+
): TimezoneLabel => ({
353+
id: timezoneId,
354+
title: getTimezoneTitle(timezoneId, date),
355+
});
356+
357+
const getTimeZones = (
358+
date = new Date(),
359+
): TimezoneData[] => timeZoneList.value.map((timezoneId) => ({
360+
id: timezoneId,
361+
title: getTimezoneTitle(timezoneId, date),
362+
offset: calculateTimezoneByValue(timezoneId, date),
363+
}));
364+
329365
const utils = {
330366
getDaylightOffset,
331367
getDaylightOffsetInMs,
@@ -347,10 +383,13 @@ const utils = {
347383
hasDSTInLocalTimeZone,
348384
isEqualLocalTimeZone,
349385
isEqualLocalTimeZoneByDeclaration,
350-
getTimeZones,
351386

352387
setOffsetsToDate,
353388
addOffsetsWithoutDST,
389+
390+
getTimeZoneLabelsAsyncBatch,
391+
getTimeZoneLabel,
392+
getTimeZones,
354393
};
355394

356395
export default utils;
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import macroTaskArray from '@ts/scheduler/utils/macro_task_array/index';
2+
3+
export {
4+
macroTaskArray,
5+
};
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/* eslint-disable @typescript-eslint/no-floating-promises */
2+
import {
3+
describe, expect, it, jest,
4+
} from '@jest/globals';
5+
6+
import dispatcher, { macroTaskIdSet } from './dispatcher';
7+
8+
jest.useFakeTimers();
9+
10+
describe('Scheduler', () => {
11+
describe('MacroTaskArray', () => {
12+
describe('Dispatcher', () => {
13+
describe('schedule', () => {
14+
it('should add timeout ids to timeout ids set', () => {
15+
dispatcher.schedule(jest.fn(), 0).finally(() => {});
16+
dispatcher.schedule(jest.fn(), 0).finally(() => {});
17+
18+
expect(macroTaskIdSet.size).toBe(2);
19+
});
20+
21+
it('should remove timeout id from timeout ids set after macro task execution', async () => {
22+
const p1 = dispatcher.schedule(jest.fn(), 0);
23+
const p2 = dispatcher.schedule(jest.fn(), 0);
24+
25+
jest.advanceTimersByTime(0);
26+
27+
await Promise.all([p1, p2]);
28+
29+
expect(macroTaskIdSet.size).toBe(0);
30+
});
31+
32+
it('should call callback as macro task', () => {
33+
const callbackMock = jest.fn();
34+
dispatcher.schedule(callbackMock, 0).finally(() => {});
35+
36+
expect(callbackMock).toHaveBeenCalledTimes(0);
37+
38+
jest.advanceTimersByTime(0);
39+
40+
expect(callbackMock).toHaveBeenCalledTimes(1);
41+
});
42+
43+
it('should use macroTaskTimeoutMs form macro task delay', () => {
44+
const callbackMock = jest.fn();
45+
const macroTaskDelayMs = 1000;
46+
dispatcher.schedule(callbackMock, macroTaskDelayMs).finally(() => {});
47+
48+
expect(callbackMock).toHaveBeenCalledTimes(0);
49+
50+
jest.advanceTimersByTime(macroTaskDelayMs / 2);
51+
52+
expect(callbackMock).toHaveBeenCalledTimes(0);
53+
54+
jest.advanceTimersByTime(macroTaskDelayMs / 2);
55+
56+
expect(callbackMock).toHaveBeenCalledTimes(1);
57+
});
58+
});
59+
60+
describe('dispose', () => {
61+
it('should clear scheduled macro tasks', () => {
62+
const clearTimeoutSpy = jest.spyOn(window, 'clearTimeout');
63+
64+
dispatcher.schedule(jest.fn(), 0).finally(() => {});
65+
dispatcher.schedule(jest.fn(), 0).finally(() => {});
66+
67+
const [firstId, secondId] = Array.from(macroTaskIdSet);
68+
69+
dispatcher.dispose();
70+
71+
expect(clearTimeoutSpy).toHaveBeenCalledTimes(2);
72+
expect(clearTimeoutSpy.mock.calls).toEqual([
73+
[firstId],
74+
[secondId],
75+
]);
76+
});
77+
78+
it('should clear timeout ids set', () => {
79+
dispatcher.schedule(jest.fn(), 0).finally(() => {});
80+
dispatcher.schedule(jest.fn(), 0).finally(() => {});
81+
82+
expect(macroTaskIdSet.size).toBe(2);
83+
84+
dispatcher.dispose();
85+
86+
expect(macroTaskIdSet.size).toBe(0);
87+
});
88+
});
89+
});
90+
});
91+
});
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// eslint-disable-next-line no-restricted-globals
2+
export const macroTaskIdSet = new Set<ReturnType<typeof setTimeout>>();
3+
4+
const schedule = async (
5+
callback: () => void,
6+
macroTaskTimeoutMs: number,
7+
): Promise<void> => new Promise<void>((resolve) => {
8+
// NOTE: Used setTimeout here because this method is used in heavy calculations,
9+
// and we wouldn't like to freeze the event loop by them
10+
// eslint-disable-next-line no-restricted-globals
11+
const taskId = setTimeout(() => {
12+
callback();
13+
macroTaskIdSet.delete(taskId);
14+
resolve();
15+
}, macroTaskTimeoutMs);
16+
macroTaskIdSet.add(taskId);
17+
});
18+
19+
const dispose = (): void => {
20+
Array.from(macroTaskIdSet).forEach((id) => {
21+
clearTimeout(id);
22+
macroTaskIdSet.delete(id);
23+
});
24+
};
25+
26+
export default {
27+
schedule,
28+
dispose,
29+
};
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import dispatcher from './dispatcher';
2+
import { macroTaskArrayForEach, macroTaskArrayMap } from './methods';
3+
4+
export default {
5+
forEach: macroTaskArrayForEach,
6+
map: macroTaskArrayMap,
7+
dispose: dispatcher.dispose,
8+
};

0 commit comments

Comments
 (0)