[WOM-5377][BpkComponentCalendar] Optimise date formatting for improved INP #3695

gwright170 · 2024-12-13T16:40:26Z

Address performance issues when instantiating BpkComponentCalendar related to excessive date formatting (many times for 13 months worth of individual days). For Skyscanner staff please refer to https://skyscanner.atlassian.net/wiki/x/9eCWSw for complete analysis of the changes:

Front-load date formatting in component constructor and make available via state
Use formatted date values from state as opposed to formatting on demand
Do not require date formatting to compare dates (reducing the number of times we format dates)
Make iso formatted dates available to consumers to reduce their need to format dates
Defer generation of date aria-labels until after initial render
memoize formatDateFull in composeCalendar

Note

Deferring generation of date aria-labels is a workaround for a poorly performing date format function from saddlebag-localisation that Skyscanner are mandated to use - see test-localisation-perf. This solution isn't inherently bad (it's a method used by React Fibre for prioritising render) but adds complexity and indicates wider performance issues we should aim to resolve.

Remember to include the following changes:

- reduce usage of date formats

github-actions · 2024-12-13T16:44:09Z

Visit https://backpack.github.io/storybook-prs/3695 to see this build running in a browser.

github-actions · 2024-12-16T16:40:25Z

Visit https://backpack.github.io/storybook-prs/3695 to see this build running in a browser.

github-actions · 2024-12-18T09:23:47Z

Visit https://backpack.github.io/storybook-prs/3695 to see this build running in a browser.

github-actions · 2024-12-18T09:25:33Z

	Warnings
⚠️	Package source files (e.g. `packages/package-name/src/Component.js`) were updated, but snapshots weren't. Have you checked that the tests still pass?
⚠️	Package source files (e.g. `packages/package-name/src/Component.tsx`) were updated, but type files weren't. Have you checked that no types have changed?

Browser support

If this is a visual change, make sure you've tested it in multiple browsers.

Generated by 🚫 dangerJS against ea2221f

github-actions · 2024-12-18T09:45:46Z

Visit https://backpack.github.io/storybook-prs/3695 to see this build running in a browser.

github-actions · 2024-12-18T12:03:07Z

Visit https://backpack.github.io/storybook-prs/3695 to see this build running in a browser.

github-actions · 2024-12-18T12:05:01Z

Visit https://backpack.github.io/storybook-prs/3695 to see this build running in a browser.

gwright170 · 2024-12-18T15:37:43Z

packages/bpk-component-calendar/src/BpkCalendarDate.tsx

@@ -185,6 +186,7 @@ class BpkCalendarDate extends PureComponent<Props> {
    }

    delete buttonProps.preventKeyboardFocus;
+    delete buttonProps.formattedDate;


Ensures prop isn't passed to html button, invalidating existing snapshots

Can you explain a little more what is going on here, I'm not sure I understand.

The returned component includes {...buttonProps}, so the html includes formattedDate="2024-12-19". This isn't an issue but:

wasn't intentional

invalidates all the existing snapshot tests

Right, button props is derived from spreading the rest of this.props. Where does this component end up getting used? I can't find where we end up needing the new formattedDate prop, but I've not got a good picture of the Component hierarchy here.

In my case it is used in the Flights Search Controls GC to prevent re-generating iso date labels - https://github.skyscannertools.net/skyscanner/global-components/pull/1607/files

I still don't understand, the prop is passed into the component, it's not referenced anywhere nor is this.props passed into anything then its deleted from buttonProps before it's used. How does including this prop in this component do anything?

Apologies. The Global Component uses a custom Date component:
composeCalendar(null, BpkCalendarGridHeader, BpkScrollableCalendarGridList, Date).

We pass formattedDate to the Date component. In the case of the GC this value is valuable, we use it to prevent duplicate generation of iso formatted dates (these dates are used to colour code dates with prices).

In the case of the default Backpack Date component that formattedDate value is not useful, we delete it from the props to avoid it appearing in the HTML and snapshots.

gwright170 · 2024-12-18T15:40:12Z

packages/bpk-component-calendar/src/BpkCalendarGrid.tsx

+  componentDidMount(): void {
+    deferCallback(() =>
+      this.setState({
+        calendarMonthWeeks: getCalendarMonthWeeks(
+          this.props.month,
+          this.props.weekStartsOn,
+          this.props.formatDateFull,
+        ),
+      }),
+    );
+  }


In the constructor we get dates and formatted labels for everything but the aria-label. Here we regenerate dates, this time with aria labels, and defer until the thread is idle (typically after render). This change reduces INP by roughly 20%.

gwright170 · 2024-12-18T15:41:33Z

packages/bpk-component-calendar/src/BpkCalendarWeek.tsx

Changes in this file cover simplification of date comparison. There were a few places we needlessly formatted dates for comparison. This change reduces INP by roughly 20%.

gwright170 · 2024-12-18T15:43:21Z

packages/bpk-component-calendar/src/composeCalendar.tsx

consistent memoization of formatDateFull (no need for consumers to implement this now)

gwright170 · 2024-12-18T15:44:07Z

packages/bpk-component-calendar/src/date-utils.ts

+        val: currDate,
+        textLabel: formatDate ? formatDate(currDate) : 'loading...',
+        isoLabel: formatIsoDate(currDate),


Perform all date formatting in a single place, rather than on demand across many files. This decreases the number of date format calls requires and contributes to a 20% reduction in INP.

gwright170 · 2024-12-18T15:44:44Z

packages/bpk-component-carousel/src/utils-test.tsx

+    const element = document.createElement('div');
+    element.scrollIntoView = jest.fn();


This test was broken but was not being run because the test pattern specifies -test rather than .test. Renamed and fixed.

gwright170 · 2024-12-18T15:44:59Z

packages/bpk-react-utils/src/__mocks__/deferCallback.ts

+ */
+
+/**
+ * Mock deferCallback to immediately invoke the callback. This ensures that


gwright170 · 2024-12-18T15:45:09Z

packages/bpk-react-utils/src/deferCallback.ts

+ */
+
+/**
+ * Defer a task until the browser is idle, or till the next "tick" if the


gwright170 · 2024-12-18T15:45:55Z

scripts/jest/setup.js

 registerRequireContextHook();
+
+jest.mock('../../packages/bpk-react-utils/src/deferCallback');


Globally mock deferCallback to ensure calendar snapshots include real aria label on initial render, not 'loading...' fallback.

scripts/jest/setup.js

packages/bpk-react-utils/index.ts

packages/bpk-react-utils/src/deferCallback.ts

packages/bpk-component-calendar/src/BpkCalendarGrid.tsx

mungodewar · 2024-12-19T09:49:57Z

packages/bpk-component-calendar/src/BpkCalendarDate.tsx

@@ -185,6 +186,7 @@ class BpkCalendarDate extends PureComponent<Props> {
    }

    delete buttonProps.preventKeyboardFocus;
+    delete buttonProps.formattedDate;


Can you explain a little more what is going on here, I'm not sure I understand.

packages/bpk-component-calendar/src/date-utils.ts

gwright170 · 2024-12-19T12:21:09Z

packages/bpk-component-calendar/src/date-utils.ts

+function getCalendarNoCustomLabel(date: Date, weekStartsOn: number) {
+  return getCalendarMonthWeeks(date, weekStartsOn);
+}
+
+function getCalendar(
+  date: Date,
+  weekStartsOn: number,
+  formatDate: (d: Date) => Date | string,
+) {
+  return getCalendarMonthWeeks(date, weekStartsOn, formatDate);
+}


Appreciate this could be cleaner but:

I wanted to avoid separating the values into new state to avoid more refactoring and complexity to rationalise back together

Mapping the values in after the initial generation is arguably slower and harder to read

github-actions · 2024-12-19T12:21:45Z

Visit https://backpack.github.io/storybook-prs/3695 to see this build running in a browser.

github-actions · 2024-12-19T15:14:56Z

Visit https://backpack.github.io/storybook-prs/3695 to see this build running in a browser.

github-actions · 2024-12-19T15:40:21Z

Visit https://backpack.github.io/storybook-prs/3695 to see this build running in a browser.

github-actions · 2024-12-19T16:15:18Z

Visit https://backpack.github.io/storybook-prs/3695 to see this build running in a browser.

- front-load date formatting for better INP

6e5d73b

- reduce usage of date formats

defer aria label formatting until thread is idle

96a051c

gwright170 changed the title ~~WOM-000 | Front-load date formatting for better INP~~ WOM-000 | Optimise date formatting for optimal Interaction to Next Paint (INP) Dec 16, 2024

gwright170 changed the title ~~WOM-000 | Optimise date formatting for optimal Interaction to Next Paint (INP)~~ WOM-000 | Optimise date formatting for optimal Interaction to Next Paint Dec 16, 2024

gwright170 changed the title ~~WOM-000 | Optimise date formatting for optimal Interaction to Next Paint~~ WOM-000 | Optimise date formatting for optimal INP Dec 16, 2024

gwright170 changed the title ~~WOM-000 | Optimise date formatting for optimal INP~~ [WOM-5377] Optimise date formatting for optimal INP Dec 16, 2024

resolve unit test failures

5b9ea00

gwright170 changed the title ~~[WOM-5377] Optimise date formatting for optimal INP~~ [WOM-5377][BpkComponentCalendar] Optimise date formatting for improved INP Dec 18, 2024

George Wright added 2 commits December 18, 2024 09:28

add license heading to new files

e8eaacb

rename test files to match expected pattern

d62ae30

George Wright and others added 3 commits December 18, 2024 11:53

do not mock deferCallback in its own tests

9588ca8

resolve utils unit test failure

34f9971

Trigger Build

3ce975b

gwright170 added the minor Non breaking change label Dec 18, 2024

gwright170 marked this pull request as ready for review December 18, 2024 13:54

gwright170 commented Dec 18, 2024

View reviewed changes

shaundillonwork approved these changes Dec 18, 2024

View reviewed changes

mungodewar reviewed Dec 19, 2024

View reviewed changes

address review comments

01664d5

gwright170 commented Dec 19, 2024

View reviewed changes

resolve "Week" storybook failure

00016e5

reduce requestIdleCallback timeout to 500ms

f3b8da6

gwright170 added major Breaking change and removed minor Non breaking change labels Dec 19, 2024

resolve unit test failures

ea2221f

gwright170 added minor Non breaking change and removed major Breaking change labels Dec 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WOM-5377][BpkComponentCalendar] Optimise date formatting for improved INP #3695

[WOM-5377][BpkComponentCalendar] Optimise date formatting for improved INP #3695

gwright170 commented Dec 13, 2024 •

edited

Loading

github-actions bot commented Dec 13, 2024

github-actions bot commented Dec 16, 2024

github-actions bot commented Dec 18, 2024

github-actions bot commented Dec 18, 2024 •

edited

Loading

github-actions bot commented Dec 18, 2024

github-actions bot commented Dec 18, 2024

github-actions bot commented Dec 18, 2024

gwright170 Dec 18, 2024

mungodewar Dec 19, 2024

gwright170 Dec 19, 2024

mungodewar Dec 19, 2024

gwright170 Dec 19, 2024 •

edited

Loading

mungodewar Dec 19, 2024

gwright170 Dec 19, 2024 •

edited

Loading

gwright170 Dec 18, 2024

gwright170 Dec 18, 2024

gwright170 Dec 18, 2024

gwright170 Dec 18, 2024

gwright170 Dec 18, 2024

gwright170 Dec 18, 2024

gwright170 Dec 18, 2024

gwright170 Dec 18, 2024

mungodewar Dec 19, 2024

gwright170 Dec 19, 2024

github-actions bot commented Dec 19, 2024

github-actions bot commented Dec 19, 2024

github-actions bot commented Dec 19, 2024

github-actions bot commented Dec 19, 2024

		const element = document.createElement('div');
		element.scrollIntoView = jest.fn();

		registerRequireContextHook();

		jest.mock('../../packages/bpk-react-utils/src/deferCallback');

[WOM-5377][BpkComponentCalendar] Optimise date formatting for improved INP #3695

Are you sure you want to change the base?

[WOM-5377][BpkComponentCalendar] Optimise date formatting for improved INP #3695

Conversation

gwright170 commented Dec 13, 2024 • edited Loading

github-actions bot commented Dec 13, 2024

github-actions bot commented Dec 16, 2024

github-actions bot commented Dec 18, 2024

github-actions bot commented Dec 18, 2024 • edited Loading

Browser support

github-actions bot commented Dec 18, 2024

github-actions bot commented Dec 18, 2024

github-actions bot commented Dec 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gwright170 Dec 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gwright170 Dec 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Dec 19, 2024

github-actions bot commented Dec 19, 2024

github-actions bot commented Dec 19, 2024

github-actions bot commented Dec 19, 2024

gwright170 commented Dec 13, 2024 •

edited

Loading

github-actions bot commented Dec 18, 2024 •

edited

Loading

gwright170 Dec 19, 2024 •

edited

Loading

gwright170 Dec 19, 2024 •

edited

Loading