[WOM-5333] Wrap BpkCalendar format date methods with memoize #3689
Conversation
|
Visit https://backpack.github.io/storybook-prs/3689 to see this build running in a browser. |
Browser supportIf this is a visual change, make sure you've tested it in multiple browsers. |
|
Hi @sarak-works, Thanks for the contribution! I suppose we could validate this be memoizing the props that are passed into the composeCalendar function? Then it could be proven before making this change? Though I don't think this is a blocker for this PR. |
| @@ -19,6 +19,8 @@ | |||
| import type { ElementType } from 'react'; | |||
| import { Component } from 'react'; | |||
|
|
|||
| import { memoize } from 'lodash'; | |||
There was a problem hiding this comment.
memoize is decent sized package. Could we memoize a different way to avoid bundling that? Like using a map instead
https://bundlephobia.com/package/lodash.memoize@4.1.2
| formatDateFull={formatDateFull} | ||
| formatMonth={formatMonth} | ||
| formatDateFull={memoize(formatDateFull)} | ||
| formatMonth={memoize(formatMonth)} |
There was a problem hiding this comment.
I looked into this before and I think there is a case where you might be memoizing twice here if the consumer uses BpkCalendarGrid as the "Grid"....which I think most consumers will
There was a problem hiding this comment.
I see, so I guess it'd be safe to only memoize as part of BpkCalendarGrid and monitor it
We made a similar fix in the GC and found it to have the desired effect of reducing the time spent in calling formatDate and hence date-fns 👿 |
|
Visit https://backpack.github.io/storybook-prs/3689 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3689 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3689 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3689 to see this build running in a browser. |
Since INP has been introduced as a Core Web Vital (CWV) in March 2024, Skyscanner has struggled to maintain a good rating.
INP stands for “Interaction To Next Paint” and is essentially the time it takes for a UI to respond to a users action (like a press or a click). A lot of evidence suggests that a lot of this is down to the Calendar found in our design system “Backpack”.
Analysis shows that we spend a lot of CPU/compute on formatting dates, which we do multiple times on each render for each date. This compute causes the UI to be slow and unresponsive on CPU limited devices (like your great aunts old Samsung Android Phone).
The theory is that using a Memoize function in front of these expensive functions can bring down this compute cost.