Skip to content

Commit ec35e00

Browse files
mxdvlsndrs
andauthored
Islands can no longer be “client-side only” (#8948)
* refactor(Island): never client-side only Components that rely on accessing `window` or `document` should handle this internally, rather than relying on a property of their parent Island. The behaviour becomes more explicit and less prone to errors. * refactor: storybook is always hydrated individual components can choose to opt out of hydration via the useHydrated hook. --------- Co-authored-by: Alex Sanders <[email protected]>
1 parent 0a2ef42 commit ec35e00

File tree

64 files changed

+353
-418
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

64 files changed

+353
-418
lines changed

dotcom-rendering/.storybook/preview.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ if (isChromatic()) {
2323
MockDate.set('Sat Jan 1 2022 12:00:00 GMT+0000 (Greenwich Mean Time)');
2424
}
2525

26+
window.IS_STORYBOOK = true;
27+
2628
mockRESTCalls();
2729

2830
setABTests(

dotcom-rendering/cypress/e2e/parallel-1/article.e2e.cy.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ describe('E2E Page rendering', function () {
7878

7979
cy.get('gu-island[name=MostViewedFooterData]', { timeout: 30000 })
8080
.scrollIntoView({ duration: 100 })
81-
.should('have.attr', 'data-island-status', 'rendered');
81+
.should('have.attr', 'data-island-status', 'hydrated');
8282

8383
cy.get('[data-cy-ab-user-in-variant=ab-test-variant]').should(
8484
'be.visible',
@@ -104,7 +104,7 @@ describe('E2E Page rendering', function () {
104104

105105
cy.get('gu-island[name=MostViewedFooterData]', { timeout: 30000 })
106106
.scrollIntoView({ duration: 100 })
107-
.should('have.attr', 'data-island-status', 'rendered');
107+
.should('have.attr', 'data-island-status', 'hydrated');
108108

109109
cy.get('[data-cy-ab-user-in-variant=ab-test-not-in-test]').should(
110110
'be.visible',

dotcom-rendering/cypress/e2e/parallel-3/article.interactivity.cy.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ describe('Interactivity', function () {
100100
// Wait for hydration
101101
cy.get('gu-island[name=MostViewedFooterData]')
102102
.last()
103-
.should('have.attr', 'data-island-status', 'rendered');
103+
.should('have.attr', 'data-island-status', 'hydrated');
104104
cy.wait('@getMostRead');
105105
cy.wait('@getMostReadGeo');
106106
cy.get('[data-cy=mostviewed-footer]').should('exist');

dotcom-rendering/cypress/e2e/parallel-3/epic.interactivity.cy.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ describe('Epics', function () {
3232
// Wait for hydration of the Epic
3333
cy.get('gu-island[name=LiveBlogEpic]')
3434
.first()
35-
.should('have.attr', 'data-island-status', 'rendered');
35+
.should('have.attr', 'data-island-status', 'hydrated');
3636
cy.get('[data-cy=contributions-liveblog-epic]').scrollIntoView();
3737
cy.get('[data-cy=contributions-liveblog-epic]').should('be.visible');
3838
});

dotcom-rendering/cypress/e2e/parallel-5/liveblog.interactivity.cy.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ describe('Liveblogs', function () {
4747
// Wait for hydration
4848
cy.get('gu-island[name=Liveness]')
4949
.first()
50-
.should('have.attr', 'data-island-status', 'rendered');
50+
.should('have.attr', 'data-island-status', 'hydrated');
5151
cy.scrollTo('center');
5252
cy.get(`[data-cy="toast"]`).should('not.exist');
5353
cy.window().then(function (win) {
@@ -75,7 +75,7 @@ describe('Liveblogs', function () {
7575
// Wait for hydration
7676
cy.get('gu-island[name=Liveness]')
7777
.first()
78-
.should('have.attr', 'data-island-status', 'rendered');
78+
.should('have.attr', 'data-island-status', 'hydrated');
7979
cy.window().then(function (win) {
8080
win.mockLiveUpdate({
8181
numNewBlocks: 1,
@@ -92,7 +92,7 @@ describe('Liveblogs', function () {
9292
// Wait for hydration
9393
cy.get('gu-island[name=Liveness]', { timeout: 30000 })
9494
.first()
95-
.should('have.attr', 'data-island-status', 'rendered');
95+
.should('have.attr', 'data-island-status', 'hydrated');
9696
cy.scrollTo('bottom');
9797
cy.get(`[data-cy="toast"]`).should('not.exist');
9898
cy.window().then(function (win) {
@@ -128,7 +128,7 @@ describe('Liveblogs', function () {
128128
// Wait for hydration
129129
cy.get('gu-island[name=Liveness]')
130130
.first()
131-
.should('have.attr', 'data-island-status', 'rendered');
131+
.should('have.attr', 'data-island-status', 'hydrated');
132132
cy.window().then(function (win) {
133133
win.mockLiveUpdate({
134134
numNewBlocks: 1,
@@ -171,7 +171,7 @@ describe('Liveblogs', function () {
171171
// Wait for hydration
172172
cy.get('gu-island[name=Liveness]', { timeout: 30000 })
173173
.first()
174-
.should('have.attr', 'data-island-status', 'rendered');
174+
.should('have.attr', 'data-island-status', 'hydrated');
175175
cy.scrollTo('bottom');
176176
cy.get(`[data-cy="toast"]`).should('not.exist');
177177
cy.window().then(function (win) {
@@ -198,7 +198,7 @@ describe('Liveblogs', function () {
198198
// Wait for hydration
199199
cy.get('gu-island[name=Liveness]')
200200
.first()
201-
.should('have.attr', 'data-island-status', 'rendered');
201+
.should('have.attr', 'data-island-status', 'hydrated');
202202
cy.scrollTo('bottom', { duration: 1000 });
203203
cy.window().then(function (win) {
204204
win.mockLiveUpdate({

dotcom-rendering/cypress/support/commands.js

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,10 @@ Cypress.Commands.add('hydrate', () => {
3535
const islandMeta = `island: ${name} defer: ${deferuntil}`;
3636

3737
if (['idle', 'visible', undefined].includes(deferuntil)) {
38-
const action = !!el.attr('clientOnly')
39-
? 'rendered'
40-
: 'hydrated';
4138
cy.log(`Scrolling to ${islandMeta}`);
4239
cy.wrap(el)
4340
.scrollIntoView({ duration: 1000, timeout: 30000 })
44-
.should('have.attr', 'data-island-status', action, {
41+
.should('have.attr', 'data-island-status', 'hydrated', {
4542
timeout: 30000,
4643
});
4744
// Additional wait to ensure island defer=visible has triggered

dotcom-rendering/index.d.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -455,8 +455,6 @@ type RichLinkCardType =
455455
// 3rd party type declarations //
456456
// ------------------------------
457457

458-
declare module 'chromatic/isChromatic';
459-
460458
declare module 'dynamic-import-polyfill' {
461459
export const initialize: ({
462460
modulePath,
@@ -518,7 +516,6 @@ declare namespace JSX {
518516
name: string;
519517
deferUntil?: 'idle' | 'visible' | 'interaction' | 'hash';
520518
rootMargin?: string;
521-
clientOnly?: boolean;
522519
props: any;
523520
children: React.ReactNode;
524521
/**

dotcom-rendering/src/client/islands/doHydration.tsx

Lines changed: 14 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import type { EmotionCache } from '@emotion/react';
33
import { CacheProvider } from '@emotion/react';
44
import { log, startPerformanceMeasure } from '@guardian/libs';
55
import { createElement } from 'react';
6-
import { createRoot, hydrateRoot } from 'react-dom/client';
6+
import { hydrateRoot } from 'react-dom/client';
77
import { ConfigProvider } from '../../components/ConfigContext';
88
import type { Config } from '../../types/configContext';
99

@@ -13,17 +13,14 @@ declare global {
1313
* Defines the current state of the Island.
1414
* `undefined` at server-side rendering
1515
*/
16-
islandStatus?: 'identified' | 'imported' | 'rendered' | 'hydrated';
16+
islandStatus?: 'identified' | 'imported' | 'hydrated';
1717
}
1818
}
1919

2020
/**
2121
* This function dynamically imports and then hydrates a specific component in
2222
* a specific part of the page
2323
*
24-
* If the content being hydrated is not present in the dom then React renders
25-
* it. This is how portals (non server side rendered content) work
26-
*
2724
* @param name The name of the component we want to hydrate
2825
* @param data The deserialised props we want to use for hydration
2926
* @param element The location on the DOM where the component to hydrate exists
@@ -52,47 +49,33 @@ export const doHydration = async (
5249
.then((module) => {
5350
/** The duration of importing the module for this island */
5451
const importDuration = endImportPerformanceMeasure();
55-
const clientOnly = element.hasAttribute('clientonly');
5652
element.dataset.islandStatus = 'imported';
5753

5854
const { endPerformanceMeasure: endIslandPerformanceMeasure } =
5955
startPerformanceMeasure('dotcom', name, 'island');
6056

61-
if (clientOnly) {
62-
element.querySelector('[data-name="placeholder"]')?.remove();
63-
const root = createRoot(element);
64-
root.render(
65-
<ConfigProvider value={config}>
66-
<CacheProvider value={emotionCache}>
67-
{createElement(module[name], data)}
68-
</CacheProvider>
69-
</ConfigProvider>,
70-
);
71-
} else {
72-
hydrateRoot(
73-
element,
74-
<ConfigProvider value={config}>
75-
<CacheProvider value={emotionCache}>
76-
{createElement(module[name], data)}
77-
</CacheProvider>
78-
</ConfigProvider>,
79-
);
80-
}
57+
hydrateRoot(
58+
element,
59+
<ConfigProvider value={config}>
60+
<CacheProvider value={emotionCache}>
61+
{createElement(module[name], data)}
62+
</CacheProvider>
63+
</ConfigProvider>,
64+
);
8165

8266
/** The duration of rendering or hydrating this island */
8367
const islandDuration = endIslandPerformanceMeasure();
8468

85-
return { clientOnly, importDuration, islandDuration };
69+
return { importDuration, islandDuration };
8670
})
87-
.then(({ clientOnly, importDuration, islandDuration }) => {
71+
.then(({ importDuration, islandDuration }) => {
8872
if (!('getEntriesByType' in window.performance)) return;
8973

90-
const action = clientOnly ? 'rendered' : 'hydrated';
91-
element.dataset.islandStatus = action;
74+
element.dataset.islandStatus = 'hydrated';
9275

9376
log(
9477
'dotcom',
95-
`🏝 <${name} /> ${action} in ${islandDuration}ms (imported in ${importDuration}ms)`,
78+
`🏝 hydrated <${name} /> in ${islandDuration}ms (imported in ${importDuration}ms)`,
9679
);
9780
})
9881
.catch((error) => {

dotcom-rendering/src/client/islands/doStorybookHydration.js

Lines changed: 0 additions & 55 deletions
This file was deleted.

dotcom-rendering/src/client/ophan/ophan.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import type {
66
OphanComponentEvent,
77
} from '@guardian/libs';
88
import { log } from '@guardian/libs';
9+
import { isServer } from '../../lib/isServer';
910
import type { ServerSideTests } from '../../types/config';
1011

1112
export type OphanRecordFunction = (
@@ -55,15 +56,15 @@ export const getOphan = async (): Promise<
5556
* @deprecated use `getOphan` instead
5657
*/
5758
export const getOphanRecordFunction = (): OphanRecordFunction => {
58-
const record = window.guardian.ophan?.record;
59-
60-
if (record) return record;
59+
if (isServer || window.guardian.ophan === undefined) {
60+
// eslint-disable-next-line no-console -- worth informing all users
61+
console.warn('window.guardian.ophan.record is not available');
62+
return () => {
63+
/* do nothing */
64+
};
65+
}
6166

62-
// eslint-disable-next-line no-console -- worth informing all users
63-
console.warn('window.guardian.ophan.record is not available');
64-
return () => {
65-
/* do nothing */
66-
};
67+
return window.guardian.ophan.record;
6768
};
6869

6970
/**

0 commit comments

Comments
 (0)