Skip to content

Commit 9f388d6

Browse files
authored
Scheduler(T1269168): fix appointment popup delay performance issue (#29069) (#29097)
1 parent 7fcbfe1 commit 9f388d6

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,15 +1,30 @@
11
/* globals Intl */
2+
// TODO(Refactoring): move this module to ./utils directory
23
import errors from '@js/core/errors';
34
import { dateUtilsTs } from '@ts/core/utils/date';
5+
import { macroTaskArray } from '@ts/scheduler/utils/index';
46

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

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

@@ -38,12 +53,6 @@ const createDateFromUTCWithLocalOffset = (date) => {
3853
return result.source;
3954
};
4055

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

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

353388
setOffsetsToDate,
354389
addOffsetsWithoutDST,
390+
391+
getTimeZoneLabelsAsyncBatch,
392+
getTimeZoneLabel,
393+
getTimeZones,
355394
};
356395

357396
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)