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

[Bug]: Infinite re-render for async RSC stories #30317

Open
dmitrc opened this issue Jan 21, 2025 · 12 comments
Open

[Bug]: Infinite re-render for async RSC stories #30317

dmitrc opened this issue Jan 21, 2025 · 12 comments

Comments

@dmitrc
Copy link

dmitrc commented Jan 21, 2025

Describe the bug

There was always some funkiness around using await in RSC story renders in Storybook (eg see amannn/next-intl#771 (comment)), but in the version 8.5.0+ the behavior has changed for the worse.

Now if I have a server component with await in it, some recent change on the Storybook side seems to be causing an infinite render. It's a bit tricky to point out what factor exactly causes this, as in my isolated repro even a single await is enough to trigger it, whereas in my actual codebase it only seems to happen when there are 2+.

Image

Reproduction link

https://github.com/dmitrc/storybook-repro

Reproduction steps

git clone https://github.com/dmitrc/storybook-repro
cd storybook-repro
pnpm install
pnpm dev # verify that the components render
pnpm storybook # open the console to see the infinite render loop

Or setup from scratch:

pnpm dlx create-next-app@latest
pnpm dlx storybook@latest init

.storybook/main.ts:

  features: {
    experimentalRSC: true,
    developmentModeForBuild: true,
  },

page.tsx:

export default function Home() {
  return (
    <div>
      <ServerComponentWithOneAwait />
      <ServerComponentWithManyAwaits />
    </div>
  );
}

export async function ServerComponentWithOneAwait() {
  console.log(`ServerComponentWithOneAwait`);
  const md1 = await getMockData(1);
  return <p>{md1}</p>
}

export async function ServerComponentWithManyAwaits() {
  console.log(`ServerComponentWithManyAwaits`);
  const md1 = await getMockData(1);
  const md2 = await getMockData(2);
  return <p>{md1} + {md2}</p>
}

export function getMockData(i: number): Promise<string> {
  return new Promise((resolve) => setTimeout(() => resolve(`mock data ${i}`), 1000));
}

ServerComponentWithOneAwait.stories.tsx:

import type { Meta, StoryObj } from "@storybook/react";
import { ServerComponentWithOneAwait } from "../app/page";

const meta: Meta<typeof ServerComponentWithOneAwait> = {
  title: "RSC/OneAwait",
  component: ServerComponentWithOneAwait,
};

export default meta;
type Story = StoryObj<typeof meta>;

export const Default: Story = {};

System

Storybook Environment Info:

  System:
    OS: Linux 5.15 Ubuntu 24.04.1 LTS 24.04.1 LTS (Noble Numbat)
    CPU: (32) x64 AMD Ryzen Threadripper PRO 3955WX 16-Cores
    Shell: 5.9 - /usr/bin/zsh
  Binaries:
    Node: 22.12.0 - ~/.nvm/versions/node/v22.12.0/bin/node
    npm: 10.9.0 - ~/.nvm/versions/node/v22.12.0/bin/npm
    pnpm: 9.15.1 - ~/.nvm/versions/node/v22.12.0/bin/pnpm <----- active
  npmPackages:
    @storybook/nextjs: ^8.5.0 => 8.5.0
    @storybook/react: ^8.5.0 => 8.5.0
    eslint-plugin-storybook: ^0.11.2 => 0.11.2
    storybook: ^8.5.0 => 8.5.0

Additional context

No response

Copy link
Contributor

greptile-apps bot commented Jan 21, 2025

** Disclaimer** This information might be inaccurate, due to it being generated automatically
This appears to be related to the RSC rendering implementation in @storybook/nextjs. The issue likely stems from the RSC rendering logic in @storybook/nextjs/dist/preview.js where React Server Components are handled. Key steps to fix: 1. Modify the RSC rendering wrapper in @storybook/nextjs to prevent re-renders by: - Adding a cache or memoization layer for async RSC results - Implementing proper boundary conditions for async component resolution 2. The specific implementation would need to be in the RSC experimental feature code, likely in: js // @storybook/nextjs/src/experimental-react-server // Add caching/memoization for async RSC results const cachedRSCResult = new WeakMap(); if (cachedRSCResult.has(Component)) { return cachedRSCResult.get(Component); } This appears to be a regression introduced in 8.5.0+ related to how RSC async boundaries are handled.

About Greptile

This response provides a starting point for your research, not a precise solution.

Help us improve! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

Edit Issue Bot Settings · Greptile

@dmitrc
Copy link
Author

dmitrc commented Jan 21, 2025

Upon further testing, it seems to be happening on older versions (~8.4.7) as well for this specific minimal repro 🤔

For the best context on my original concern, the way I discovered it is by experimenting with the issue outlined here on the newest release of Storybook: amannn/next-intl#771 (reply in thread).

  • In both versions, using await getTranslations() as the very first statement works
  • In 8.4.7, attempting to use await getTranslations() as the second await fails with "context not found"
  • In 8.5.0, attempting to use await getTranslations() as the second await fails with the infinite re-render

In this specific example, both await methods are actually synchronous (one is being mocked to return a data object directly, the other one is being mocked to call a client-side hook instead of server-only method).

Hence me opening this issue. There is probably a lot more underground currents here re: RSC/Suspense/etc that I am not seeing, so just trying to provide as much info as possible to help isolate and troubleshoot here.

@valentinpalkovic
Copy link
Contributor

Hi @dmitrc,

Thanks for posting the issue. I read through the next-intl thread as well, and the behavior is indeed pretty strange. I try to take a look the next days.

@dmitrc
Copy link
Author

dmitrc commented Jan 21, 2025

Hi @dmitrc,

Thanks for posting the issue. I read through the next-intl thread as well, and the behavior is indeed pretty strange. I try to take a look the next days.

Thanks @valentinpalkovic,
I have now updated the linked repro to include the whole next-intl setup for your convenience.

  • LocalizedComponentTranslationsFirst renders fine
  • LocalizedComponentTranslationsLater throws context error on 8.4.x and infinite re-renders on 8.5.x

Also added some other components to play around with, including async no await and fake await (actually sync).

Please let me know if I can clarify anything further or help in any way!
Thanks again for looking into it!

LocalizedComponentTranslationsFirst LocalizedComponentTranslationsLater
8.5.0 Image Image
8.4.7 Image Image

@davidcarr-au
Copy link

@dmitrc @valentinpalkovic

Looks like the promise now needs to be cached otherwise each render of the rsc/action in storybook returns a new promise object causing the infinite render loop.

Image Image

@boylec
Copy link

boylec commented Jan 22, 2025

Likely a duplicate of #30033

@boylec
Copy link

boylec commented Jan 23, 2025

See my comment there on #30033

Here is the diff between next.js v15.0.4-canary.26 and v15.0.4-canary.27

In my environment, I've isolated the regression behavior in storybook to this change. In other words when I revert my Next.JS version to v15.0.4-canary.26 it works. When I upgrade to v15.0.4-canary.27 it starts failing as the OP described.

This Next.JS diff includes a React upgrade from 19.0.0-rc-380f5d67-20241113 to 19.0.0-rc-b01722d5-20241114.

Here is the diff for those react versions which represents going from React RC 0 to React RC 1.

Here is a post about React 19 RC 1 from the React team.

It seems the big difference is that the React 19 enableSiblingPrerendering feature flag is switched to true. Not sure yet how or why this affects Storybook the way it does: causing infinite Suspense loops in RSC components.

@dmitrc
Copy link
Author

dmitrc commented Jan 24, 2025

Thanks for looking into it, @boylec!

It seems there are two distinct issues described in this thread:

  • Async RSC in infinite render loop (related to Suspense concurrency changes you have highlighted), tracked in [Bug]: Failed to render Async Component(RSC) with Next 15.1 #30033.

  • Some weird behavior wrt "losing" upstream context when a hook is called after the first await (see the next-intl part of my repro, it was like that for a long while now, before becoming an infinite render loop in 8.5.0).

@shilman
Copy link
Member

shilman commented Jan 24, 2025

Acknowledging that Storybook's handling of RSC is kind of funky to begin with, do we know if recent changes to Storybook contributed to these latest problems? Or are these entirely due to upgrading the version of React?

@NEKOYASAN
Copy link

I don't know the problem of losing the upstream context of the latter...
As for the infinite rendering loop, we confirmed that the situation did not change even when the Storybook version was changed during the investigation of #30033, and that only the Next.js version (React version) was having an effect.

@boylec
Copy link

boylec commented Jan 24, 2025

@dmitrc I have a hunch that the losing of the upstream context is related to the RSC infinite suspense issue, although I don't have proof.

Considering the example provided by @davidcarr-au:

Expected:
* Uncached initial render begins
* Uncached suspends due to 'use' hook invocation with a promise
* This promise causes suspense, and becomes part of the upstream React context in some way.
* Uncached render cycle re-runs... eventually the promise resolves or rejects and the render cycle is allowed to proceed past the point of that use hook.

Potential Actual:
* Uncached initial render begins
* Uncached suspends due to 'use' hook invocation with a promise
* This promise causes suspense, and becomes part of the upstream React context in some way.
* The component's context is being wiped for some reason at this point, its likely this promise is being wiped as well
* Uncached render cycle re-runs and a new promise is instantiated each time the 'use' hook is invoked (considering the original one is wiped), resulting in an infinite suspense loop.

@davidcarr-au 's 'Cached' example works because the promise's lifetime is not dictated by the component's context.

EDIT: In hindsight, I would expect that the behavior of the Uncached component in @davidcarr-au 's example is by design. If you call the 'use' hook with a newly instantiated promise on each invocation, it should be expected to suspend infinitely. I think one should always pass the same instance to the 'use' hook if one expects it to eventually exit the suspense state.

@davidcarr-au
Copy link

@boylec

Digging deeper (specifically the infinite render issue in React 19-rc1), here’s my understanding:

Pre-RC1:
React would render the nodes below the Suspense boundary along with the fallback. When the Promise resolved, React replaced the fallback with the resolved content. Since the child components retained the same context and memoized props, everything worked as expected. No new Promises were instantiated during this process.

RC1:
React now renders the fallback immediately and pre-warms the child nodes in parallel to fetch data asynchronously. However, if the child nodes generate new Promises during this pre-fetching step (e.g., due to unstable async functions), React re-suspends the boundary. This triggers a re-render, which creates a new Promise, leading to an infinite loop.

This is not exclusive to the new 'use' hook, same deal with rendering nested async components using the traditional await data-fetching pattern inside a Suspense boundary. Each render instantiates a new Promise, destabilizing the boundary and causing React to repeatedly suspend and re-render.

In Nextjs, server actions make this behaviour stable by resolving data on the server, preventing the client from generating new Promises during rendering. However, Storybook doesn’t have this advantage, as it runs purely client-side, exposing issues with unstable async functions or Promises.

So we can use promises and async components like we did pre-rc1 within storybook, but it requires a stabilisation mechanism, hence why the singleton example i provided worked, i've resorted to running calls through a middleware to cache the promise and then remove it on first re-request when mocked for storybook.

FYI the React docs also highlight this caveat: client-side Promises recreated on each render will cause unnecessary re-suspends:
https://react.dev/reference/react/use#caveats

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Needs Discussion
Development

No branches or pull requests

6 participants