swr icon indicating copy to clipboard operation
swr copied to clipboard

Suspense and Server-Side Rendering

Open pkellner opened this issue 3 years ago • 36 comments

Bug report

Description / Observed Behavior

React 18 and SWR fails on the simplest of examples. I've created a sandbox at this URL: https://codesandbox.io/s/long-cdn-qxkc3v?file=/pages/indexSuspense.js that if your browse (to that page), you'll get the error:

error: The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.

Expected Behavior

I expect to see a page rendered just like at the default root which is in the file /pages/index.js

Repro Steps / Code Example

Run the code sandbox, browse to http://.../indexSuspense and you will get the error I observe

Additional Context

Below is the code from code sandbox along with the associated package.json

import { Suspense } from "react";
import useSWR from "swr";

const fetcher = (url) => fetch(url).then((r) => r.json());

function Profile() {
  const { data } = useSWR("/api/cities", fetcher, {
    suspense: true
  });
  return <pre>{JSON.stringify(data, null, 2)}</pre>;
}

export default function Example() {
  return (
    <Suspense fallback={<div>Loading...</div>}>
      <Profile />
    </Suspense>
  );
}

package.json

{
  "name": "example-react-suspense",
  "version": "0.1.0",
  "private": true,
  "scripts": {
    "build": "next build",
    "dev": "next dev",
    "start": "next start"
  },
  "dependencies": {
    "next": "12.1.4",
    "react": "^18.0.0",
    "react-dom": "^18.0.0",
    "swr": "^1.2.2"
  }
}

I've also been testing with the latest canary from next and the latest pre-release from react 18.1 and getting the same errors.

erro1

pkellner avatar Apr 06 '22 01:04 pkellner

@pkellner I just checked your example, and it turns out that since React 18 now supports Suspense on the server side, that fetch('/api/cities') will happen on the server. So I saw this error:

TypeError: Only absolute URLs are supported at getNodeRequestOptions ...

which is the root cause. You can't fetch a relative URL (/api/cities) on the server side.

In fact, if you change it to an absolute API like https://api.github.com/repos/vercel/swr it will SSR correctly.

shuding avatar Apr 11 '22 00:04 shuding

I will keep this issue open until we add more clarification and guidance on Suspense and SSR to the documentation. Let me know if there's any suggestion on this!

shuding avatar Apr 11 '22 00:04 shuding

Thanks @shuding for the help here. I'm not sure whether I should continue this issue, as it's related to instability, or start another one, as the problem has morphed but it I think it's still SWR and Suspense instability, as this example I'm about to describe works correctly when using with create-react-app (please let me know and I'll be happy to re-create this as a new issue).

Here is the new problem. I've slightly modified the example shown above to include a simple loop that renders a button with a click event.

https://github.com/pkellner/swr_issue_1906_suspense and associated codesandbox

Actual Behavior

With the <Profile /> component included (that has the suspense boundary and useSwr call), when clicking on any of the three buttons the first time, the click event does not get fired. However, on the 2nd or 3rd click it does fire.

With the <Profile /> component commented out (removed), the button click event fires and you see the console.log message immediately on the first time clicking any row.

Expected Behavior

I expect that when clicking on a button the first time, it will output the console message in the click event whether or not a Suspense element is included in the render.

Aside Note

I've included a slightly more complex source code example here that I'm tracking with an issue on the React repository that has nothing to do with this issue, but points out the complexity of using Suspense compared to not using Suspense. I did the example using create-react-app and not nextjs because of this issue (which just makes the example over there confusing to run). That is, I've brought that example back to this repo running in NextJS. You can browse to that code running with the absorbing click event here by browsing to the URL: http://localhost:3000/indexApp

Associated Relevant Code

import { Suspense } from "react";
import useSWR from "swr";

const fetcher = (url) => fetch(url).then((r) => r.json());

function Profile() {
  //const url = "/api/cities";
  const url = "https://airquality.peterkellner.net/api/city";
  const { data } = useSWR(url, fetcher, {
    suspense: true,
  });
  return <pre>{JSON.stringify(data, null, 2)}</pre>;
}

/*
If comment out call to <Profile /> the console.log on the click event happens correctly.  That is, on the first click.
  If you do not comment out <Profile /> as is shown here, the first time you cick on any of the three rows, the click is not executed until
  the second or third time you click a button.
 */

export default function Example() {
  return (
    <Suspense fallback={<div>Loading...</div>}>
      {[1, 2, 3].map((counter) => {
        return (
          <button key={counter}
            onClick={() => {
              console.log(`click Me clicked ${counter}`);
            }}
          >
            Click Me {counter}
          </button>
        );
      })}
      <Profile />
    </Suspense>
  );
}

pkellner avatar Apr 11 '22 14:04 pkellner

It seems a bit confusing to me that useSWR runs on the server at all, given that it's not designed to transfer data to the client. So there is no way for it to hydrate properly. Would it be better to disable useSWR on the server altogether and make it throw until there is some strategy?

gaearon avatar Apr 11 '22 16:04 gaearon

@gaearon yeah, that was one concern I had here.

My feeling is that most suspense-like libraries probably don't properly support SSR, given as how until React 18 it was not possible to run them on the server at all. It might be nice if e.g. updating these libraries to throw during SSR was a more official recommendation?

devknoll avatar Apr 11 '22 17:04 devknoll

Yep @gaearon @devknoll this is what we are going to do, there is also #1832 and #1841.

Our plan is to:

  • 1.x which is the current version: show a warning if you are using SWR and suspense: true on the server side.
  • 2.x which will come in a couple of weeks: change the warning above to an error as its breaking.

The tricky thing is, frameworks do pre-rendering to improve SEO, etc. SWR doesn’t want to run on the server, but unlike fetching data inside an effect, there’s no easy way to skip that with Suspense on the server side, without causing an error.

shuding avatar Apr 12 '22 00:04 shuding

most suspense-like libraries probably don't properly support SSR

I see this as a general problem of “Suspense for data fetching in SSR” though.

But still, if the data is not going to cause a mismatch (e.g. fully static, or “almost” static as hydration happens very soon after SSR, usually), Suspense in SSR still makes sense.

shuding avatar Apr 12 '22 00:04 shuding

My understanding from a convo with @sebmarkbage is that a proper solution for useSWR is to take “initial data” argument. That’s taken from getServerSideProps on the server. Which means it also becomes available on the client for hydration. The initial fetch happens by calling the actual implementation (not /api/ route) from the server itself. So there’s also no “absolute URL” problem because there’s no fetch call on the server at all. Then useSWR on the client hydrates with that initial data. And for future navigations on the client useSWR does actual requests.

Then you only need to throw if you’re on the server and data wasn’t preloaded.

gaearon avatar Apr 12 '22 00:04 gaearon

@gaearon See “Fallback Data” https://swr.vercel.app/blog/swr-v1#fallback-data

“Pre-Rendering with Default Data” https://swr.vercel.app/docs/with-nextjs#pre-rendering-with-default-data

stefee avatar Apr 12 '22 07:04 stefee

@shuding , do you have any insight now into a path forward to supporting Suspense with SWR? Above you mentioned I believe, that you plan on disabling the suspense: true option shortly. Is that still your thinking?

I'm also wondering if whether using CRA vs NextJS has any impact on this. I'm assuming that Suspense integration with CRA is just as broken on CRA as it is on Next but I'm not 100% clear on that. If your implementation does work correctly for CRA with Suspense enabled, I'd love to know that for sure. I understand from @gaearon that CRA integration might not work either.

Thanks for paying attention to this. I can't say much more publicly, but I have managed to delay my course work on React 18 with Suspense until this is sorted.

pkellner avatar Apr 14 '22 18:04 pkellner

Suspense integration with CRA is just as broken on CRA as it is on Next

To be clear, there is nothing broken in Suspense integration with Next. What's broken here is the useSWR helper and how it implements Suspense. Suspense is only ready for usage in data frameworks (like Relay), not in ad-hoc helpers like useSWR. So the useSWR story is not fleshed out, and that's where the issue is from.

The issue is specific to server rendering, so it won't appear in CRA. However, still, we don't recommend using Suspense implementations in ad-hoc data libraries like useSWR in stable code at the moment. Suspense-powered opinionated frameworks are OK.

gaearon avatar Apr 14 '22 18:04 gaearon

I know asking timeframe type questions is fraught with peril, but does this feel like something that will get sorted in weeks, months, or many months? (afraid to say years).

Also, besides the FakeAPI example which is clearly meant NOT for production, are the any cases where Suspense could currently be used in production (that is not using Relay specifically).

pkellner avatar Apr 14 '22 18:04 pkellner

It seems like https://swr.vercel.app/docs/with-nextjs#pre-rendering-with-default-data gives you a solution that should work.

gaearon avatar Apr 14 '22 18:04 gaearon

I realize I posted this on the wrong issue so I've reposted it here:

https://github.com/facebook/react/issues/23045#issuecomment-1101858756

pkellner avatar Apr 18 '22 22:04 pkellner

It seems a bit confusing to me that useSWR runs on the server at all, given that it's not designed to transfer data to the client. So there is no way for it to hydrate properly. Would it be better to disable useSWR on the server altogether and make it throw until there is some strategy?

@shuding - Wasn't a potential strategy the extend cache provider feature?

We've been using this without issue in our deno/react framework, we populate the cache server side, which is used to hydrate client side. The standard SWR options can be configured to prevent fetch calls on mount, if this is the desired outcome.

It seems most of the comments are in the 'it works, but not exactly how we want' way, instead of throwing an error, couldn't this be something that is opt-in and documented a bit more clearly?

mashaal avatar May 14 '22 03:05 mashaal

Has there been any progress on getting SWR to work reliably with Suspense?

pkellner avatar May 15 '22 13:05 pkellner

@gaearon , in another issue on the Apollo repo, another user is claiming that Suspense with react-query is stable in production if using just client side rendering. Could you clarify if that is true please.

https://github.com/apollographql/apollo-client/issues/9627#issuecomment-1133436600

pkellner avatar May 20 '22 22:05 pkellner

The only Suspense implementation we've vetted so far is Relay. There is ongoing work on the Next.js side to add support for Suspense Data Fetching, but it will be through React Server Components and might not support refetching in the first iteration (but later will). Any broader support is not official at this moment, and you can expect additions/changes in the low-level APIs as we figure this out. So while today's implementations might "work", we can't vouch for them, and there might be some API and/or implementation changes necessary in them in the future.

gaearon avatar May 23 '22 21:05 gaearon

Hi @shuding , I notice you are working on caching in NextJS with the latest canary. Will this work go towards useSwr working with Suspense correctly? https://github.com/vercel/next.js/pull/37258/files/7401927ee29a53f3203da3b8fd8d2e750a86ea8f

pkellner avatar May 29 '22 21:05 pkellner

Hi @shuding and @gaearon In the latest React blog post and the 18.2 release, I don't see any mention of work around Suspense and Data access regarding non relay libraries. Did I miss something? Can you provide an update on status, in particular SWR and Suspense? I remember there was talk about removing the Suspense option from useSWR because of the confusion it creates (that is, it is not recommended for production according to Dan and the React team docs).

Thanks

https://reactjs.org/blog/2022/06/15/react-labs-what-we-have-been-working-on-june-2022.html

pkellner avatar Jun 18 '22 14:06 pkellner

Hey @pkellner! From the SWR side there isn't much I can do – in #1931 we already made SWR behaving correctly with Suspense and SSR. I guess the next step is to have React release something similar to this for libraries to adopt, but I'm unsure.

shuding avatar Jul 04 '22 22:07 shuding

Hi @gaearon, another use case for data-fetching with suspense on the server, are zero-KB JavaScript environments like Astro or Capri. Unlike Next.js, Capri does not have its own router and can't fetch the data upfront, outside of React. I guess what I'd really want for SSG would be an async version of renderToStaticMarkup that lets components fetch their own data. Until now, using something like SWR with suspense was the only way to achieve this. Can you think of a future-proof way to keep on supporting this pattern?

fgnass avatar Jul 13 '22 07:07 fgnass

related https://github.com/reactjs/rfcs/pull/229

pkellner avatar Oct 15 '22 23:10 pkellner

I must be doing something wrong, but when I provide (static) fallback data, my component never suspends and show my fallback loader

ricksmit3000 avatar May 07 '23 21:05 ricksmit3000

@rickiesmooth can you provide a simple codesandbox type example?

pkellner avatar May 07 '23 22:05 pkellner

@pkellner sure thing! I made a simple codesandbox example here: https://codesandbox.io/p/sandbox/suspicious-chaum-u68vrm?file=%2Fapp%2Fpage.tsx&selection=%5B%7B%22endColumn%22%3A22%2C%22endLineNumber%22%3A12%2C%22startColumn%22%3A22%2C%22startLineNumber%22%3A12%7D%5D

Notice you never see "Loading..."

I guess it's because by providing the fallbackData there's no pending promise initially, but also providing an unresolved promise as fallback data doesn't trigger the suspense fallback.

ricksmit3000 avatar May 08 '23 08:05 ricksmit3000

Hi @rickiesmooth Thanks for the example. You are trying to use Suspense on the client side for data and unfortunately, that scenario is not supported by React at the moment. The only real support for suspense and data isn with NextJS 13.4 with server components. You can see a big warning at this URL: https://swr.vercel.app/docs/suspense

pkellner avatar May 08 '23 12:05 pkellner

@rickiesmooth I encountered the same scenario when my team began using the beta App router. The beta App router docs suggested using SWR as an alternative to React's use hook for fetching client-side data. It didn't explicitly call out suspense as a supported option, but it didn't have verbiage that advised against its usage—that was only listed on SWR's docs as @pkellner pointed out.

The interesting thing was that if you simply don't provide the fallbackData, suspense appears to work as expected aside from throwing the error about the missing fallbackData. If I remember correctly, the production app would still work since the error was simply logged to the console and not visible to the user. I'm not sure if this is still the case.

I'm unfamiliar with the internals of React with respect to client-side data fetching and suspense, so I'm sure I'm unaware of pitfalls, bugs, and general misusage, but it would be nice to have some sort of confirmation that this truly would cause bugs in production. Otherwise it could be better to let developers try ignoring the fallbackData error when suspense is turned on. Even if it was as simple as...

const swrConfig = {
  suspense: true,
  unstable_noFallbackDataError: true,
}

dbk91 avatar May 08 '23 13:05 dbk91

@dbk91 thanks for sharing your experience!

The interesting thing was that if you simply don't provide the fallbackData, suspense appears to work as expected aside from throwing the error about the missing fallbackData. If I remember correctly, the production app would still work since the error was simply logged to the console and not visible to the user. I'm not sure if this is still the case.

I think this kinda worked, but it would switch to client side rendering? However, when I now build my small codesandbox example it throws the following error:

[    ] info  - Generating static pages (0/3)Error: Fallback data is required when using suspense in SSR.

so it doesn't appear to still be the case.

ricksmit3000 avatar May 08 '23 13:05 ricksmit3000

@rickiesmooth Oh right, did forget one caveat. Any component using suspense + SWR would need to opt out of build-time optimization. Potentially a big downside, but the app directory for Next makes this a bit easier to control if the component are organized correctly. I think if you use useSearchParams wherever you're using SWR + suspense, and more importantly before the useSWR hook, it'll opt out of SSG and the build will work. However, that was for versions < 13.4. I'd have to verify that's still the case.

dbk91 avatar May 08 '23 13:05 dbk91