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

feat(gatsby): enable async rendering with react 18 #32188

Merged
merged 14 commits into from
Jul 2, 2021

Conversation

wardpeet
Copy link
Contributor

@wardpeet wardpeet commented Jul 1, 2021

Description

Enable Async rendering like Suspense during SSR. pipeToNodeWritable is a stream rendering that support Suspense boundaries and works with the new createRoot/hydrateRoot APIs.

reactwg/react-18#22

After ssr:
image

During client side navigation:
image

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Jul 1, 2021
@wardpeet wardpeet changed the base branch from master to feat/async-browser July 1, 2021 12:01
@wardpeet wardpeet added topic: frontend Relates to frontend issues (e.g. reach/router, gatsby-link, page-loading, asset-loading, navigation) and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Jul 1, 2021
@wardpeet wardpeet force-pushed the feat/react-async-rendering branch from d7cb34c to e744840 Compare July 1, 2021 12:03
@@ -234,7 +234,7 @@ const renderHTMLQueue = async (
payload: pageSegment,
})

for (const [_pagePath, arrayOfUsages] of Object.entries(
for (const [, arrayOfUsages] of Object.entries(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fix eslint warning

@wardpeet wardpeet force-pushed the feat/react-async-rendering branch from 8faee2f to 7f84656 Compare July 1, 2021 12:15
Copy link
Contributor

@vladar vladar left a comment

Choose a reason for hiding this comment

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

Just to clarify - this doesn't use streaming per se for SSR, right? We aggregate changes and resolve full HTML as before. So is it just about working around the deprecation of renderToString?

onCompleteAll() {
startWriting()
},
onError() {},
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to ignore errors here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we won't do streaming yet until libs have adopted and maybe we never do.
React still throws the error so this is just to don't show the warning.

vladar
vladar previously approved these changes Jul 1, 2021
@wardpeet wardpeet force-pushed the feat/react-async-rendering branch from 5211f56 to 8b8623e Compare July 1, 2021 18:00
@wardpeet wardpeet force-pushed the feat/react-async-rendering branch from 8b8623e to b8f9937 Compare July 2, 2021 07:23
@wardpeet wardpeet force-pushed the feat/react-async-rendering branch from b8f9937 to cc09599 Compare July 2, 2021 12:39
@wardpeet wardpeet closed this Jul 2, 2021
@wardpeet wardpeet deleted the branch gatsbyjs:master July 2, 2021 13:41
@wardpeet wardpeet reopened this Jul 2, 2021
@wardpeet wardpeet changed the base branch from feat/async-browser to master July 2, 2021 13:42
@wardpeet wardpeet dismissed vladar’s stale review July 2, 2021 13:42

The base branch was changed.

@wardpeet wardpeet added the bot: merge on green Gatsbot will merge these PRs automatically when all tests passes label Jul 2, 2021
@gatsbybot gatsbybot merged commit 32e9b4a into gatsbyjs:master Jul 2, 2021
vladar pushed a commit that referenced this pull request Jul 6, 2021
* feat(gatsby): enable replaceRenderer to be async

* update tests

* duplicated apiRunner for better results

* enable async in everything

* SAVEPOINT

* fix error parsgin

* remove console.log

* fix tests

* add sync test

* enable async rendering

* remove debug info

* fix async

* remove debug

(cherry picked from commit 32e9b4a)
vladar added a commit that referenced this pull request Jul 6, 2021
* feat(gatsby): enable replaceRenderer to be async

* update tests

* duplicated apiRunner for better results

* enable async in everything

* SAVEPOINT

* fix error parsgin

* remove console.log

* fix tests

* add sync test

* enable async rendering

* remove debug info

* fix async

* remove debug

(cherry picked from commit 32e9b4a)

Co-authored-by: Ward Peeters <ward@coding-tech.com>
Co-authored-by: Vladimir Razuvaev <vladimir.razuvaev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: merge on green Gatsbot will merge these PRs automatically when all tests passes topic: frontend Relates to frontend issues (e.g. reach/router, gatsby-link, page-loading, asset-loading, navigation)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants