gatsbyjs / gatsby

The best React-based framework with performance, scalability and security built in.
https://www.gatsbyjs.com
MIT License
55.28k stars 10.31k forks source link

Error with Suspense and lazy loading with gatbsy 4.23.1 and react 18.2 on build #36678

Closed lezan closed 1 year ago

lezan commented 2 years ago

Preliminary Checks

Description

I am unable to use <Suspense> and lazy loading with gatsbyjs 4.23.1 and react 18.2.0. The problem arises with the build and in dev mode with DEV_SSR flag.

The error I get is: Uncaught Error: The server did not finish this Suspense boundary: The server used "renderToString" which does not support Suspense. If you intended for this Suspense boundary to render the fallback content on the server consider throwing an Error somewhere within the Suspense boundary. If you intended to have the server wait for the suspended component please switch to "renderToPipeableStream" which supports Suspense on the server.

Repo 1 code: https://t.co/VPlZUmZ8d0 Repo 1 preview: https://t.co/Rr7NebYwYA Repo 2 code&preview: https://codesandbox.io/s/suspense-error-n0gby5

Link to a thread with @Paulie about that: https://twitter.com/hellovizart/status/1572961555959975936?s=20&t=Ed9sYfMRBN7TSo-50Wz9Mg

Reproduction Link

https://t.co/Rr7NebYwYA

Steps to Reproduce

  1. gatsby dev

or

  1. gatsby build / gatsby serve

Expected Result

Lazy loading work without error

Actual Result

Error: Uncaught Error: The server did not finish this Suspense boundary: The server used "renderToString" which does not support Suspense. If you intended for this Suspense boundary to render the fallback content on the server consider throwing an Error somewhere within the Suspense boundary. If you intended to have the server wait for the suspended component please switch to "renderToPipeableStream" which supports Suspense on the server.

Environment

System:
    OS: Windows 10 10.0.19044
    CPU: (4) x64 Intel(R) Core(TM) i5-4670K CPU @ 3.40GHz    
  Binaries:
    Node: 18.7.0 - E:\Program Files\nodejs\node.EXE
    Yarn: 1.22.10 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 8.4.1 - E:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.19041.1266.0), Chromium (105.0.1343.42)
  npmPackages:
    gatsby: ^4.23.0 => 4.23.0
    gatsby-plugin-canonical-urls: ^4.23.0 => 4.23.0
    gatsby-plugin-image: ^2.23.0 => 2.23.0
    gatsby-plugin-manifest: ^4.23.0 => 4.23.0
    gatsby-plugin-no-sourcemaps: ^4.21.0 => 4.23.0
    gatsby-plugin-offline: ^5.23.0 => 5.23.0
    gatsby-plugin-robots-txt: ^1.7.1 => 1.7.1
    gatsby-plugin-sass: ^5.23.0 => 5.23.0
    gatsby-plugin-sharp: ^4.23.0 => 4.23.0
    gatsby-plugin-sitemap: ^5.23.0 => 5.23.0
    gatsby-source-filesystem: ^4.23.0 => 4.23.0
    gatsby-source-shopify: ^6.10.2 => 6.10.2
    gatsby-transformer-json: ^4.23.0 => 4.23.0
    gatsby-transformer-sharp: ^4.23.0 => 4.23.0
  npmGlobalPackages:
    gatsby-cli: 4.23.1

Config Flags

DEV_SSR: true

lezan commented 2 years ago

Same error with gatsbtjs 4.24.0.

lezan commented 2 years ago

Do not wanna bother anyone, but if someone could point out if I am doing something wrong or it is a bug, it can help me to know what I need to do (migration, downgrading or something else)

lezan commented 2 years ago

I am digging into all github repo using gatsby, react 18 and Suspense. I find theme/ui docs is using Gatsby 4.21, React 18 and in layout.js they are using Suspense without lazy loading. If you check the website here you get:

At this point I think it is a bug.

graysonhicks commented 2 years ago

Hi @lezan the sandbox links above don't work, can you link to the reproduction's code?

graysonhicks commented 2 years ago

Ideally a minimal reproduction:

https://www.gatsbyjs.com/contributing/how-to-make-a-reproducible-test-case/

lezan commented 2 years ago

Hello @graysonhicks the previous codesanbox stopped working for some reason. The new codesanbox is https://codesandbox.io/s/suspense-error-n0gby5. By the way I had already shared another minimal reproduction repo in the first post.

graysonhicks commented 2 years ago

Thanks @lezan I've triaged this now!

lezan commented 2 years ago

Is there an ETA on that? Or maybe a workaround I can try, for instance downgrading gatsby and react. Because I am 18 days beyond the deadline.

graysonhicks commented 2 years ago

Can you try the v5 beta?

struct78 commented 2 years ago

@graysonhicks I've forked @lezan's codesandbox and updated to node 18 & gatsby@next

https://codesandbox.io/s/suspense-error-forked-tl7qp2

lezan commented 2 years ago

@graysonhicks I already tried before (because I am desperate), but still same error. A codesanbox here: https://codesandbox.io/s/suspense-error-v5-beta-y0rh49

graysonhicks commented 2 years ago

Tested the v4 sandbox and am seeing this (slightly different) error:

image

If I remove the DEV_SSR flag, it renders fine, so it must be something with that flag.

lezan commented 2 years ago

@graysonhicks it runs without issues also on build? Because I started adding the flag because I was getting errors on builds.

titanlab commented 2 years ago

@lezan Maybe this helps you... I was having the same problem and tracked down the bug to Static Queries which can't be executed from the Head-Component (I use the new Gatsby Head API). I found out that the Static Query Context with the fetched data is empty while being inside the Head component executing the useStaticQuery hook. So I moved the GraphQL to the page query of the affected page and use the data argument of the Head API instead. This works now and I can develop + build again.

facugra commented 2 years ago

@lezan did you find a workaround? @graysonhicks DEV_SSR makes gatsby develop work as an SSR app too, disabling it will just delay these errors until you run gatsby build or use the resulting compiled code.

lezan commented 2 years ago

@lezan Maybe this helps you... I was having the same problem and tracked down the bug to Static Queries which can't be executed from the Head-Component (I use the new Gatsby Head API). I found out that the Static Query Context with the fetched data is empty while being inside the Head component executing the useStaticQuery hook. So I moved the GraphQL to the page query of the affected page and use the data argument of the Head API instead. This works now and I can develop + build again.

I am gona try that, but if you check the codesanbox there is not Head, so I do not think it could be this component.

@lezan did you find a workaround? @graysonhicks DEV_SSR makes gatsby develop work as an SSR app too, disabling it will just delay these errors until you run gatsby build or use the resulting compiled code.

Nope, still give the same error. I want try soon the v5.

struct78 commented 2 years ago

@lezan Maybe this helps you... I was having the same problem and tracked down the bug to Static Queries which can't be executed from the Head-Component (I use the new Gatsby Head API). I found out that the Static Query Context with the fetched data is empty while being inside the Head component executing the useStaticQuery hook. So I moved the GraphQL to the page query of the affected page and use the data argument of the Head API instead. This works now and I can develop + build again.

This isn't relevant.

The minimal reproduction makes it clear that this happens with even the most basic Gatsby site.

Orainsink commented 1 year ago

I'm facing the same problem for my blog with react 18.2 and gatsby 5.2. Still waiting for a solution.

zamson commented 1 year ago

Any updates?

d4v1d82 commented 1 year ago

about the DEV_SSR issue, It seems that the issue is here: https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby/cache-dir/ssr-develop-static-entry.js#L300 ssr-develop-static-entry.js is using renderToString instead of renderToPipeableStream

I've tried to path my local Gatsby code with

diff --git a/node_modules/gatsby/cache-dir/ssr-develop-static-entry.js b/node_modules/gatsby/cache-dir/ssr-develop-static-entry.js
index 3b472ac..136c498 100644
--- a/node_modules/gatsby/cache-dir/ssr-develop-static-entry.js
+++ b/node_modules/gatsby/cache-dir/ssr-develop-static-entry.js
@@ -1,7 +1,7 @@
 /* global BROWSER_ESM_ONLY */
 import React from "react"
 import fs from "fs-extra"
-import { renderToString, renderToStaticMarkup } from "react-dom/server"
+import { renderToString, renderToStaticMarkup, renderToPipeableStream } from "react-dom/server"
 import { get, merge, isObject, flatten, uniqBy, concat } from "lodash"
 import nodePath from "path"
 import { apiRunner, apiRunnerAsync } from "./api-runner-ssr"
@@ -12,6 +12,7 @@ import { RouteAnnouncerProps } from "./route-announcer-props"
 import { ServerLocation, Router, isRedirect } from "@gatsbyjs/reach-router"
 import { headHandlerForSSR } from "./head/head-export-handler-for-ssr"
 import { getStaticQueryResults } from "./loader"
+import { WritableAsPromise } from "./server-utils/writable-as-promise"

 // prefer default export if available
 const preferDefault = m => (m && m.default) || m
@@ -297,8 +298,22 @@ export default async function staticPage({
     // If no one stepped up, we'll handle it.
     if (!bodyHtml) {
       try {
-        bodyHtml = renderToString(bodyComponent)
+        //bodyHtml = renderToString(bodyComponent)
+
+        const writableStream = new WritableAsPromise()
+         const { pipe } = renderToPipeableStream(bodyComponent, {
+           onAllReady() {
+             pipe(writableStream)
+           },
+           onError(error) {
+             writableStream.destroy(error)
+           },
+         })
+
+         bodyHtml = await writableStream;
+
       } catch (e) {
+        console.error("SSR Error", e);
         // ignore @reach/router redirect errors
         if (!isRedirect(e)) throw e
       }

but I'm not sure if is correct or there are other places that needs to be updated and it seems that the hot reload is not working correctly.

panzacoder commented 1 year ago

@graysonhicks I see you gave a 👍 to the above suggested solution. Does this mean the Gatsby team recognizes this rather large bug and are working on it?

With DEV_SSR not working with React 18 and Suspense, Gatsby doesn't actually fully support React 18... which is required for using Gatsby 5.

This seems like a priority bug at the least.

Not sure if the Gatsby team is aware of this but:

amandasaffer commented 1 year ago

@panzacoder Heavily agree. I am able to reproduce this issue by forking the Shopify starter using Gatsby 5 and adding a Suspense component. I will link it here when I have a bit of time to set up the repo. We're not able to use Suspense OR loadable components at this point... therefore can't upgrade to Gatsby 5.

lezan commented 1 year ago

@panzacoder Heavily agree. I am able to reproduce this issue by forking the Shopify starter using Gatsby 5 and adding a Suspense component. I will link it here when I have a bit of time to set up the repo. We're not able to use Suspense OR loadable components at this point... therefore can't upgrade to Gatsby 5.

We already provided a reproducible test case (three or more) with only plugins that are needed to demonstrate the problem (actually no one, just gatsby and react packages are included). No need for a more "complicated" test case I think, do not waste your time.

amandasaffer commented 1 year ago

@panzacoder Heavily agree. I am able to reproduce this issue by forking the Shopify starter using Gatsby 5 and adding a Suspense component. I will link it here when I have a bit of time to set up the repo. We're not able to use Suspense OR loadable components at this point... therefore can't upgrade to Gatsby 5.

We already provided a reproducible test case (three or more) with only plugins that are needed to demonstrate the problem (actually no one, just gatsby and react packages are included). No need for a more "complicated" test case I think, do not waste your time.

Ah, ok. I missed that you provided this. Thank you!

panzacoder commented 1 year ago

@amandasaffer @graysonhicks I've opened a PR based on @d4v1d82 's comment above. This is working for me locally for dev ssr. If you are able to try this out in your project, please feel free to leave feedback.

LekoArts commented 1 year ago

https://github.com/gatsbyjs/gatsby/pull/37534: You can try it out with gatsby@next and it'll be released as stable in gatsby@5.6.0

panzacoder commented 1 year ago

but I'm not sure if is correct or there are other places that needs to be updated and it seems that the hot reload is not working correctly.

@d4v1d82 I was not seeing the HMR issues you reported, would be interested to see if gatsby@next solves this issue for you or not.

lezan commented 1 year ago

For me the error it is still here also with a basic repo with gatsby@next. Hot reloading is an issue now, as reported by @d4v1d82.

Getting Expected server HTML to contain a matching <button> in <div>. Error: Hydration failed because the initial UI does not match what was rendered on the server. Error: There was an error while hydrating this Suspense boundary. Switched to client rendering.

if I change something from

<header>
  <div>
    Test
  </div>
</header>

to

<header>
  <div>
    <button type="button">
      Test
    </button>
  </div>
</header>

If I restart the server with the new changes, I am not getting issue.

Not able to create a codesanbox with Node v18. Suggestions? For the moment I create a repo here: https://github.com/lezan/gatsby-suspense-5.6-0-next

d4v1d82 commented 1 year ago

@panzacoder @lezan I still have some issues with the HMR, but I can't figure out some deterministic behaviors: sometimes I have to restart the server, sometimes it works. It seems that the issue is when you have a Suspence and lazy loaded component that is not hot reloaded correctly. I'm not working on the Gatsby project right now but I'll try to make some specific tests to understand if there are any patterns when HMR is not working

tsdexter commented 1 year ago

37534: You can try it out with gatsby@next and it'll be released as stable in gatsby@5.6.0

ETA on 5.6.0 release date? @LekoArts

MRRaduan commented 1 year ago

Same issue here described by @d4v1d82 and @lezan with the hot reloading after updated Gatbsy to 5.6 or 5.7 released in February.

The behavior that I'm facing is like:

  1. In the first load the page runs correctly without any errors.
  2. After the page been refreshed manually OR as consequence of hot reloading, the page shows this error message:
Screenshot 2023-02-28 at 08 52 42
donaldboulton commented 1 year ago

Try this.

Suspense Helper

I works great in my gatsby 5.12.4 just be sure to add react like below. Tutorial is missing Suspense call in react as `import * as React from 'react'

import { ReactNode, useEffect, useState, Suspense } from 'react

type Props = { fallback?: ReactNode, children: ReactNode }

export const SuspenseHelper: React.FC<Props> = ({fallback, children}) => {

`const [isMounted, setMounted] = useState<boolean>(false);

useEffect( () => {
    if(!isMounted)
    {
        setMounted(true);
    }
})

return (
    <Suspense fallback={fallback}>
        {!isMounted ? fallback : children}
    </Suspense>
)

};`

then wrap your component as so

const Header = React.lazy(() => import ('@/components/Header'));

`<SuspenseHelper fallback={

Loading...
}>

`

Mario8419 commented 1 year ago

Try this.

Suspense Helper

I works great in my gatsby 5.12.4 just be sure to add react like below. Tutorial is missing Suspense call in react as `import * as React from 'react'

import { ReactNode, useEffect, useState, Suspense } from 'react

type Props = { fallback?: ReactNode, children: ReactNode }

export const SuspenseHelper: React.FC<Props> = ({fallback, children}) => {

`const [isMounted, setMounted] = useState<boolean>(false);

useEffect( () => {
    if(!isMounted)
    {
        setMounted(true);
    }
})

return (
    <Suspense fallback={fallback}>
        {!isMounted ? fallback : children}
    </Suspense>
)

};`

then wrap your component as so

const Header = React.lazy(() => import ('@/components/Header'));

<SuspenseHelper fallback={<div>Loading...</div>}> <Header /> </SuspenseHelper>

With this solution I got a Minified React error #421

adham618 commented 12 months ago

@Mario8419 did you find any solution?

Mario8419 commented 12 months ago

no, but didn't tested until now with a newer gatsby version, maybe?

adham618 commented 12 months ago

@Mario8419 No, still the same the problem with me is a hydration error when rendering the content in mdxprovider and i can't fix it

adham618 commented 11 months ago

If anyone is facing this issue, Gatsby has a beta feature called partial hydration, somewhat like Next.js. To implement it, add "use client" at the beginning of each component, excluding pages or templates. Also, don't forget to remove hooks from pages or templates. Now you can use react-lazy and suspense without throwing errors. If it does throw an error, you can fix it by disabling SSR as mentioned in the above comments. However, in the docs, it says to enable the partial hydration flag in the config file. If you enable it, the build fails, so don't enable it in newer versions. I don't know why, but this works without enabling the flag.

Mario8419 commented 11 months ago

If anyone is facing this issue, Gatsby has a beta feature called partial hydration, somewhat like Next.js. To implement it, add "use client" at the beginning of each component, excluding pages or templates. Also, don't forget to remove hooks from pages or templates. Now you can use react-lazy and suspense without throwing errors. If it does throw an error, you can fix it by disabling SSR as mentioned in the above comments. However, in the docs, it says to enable the partial hydration flag in the config file. If you enable it, the build fails, so don't enable it in newer versions. I don't know why, but this works without enabling the flag.

any example code?