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 19] Inconsistent "cache" api with Async Operations in react-server #29955

Open
nestarz opened this issue Jun 19, 2024 · 4 comments
Open
Labels

Comments

@nestarz
Copy link

nestarz commented Jun 19, 2024

Description:

When running the provided code with node, I encounter an issue where the cache is inconsistent depending on whether an async operation is present.

Steps to Reproduce:

  1. Run the following code with node:
const { renderToReadableStream } = require("react-server-dom-webpack/server.edge");
const { cache, createElement } = require("react");

const getId = cache(() => ({ id: Math.random() }));

const A = async () => {
    await new Promise(setImmediate);
    const id = getId();
    console.log(id);
    return null;
};

const B = async () => {
    const id = getId();
    console.log(id);
    return createElement(A, {}, null);
};

renderToReadableStream(createElement(B, {}, null), null);
  1. Observe the console output. The id values logged in A and B are different.
  2. Modify the code to remove the await new Promise(setImmediate); line from A:
const A = async () => {
  const id = getId();
  console.log(id);
  return null;
};
  1. Run the modified code with node again.
  2. Observe the console output. The id values logged in A and B are now the same.

Expected Behavior:

The id values should be consistent regardless of the presence of async operations during the render.

Actual Behavior:

The id values are different when an async operation is present in the render method.

Reproduction Code:

You can run the code using the following command:

npm run start

or a codesandbox: https://codesandbox.io/p/github/nestarz/react-19-cache-issue/main?import=true
and the github repo: https://github.com/nestarz/react-19-cache-issue

Additional Information:

It seems there might be an issue with the request context propagation when async operations are involved during the render process. This leads to inconsistent behavior which could cause unpredictable bugs.

Environment:

  • node v20.8.1
{
  "name": "test",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "start": "node --conditions react-server index.js",
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "dependencies": {
    "react": "^19.0.0-rc-e684ca66ab-20240619",
    "react-server-dom-webpack": "^19.0.0-rc-e684ca66ab-20240619"
  }
}

It impacts the consistency and reliability of cache in async rendering scenarios, and it's critical when used as an alternative to server context, is this normal ? Thank you!

@nestarz nestarz changed the title [React 19] Inconsistent Cache with Async Operations in react-server-dom [React 19] Inconsistent "cache" api with Async Operations in react-server Jun 19, 2024
@sebmarkbage
Copy link
Collaborator

Note that the .edge package is not intended to be used in Node.js. At least not without more fully controlling the environment like Next.js is doing but even then it's not even recommended for Next.js.

You should be using react-server-dom-webpack/server (which in the "node" condition resolves to react-server-dom-webpack/server.node).

There are many subtle things that are environment dependent. For example this check is a legacy check that looks for AsyncLocalStorage on the global object because importing it doesn't work in some "edge" environments or requires a flag.

https://github.com/facebook/react/blob/main/packages/react-server/src/forks/ReactFlightServerConfig.dom-edge.js#L17

So you'd need to set it up to expose it on the global but that's really something we're intending to change once all environments catch up to have it.

But there are other things that lead to much worse performance when you use Edge in Node.js due to the streaming as well as the edge using setTimeout for scheduling. There's also debugging information that will only be available using the Node.js build.

Therefore, using the "edge" build in Node.js is currently not really supported.

@nestarz
Copy link
Author

nestarz commented Jun 21, 2024

Thanks @sebmarkbage, in which environment is the edge build meant to work?

I figured out how to create a simple reproduction here, but my real issue arises when I use "react-server-dom-esm/server.node," which I have built from the source.

Here is a CodeSandbox: https://codesandbox.io/p/github/nestarz/react-19-cache-issue/jsr
And the related built file: https://jsr.io/@bureaudouble/rsc-engine/0.0.101/vendor/react-server-dom-esm/cjs/react-server-dom-esm-server.node.development.js

So, here it's not because of the runtime but because of the "esm" build. I know it's not well-supported either, but can you point me to a fix so that AsyncLocalStorage works better?

For disclosure, I'm trying to build a simple meta-framework for use in multiple runtimes, like Node and Deno.
Instead of using webpack, I solely rely on esbuild for client components and their dependencies, and make use of import-maps for resolution.
The framework works well; see here: https://github.com/nestarz/bureaudouble-rsc-demo/
The only missing piece is a working request context preservation throughout the render lifecycle and through multiple async operations; and I'm kind of confident that the issue is from the react codebase.

Copy link

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@github-actions github-actions bot added the Resolution: Stale Automatically closed due to inactivity label Sep 19, 2024
@nestarz
Copy link
Author

nestarz commented Sep 20, 2024

I'm still affected by this issue, the AsyncStorage usage by the cache api is not supported even on the esm/node version and I provided a codesandbox with a minimal reproduction code to help.

@github-actions github-actions bot removed the Resolution: Stale Automatically closed due to inactivity label Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants