From aa17e15916c9b74d03d3e9089ee3c8c6512c0def Mon Sep 17 00:00:00 2001 From: Alex Sanders Date: Wed, 4 Oct 2023 14:27:34 +0100 Subject: [PATCH 1/4] Hydrate islands according to their priority Co-authored-by: Max Duval --- .../src/client/islands/initHydration.ts | 54 +++++++++++++------ 1 file changed, 37 insertions(+), 17 deletions(-) diff --git a/dotcom-rendering/src/client/islands/initHydration.ts b/dotcom-rendering/src/client/islands/initHydration.ts index b8b2bdbec25..3901a95458a 100644 --- a/dotcom-rendering/src/client/islands/initHydration.ts +++ b/dotcom-rendering/src/client/islands/initHydration.ts @@ -1,4 +1,5 @@ import type { EmotionCache } from '@emotion/cache'; +import { schedule } from '../../lib/scheduler'; import { doHydration } from './doHydration'; import { getConfig } from './getConfig'; import { getName } from './getName'; @@ -45,7 +46,12 @@ export const initHydration = async ( switch (deferUntil) { case 'idle': { whenIdle(() => { - void doHydration(name, props, element, emotionCache, config); + void schedule( + name, + () => + doHydration(name, props, element, emotionCache, config), + { priority: 'feature' }, + ); }); return; } @@ -54,12 +60,17 @@ export const initHydration = async ( whenVisible( element, () => { - void doHydration( + void schedule( name, - props, - element, - emotionCache, - config, + () => + doHydration( + name, + props, + element, + emotionCache, + config, + ), + { priority: 'feature' }, ); }, { rootMargin }, @@ -68,12 +79,11 @@ export const initHydration = async ( } case 'interaction': { onInteraction(element, (targetElement) => { - void doHydration( + void schedule( name, - props, - element, - emotionCache, - config, + () => + doHydration(name, props, element, emotionCache, config), + { priority: 'feature' }, ).then(() => { targetElement.dispatchEvent(new MouseEvent('click')); }); @@ -82,7 +92,12 @@ export const initHydration = async ( } case 'hash': { if (window.location.hash.includes(name) || hasLightboxHash(name)) { - void doHydration(name, props, element, emotionCache, config); + void schedule( + name, + () => + doHydration(name, props, element, emotionCache, config), + { priority: 'feature' }, + ); } else { // If we didn't find a matching hash on page load, set a // listener so that we check again each time the reader @@ -92,12 +107,17 @@ export const initHydration = async ( window.location.hash.includes(name) || hasLightboxHash(name) ) { - void doHydration( + void schedule( name, - props, - element, - emotionCache, - config, + () => + doHydration( + name, + props, + element, + emotionCache, + config, + ), + { priority: 'feature' }, ); } }); From 2bf0fe3ef850e4dfe79f74816aceea203bfa36f0 Mon Sep 17 00:00:00 2001 From: Alex Sanders Date: Thu, 5 Oct 2023 16:37:41 +0100 Subject: [PATCH 2/4] Do actually hydrate islands according to their priority... (#9035) * actually apply priority settings to islands! * dont bind --- .../src/client/islands/getPriority.ts | 22 ++++++ .../src/client/islands/initHydration.ts | 73 +++++-------------- .../src/client/islands/onInteraction.ts | 4 +- .../src/client/islands/whenVisible.ts | 4 +- dotcom-rendering/src/lib/scheduler.ts | 6 +- 5 files changed, 50 insertions(+), 59 deletions(-) create mode 100644 dotcom-rendering/src/client/islands/getPriority.ts diff --git a/dotcom-rendering/src/client/islands/getPriority.ts b/dotcom-rendering/src/client/islands/getPriority.ts new file mode 100644 index 00000000000..d7875c0b934 --- /dev/null +++ b/dotcom-rendering/src/client/islands/getPriority.ts @@ -0,0 +1,22 @@ +import type { Priority } from '../../lib/scheduler'; +import { isValidSchedulerPriority } from '../../lib/scheduler'; + +/** + * getPriority takes the given html element and returns its priority attribute + * + * We expect the element to always be a `gu-*` custom element + * + * @param marker : The html element that we want to read the priority attribute from; + * @returns + */ +export const getPriority = (marker: HTMLElement): Priority | undefined => { + const priority = marker.getAttribute('priority'); + + if (isValidSchedulerPriority(priority)) { + return priority; + } + + console.error('Unable to find priority attribute on gu-island', marker); + + return; +}; diff --git a/dotcom-rendering/src/client/islands/initHydration.ts b/dotcom-rendering/src/client/islands/initHydration.ts index 3901a95458a..a589eadd99e 100644 --- a/dotcom-rendering/src/client/islands/initHydration.ts +++ b/dotcom-rendering/src/client/islands/initHydration.ts @@ -1,8 +1,10 @@ import type { EmotionCache } from '@emotion/cache'; +import { isUndefined } from '@guardian/libs'; import { schedule } from '../../lib/scheduler'; import { doHydration } from './doHydration'; import { getConfig } from './getConfig'; import { getName } from './getName'; +import { getPriority } from './getPriority'; import { getProps } from './getProps'; import { onInteraction } from './onInteraction'; import { onNavigation } from './onNavigation'; @@ -39,65 +41,39 @@ export const initHydration = async ( const name = getName(element); const props = getProps(element); const config = getConfig(element); + const priority = getPriority(element); if (!name) return; + if (isUndefined(priority)) return; + + const scheduleHydration = () => + schedule( + name, + () => doHydration(name, props, element, emotionCache, config), + { priority }, + ); const deferUntil = element.getAttribute('deferuntil'); switch (deferUntil) { case 'idle': { - whenIdle(() => { - void schedule( - name, - () => - doHydration(name, props, element, emotionCache, config), - { priority: 'feature' }, - ); - }); + whenIdle(scheduleHydration); return; } case 'visible': { const rootMargin = element.getAttribute('rootmargin') ?? undefined; - whenVisible( - element, - () => { - void schedule( - name, - () => - doHydration( - name, - props, - element, - emotionCache, - config, - ), - { priority: 'feature' }, - ); - }, - { rootMargin }, - ); + whenVisible(element, scheduleHydration, { rootMargin }); return; } case 'interaction': { - onInteraction(element, (targetElement) => { - void schedule( - name, - () => - doHydration(name, props, element, emotionCache, config), - { priority: 'feature' }, - ).then(() => { - targetElement.dispatchEvent(new MouseEvent('click')); - }); + onInteraction(element, async (targetElement) => { + await scheduleHydration(); + targetElement.dispatchEvent(new MouseEvent('click')); }); return; } case 'hash': { if (window.location.hash.includes(name) || hasLightboxHash(name)) { - void schedule( - name, - () => - doHydration(name, props, element, emotionCache, config), - { priority: 'feature' }, - ); + return scheduleHydration(); } else { // If we didn't find a matching hash on page load, set a // listener so that we check again each time the reader @@ -107,25 +83,14 @@ export const initHydration = async ( window.location.hash.includes(name) || hasLightboxHash(name) ) { - void schedule( - name, - () => - doHydration( - name, - props, - element, - emotionCache, - config, - ), - { priority: 'feature' }, - ); + void scheduleHydration(); } }); } return; } default: { - return doHydration(name, props, element, emotionCache, config); + return scheduleHydration(); } } }; diff --git a/dotcom-rendering/src/client/islands/onInteraction.ts b/dotcom-rendering/src/client/islands/onInteraction.ts index d06bc73fd8c..079513648fe 100644 --- a/dotcom-rendering/src/client/islands/onInteraction.ts +++ b/dotcom-rendering/src/client/islands/onInteraction.ts @@ -7,13 +7,13 @@ */ export const onInteraction = ( element: HTMLElement, - callback: (e: HTMLElement) => void, + callback: (e: HTMLElement) => Promise, ): void => { element.addEventListener( 'click', (e) => { if (e.target instanceof HTMLElement) { - callback(e.target); + void callback(e.target); } }, { once: true }, diff --git a/dotcom-rendering/src/client/islands/whenVisible.ts b/dotcom-rendering/src/client/islands/whenVisible.ts index 3383d344605..74e9984daed 100644 --- a/dotcom-rendering/src/client/islands/whenVisible.ts +++ b/dotcom-rendering/src/client/islands/whenVisible.ts @@ -13,7 +13,7 @@ type WhenVisibleOptions = { */ export const whenVisible = ( element: HTMLElement, - callback: () => void, + callback: () => Promise, { rootMargin }: WhenVisibleOptions = { rootMargin: '100px' }, ): void => { if ('IntersectionObserver' in window) { @@ -22,7 +22,7 @@ export const whenVisible = ( if (!entry?.isIntersecting) return; // Disconnect this IntersectionObserver once seen io.disconnect(); - callback(); + void callback(); }, { rootMargin }, ); diff --git a/dotcom-rendering/src/lib/scheduler.ts b/dotcom-rendering/src/lib/scheduler.ts index fc14138ef09..ee7b243d60d 100644 --- a/dotcom-rendering/src/lib/scheduler.ts +++ b/dotcom-rendering/src/lib/scheduler.ts @@ -1,4 +1,6 @@ import { startPerformanceMeasure } from '@guardian/libs'; +import type { Guard } from './guard'; +import { guard } from './guard'; const START = Date.now(); @@ -22,9 +24,11 @@ let CONCURRENCY_COUNT = Infinity; * priorities, and the scheduler will prefer the priority with the lowest index. **/ const PRIORITIES = ['critical', 'feature', 'enhancement'] as const; -type Priority = (typeof PRIORITIES)[number]; +export type Priority = Guard; export type SchedulePriority = { [K in Priority]: K }; +export const isValidSchedulerPriority = guard(PRIORITIES); + /** * A thing that a consumer want to do. Should be a function that returns a promise. */ From f689b804a25adf12c02fcfde6367eca8f11d6ac4 Mon Sep 17 00:00:00 2001 From: Alex Sanders Date: Thu, 5 Oct 2023 17:07:44 +0100 Subject: [PATCH 3/4] docs(getPriority): more explicit error message Co-authored-by: Charlotte Emms <43961396+cemms1@users.noreply.github.com> --- dotcom-rendering/src/client/islands/getPriority.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/dotcom-rendering/src/client/islands/getPriority.ts b/dotcom-rendering/src/client/islands/getPriority.ts index d7875c0b934..7fd2748f9b1 100644 --- a/dotcom-rendering/src/client/islands/getPriority.ts +++ b/dotcom-rendering/src/client/islands/getPriority.ts @@ -16,7 +16,10 @@ export const getPriority = (marker: HTMLElement): Priority | undefined => { return priority; } - console.error('Unable to find priority attribute on gu-island', marker); + console.error( + 'Unable to find valid priority attribute on gu-island', + marker, + ); return; }; From 70d31824bd55953108e047e252723ac94f61c08f Mon Sep 17 00:00:00 2001 From: Alex Sanders Date: Fri, 6 Oct 2023 10:33:13 +0100 Subject: [PATCH 4/4] push the void to the top level of initHydration --- dotcom-rendering/src/client/islands/initHydration.ts | 6 ++++-- dotcom-rendering/src/client/islands/onInteraction.ts | 2 +- dotcom-rendering/src/client/islands/whenVisible.ts | 4 ++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/dotcom-rendering/src/client/islands/initHydration.ts b/dotcom-rendering/src/client/islands/initHydration.ts index a589eadd99e..196a3e40b98 100644 --- a/dotcom-rendering/src/client/islands/initHydration.ts +++ b/dotcom-rendering/src/client/islands/initHydration.ts @@ -56,12 +56,14 @@ export const initHydration = async ( const deferUntil = element.getAttribute('deferuntil'); switch (deferUntil) { case 'idle': { - whenIdle(scheduleHydration); + whenIdle(() => void scheduleHydration()); return; } case 'visible': { const rootMargin = element.getAttribute('rootmargin') ?? undefined; - whenVisible(element, scheduleHydration, { rootMargin }); + whenVisible(element, () => void scheduleHydration(), { + rootMargin, + }); return; } case 'interaction': { diff --git a/dotcom-rendering/src/client/islands/onInteraction.ts b/dotcom-rendering/src/client/islands/onInteraction.ts index 079513648fe..5d6599ca54f 100644 --- a/dotcom-rendering/src/client/islands/onInteraction.ts +++ b/dotcom-rendering/src/client/islands/onInteraction.ts @@ -7,7 +7,7 @@ */ export const onInteraction = ( element: HTMLElement, - callback: (e: HTMLElement) => Promise, + callback: (e: HTMLElement) => Promise | void, ): void => { element.addEventListener( 'click', diff --git a/dotcom-rendering/src/client/islands/whenVisible.ts b/dotcom-rendering/src/client/islands/whenVisible.ts index 74e9984daed..3383d344605 100644 --- a/dotcom-rendering/src/client/islands/whenVisible.ts +++ b/dotcom-rendering/src/client/islands/whenVisible.ts @@ -13,7 +13,7 @@ type WhenVisibleOptions = { */ export const whenVisible = ( element: HTMLElement, - callback: () => Promise, + callback: () => void, { rootMargin }: WhenVisibleOptions = { rootMargin: '100px' }, ): void => { if ('IntersectionObserver' in window) { @@ -22,7 +22,7 @@ export const whenVisible = ( if (!entry?.isIntersecting) return; // Disconnect this IntersectionObserver once seen io.disconnect(); - void callback(); + callback(); }, { rootMargin }, );