Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🏝 React Islands #3629

Merged
merged 19 commits into from
Dec 16, 2021
Merged

🏝 React Islands #3629

merged 19 commits into from
Dec 16, 2021

Conversation

oliverlloyd
Copy link
Contributor

@oliverlloyd oliverlloyd commented Nov 12, 2021

What?

React islands are a form of partial hydration where instead of hydrating the entire server side rendered page, we only hydrate the parts of the page that contain interactivity.

This PR takes the existing partial hydration solution in DCR, extends it to be more aligned with the original island hydration work by @AWare and @walaura and then adds some new features inspired by AstroJS.

Why?

The primary goal for this work was to provide a better developer experience for other teams working on DCR but we're also adding two features here that were raised on the original PR by @gtrufitt that we never got around to implementing.

Developer experience

Although technically effective, the old pattern was complex and led to errors. To hydrate a component required edits in five different parts of the codebase as well as having to reason through the App.tsx lifecycle methods.

This PR presents a simpler api that removes the need for new developers to learn non standard, complex patterns and also, because we're no longer executing in a React context, we remove the problem of re-renders. The complexity that does still remain is now abstracted behind a developer friendly api.

Only send the data required

By serialising the props for each component alongside its declaration and not in a global window object, we ensure we only send what is required.

Lazy hydration

We now have an api to only hydrate components when visible, deferring content below the fold until scrolled into view. We can also tell the browser to hydrate less important content when it is idle, preventing the thread from being blocked.

Usage

// Server
<Hydrate when="idle">
    <EditionDropdown
        edition={CAPI.editionId}
    />
</Hydrate>

To hydrate a new component on DCR, all developers have to do is wrap it with <Hydrate when="visible"> on the server and then make sure that the component filename follows the naming convention of [MyThing].importable.tsx.

What about global state?

One of the reasons the api offered here is so simple is the lack of any global state. Some hydrated components use shared state managed by App.tsx and for these to migrate to this api they would first need to become more independent. This document talks about this concept more.

What about Portals?

Portals are where no content is server side rendered and we're simply inserting new html into the dom. This use case could follow a very similar pattern to the one presented here. It could even share the same utilities like whenVisible.

@github-actions
Copy link

github-actions bot commented Nov 12, 2021

Size Change: +322 B (0%)

Total Size: 3.15 MB

Filename Size Change
dotcom-rendering/dist/frontend.server.js 2.48 MB +282 B (0%)
dotcom-rendering/dist/react.js 139 kB +18 B (0%)
dotcom-rendering/dist/react.legacy.js 145 kB +22 B (0%)
ℹ️ View Unchanged
Filename Size
dotcom-rendering/dist/101.js 21.1 kB
dotcom-rendering/dist/101.legacy.js 21.1 kB
dotcom-rendering/dist/195.js 1.11 kB
dotcom-rendering/dist/195.legacy.js 1.23 kB
dotcom-rendering/dist/atomIframe.js 1.87 kB
dotcom-rendering/dist/atomIframe.legacy.js 2.13 kB
dotcom-rendering/dist/bootCmp.js 7.5 kB
dotcom-rendering/dist/bootCmp.legacy.js 11.1 kB
dotcom-rendering/dist/braze-web-sdk-core.js 36.1 kB
dotcom-rendering/dist/braze-web-sdk-core.legacy.js 36.1 kB
dotcom-rendering/dist/cmp.js 7.78 kB
dotcom-rendering/dist/coreVitals.js 4.03 kB
dotcom-rendering/dist/coreVitals.legacy.js 4.31 kB
dotcom-rendering/dist/dynamicImport.js 2.99 kB
dotcom-rendering/dist/dynamicImport.legacy.js 3.27 kB
dotcom-rendering/dist/EditionDropdown.js 685 B
dotcom-rendering/dist/EditionDropdown.legacy.js 694 B
dotcom-rendering/dist/elements-CalloutBlockComponent.js 4.15 kB
dotcom-rendering/dist/elements-CalloutBlockComponent.legacy.js 4.46 kB
dotcom-rendering/dist/elements-DocumentBlockComponent.js 571 B
dotcom-rendering/dist/elements-DocumentBlockComponent.legacy.js 602 B
dotcom-rendering/dist/elements-InstagramBlockComponent.js 434 B
dotcom-rendering/dist/elements-InstagramBlockComponent.legacy.js 452 B
dotcom-rendering/dist/elements-InteractiveBlockComponent.js 2.96 kB
dotcom-rendering/dist/elements-InteractiveBlockComponent.legacy.js 3.1 kB
dotcom-rendering/dist/elements-InteractiveContentsBlockComponent.js 1.88 kB
dotcom-rendering/dist/elements-InteractiveContentsBlockComponent.legacy.js 1.95 kB
dotcom-rendering/dist/elements-MapEmbedBlockComponent.js 1.88 kB
dotcom-rendering/dist/elements-MapEmbedBlockComponent.legacy.js 1.94 kB
dotcom-rendering/dist/elements-RichLinkComponent.js 3.26 kB
dotcom-rendering/dist/elements-RichLinkComponent.legacy.js 3.3 kB
dotcom-rendering/dist/elements-SpotifyBlockComponent.js 1.8 kB
dotcom-rendering/dist/elements-SpotifyBlockComponent.legacy.js 1.86 kB
dotcom-rendering/dist/elements-VideoFacebookBlockComponent.js 1.88 kB
dotcom-rendering/dist/elements-VideoFacebookBlockComponent.legacy.js 1.94 kB
dotcom-rendering/dist/elements-VineBlockComponent.js 579 B
dotcom-rendering/dist/elements-VineBlockComponent.legacy.js 594 B
dotcom-rendering/dist/elements-YoutubeBlockComponent.js 2.57 kB
dotcom-rendering/dist/elements-YoutubeBlockComponent.legacy.js 2.7 kB
dotcom-rendering/dist/embedIframe.js 1.88 kB
dotcom-rendering/dist/embedIframe.legacy.js 2.13 kB
dotcom-rendering/dist/ga.js 3.88 kB
dotcom-rendering/dist/ga.legacy.js 4.13 kB
dotcom-rendering/dist/GetMatchStats.js 3.29 kB
dotcom-rendering/dist/GetMatchStats.legacy.js 3.36 kB
dotcom-rendering/dist/guardian-braze-components-banner.js 9.78 kB
dotcom-rendering/dist/guardian-braze-components-banner.legacy.js 9.79 kB
dotcom-rendering/dist/guardian-braze-components-end-of-article.js 6.56 kB
dotcom-rendering/dist/guardian-braze-components-end-of-article.legacy.js 6.57 kB
dotcom-rendering/dist/hydration.js 5.84 kB
dotcom-rendering/dist/hydration.legacy.js 6.46 kB
dotcom-rendering/dist/MostViewedFooterData.js 6.27 kB
dotcom-rendering/dist/MostViewedFooterData.legacy.js 6.36 kB
dotcom-rendering/dist/MostViewedRightWrapper.js 3.89 kB
dotcom-rendering/dist/MostViewedRightWrapper.legacy.js 4.07 kB
dotcom-rendering/dist/newsletterEmbedIframe.js 1.83 kB
dotcom-rendering/dist/newsletterEmbedIframe.legacy.js 2.08 kB
dotcom-rendering/dist/OnwardsLower.js 9.62 kB
dotcom-rendering/dist/OnwardsLower.legacy.js 9.86 kB
dotcom-rendering/dist/OnwardsUpper.js 14 kB
dotcom-rendering/dist/OnwardsUpper.legacy.js 14.3 kB
dotcom-rendering/dist/ophan.js 7.17 kB
dotcom-rendering/dist/ophan.legacy.js 7.36 kB
dotcom-rendering/dist/relativeTime.js 2.41 kB
dotcom-rendering/dist/relativeTime.legacy.js 2.67 kB
dotcom-rendering/dist/sentry.js 677 B
dotcom-rendering/dist/sentry.legacy.js 687 B
dotcom-rendering/dist/sentryLoader.js 4.74 kB
dotcom-rendering/dist/sentryLoader.legacy.js 7.69 kB
dotcom-rendering/dist/shimport.js 2.75 kB
dotcom-rendering/dist/shimport.legacy.js 2.76 kB
dotcom-rendering/dist/SignInGateMain.js 1.82 kB
dotcom-rendering/dist/SignInGateMain.legacy.js 1.85 kB

compressed-size-action

@oliverlloyd oliverlloyd changed the title Hydrate 🏝 React Islands Nov 14, 2021
@oliverlloyd
Copy link
Contributor Author

Co-authored-by: Simon Adcock <simonadcock2@gmail.com>
/**
* getName takes the given html element and returns its name attibute
*
* We expect the element to always be a `gu-*` custom element
Copy link
Contributor

@SiAdcock SiAdcock Nov 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future reference (and for reviewers in a hurry!) what is the role and significance of gu-* custom elements?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're cooler 😉

Maybe they also provide a nice separation from standard elements as a way to highlight these markers as not being part of the semantic dom structure?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay thanks for clarifying that, I agree it flags up the hydratable markup better than a class. I'm wondering if the naming scales well with some possible future in which many of our components are custom elements. Do you think we should choose a name that implies these elements need to be hydrated on the client?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if the naming scales well with some possible future in which many of our components are custom elements

Good point. It's worth pointing out I had in mind using gu-hydrate + gu-portal at some point soon, hence the asterisk

window
.guardianPolyfilledImport(
/* webpackInclude: /\.importable\.(tsx|jsx)$/ */
`../../components/${name}.importable`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd still like to find an improved way to do this step that doesn't depend on a static path to import components

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd love to find a route around this 🙏

*/
export const whenIdle = (callback: () => void) => {
if ('requestIdleCallback' in window) {
window.requestIdleCallback(callback);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. You can pass a timeout option to requestIdleCallback
  2. Does requestIdleCallback get poly-filled as it's not supported by Safari or IE.

https://developer.mozilla.org/en-US/docs/Web/API/Window/requestIdleCallback

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can pass a timeout option to requestIdleCallback

Do we want to timeout? It's a bit hand wavy but isn't waiting for idle the contract here? Adding a timeout feels like we'd be overriding the intention.

Does requestIdleCallback get poly-filled as it's not supported by Safari or IE.

I guess the 'polyfill' here is we fallback to 300 milliseconds. I can investigate if there's an actual polyfill 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to timeout?

The docs recommend a timeout:

A timeout option is strongly recommended for required work, as otherwise it's possible multiple seconds will elapse before the callback is fired.

You're right though, the contract is in the name :)

I just wonder if consumer expectations will be sub-second when it can in theory take an unknown amount of time for the callback to hit the event loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. I think the key is in the word 'required'. We can't know for sure but we probably do want to think of all hydration as required, even if the original intent was to delay it in favour of other, more important, work.

So yes, let's add a timeout. How long do you think?

Copy link
Member

@arelra arelra Nov 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are so many factors (device, network, component interactivity etc) that could influence a suitable value for this.

Keeping it simple and not wanting to block a busy main thread too early:

Going by Speedcurve's upper bound for synthetics (TBT caveats aside) and RAIL... 500ms or thereabouts to start with 🤔 ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's this (gold-plated) pattern too: idle-until-urgent

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for that link, this was a really interesting read! I'd love to spend more time on this to see if there are more places we can use this pattern. It'd be amazing to extend the whenIdle abstraction to return an override and then we could use the same thing for libs like Ophan or CMP where we want to defer initialisation but trigger it if there's a sudden requirement. I guess essentially, you'd be setting up a situation where you're deferring initialising things until the point where they're needed.

Copy link
Member

@arelra arelra Nov 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels partly similar to what React Concurrent mode is attempting (albeit via internals) i.e. interruptible rendering that can be sliced up so that the main thread stays responsive.

Back in our world we have the refactor goals of:

  1. islands style hydration (this PR)
  2. declaratively trigger component render via an async trigger (e.g. onConsentChange)

Separate to hydration can we achieve 2 by using Suspense?

aiui Suspense works by catching throws or a return value from what they call a Resource object.

But we could in theory use any 'event' (Observables even!) to resolve the promise and trigger completion of the enclosing Suspense.

The important bit is the Resource which 'suspends' the component until the underlying promise resolves e.g.:

const suspendUntilConsent = suspendResourceFromPromise(
  onConsentChangePromise()
);
const YouTubePlayer = () => {
  const consentState = suspendUntilConsent.read();
  return (<iframe />)
}

Which can then be declaratively suspended as follows:

<Suspense fallback={<Overlay />}>
  <YouTubePlayer />
</Suspense>

Then it should be possible to write a user friendly abstraction like:

<SuspendUntil fallback={<Overlay />} until={[onConsentChangePromise]}>
  <YouTubePlayer />
</SuspendUntil>

I've created a poc to demonstrate:

https://codesandbox.io/s/zen-tereshkova-c1nsh

The main point of interest is the player is the suspended loader:

https://codesandbox.io/s/zen-tereshkova-c1nsh?file=/src/YouTubePlayer.jsx

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here’s the polyfill, by the way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here’s the polyfill, by the way.

That's a lot of polyfill. Is it worth it? Genuine question.

Polyfill cons:

  • more code
  • extra complexity (risk of issues)

Polyfill pros:

  • The callback will fire sooner if the thread is idle before 300ms
  • This function becomes simpler

@oliverlloyd oliverlloyd merged commit 287c97c into main Dec 16, 2021
@oliverlloyd oliverlloyd deleted the oliver/hydration branch December 16, 2021 13:21
@gtrufitt
Copy link
Contributor

👏

@mxdvl
Copy link
Member

mxdvl commented Mar 25, 2022

Published on the Engineering Blog: https://www.theguardian.com/info/2022/mar/25/react-islands-on-theguardiancom

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants