-
-
Notifications
You must be signed in to change notification settings - Fork 10.5k
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
Simplify default Node server entry #11619
Conversation
|
const body = new PassThrough(); | ||
const stream = createReadableStreamFromReadable(body); | ||
onAllReady() { | ||
if (prohibitOutOfOrderStreaming) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This approach is a little verbose, but as mentioned, matches the example in https://react.dev/reference/react-dom/server/renderToPipeableStream#waiting-for-all-content-to-load-for-crawlers-and-static-generation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mhmm - I'd personally vote for something like this to DRY it up a tiny bit if the two paths are identical (and have been for sometime?), but not a refactor I'd fight super hard for :)
let readyOption =
(userAgent && isbot(userAgent)) || remixContext.isSpaMode
? "onAllReady"
: "onShellReady";
...
const { pipe, abort } = renderToPipeableStream(
<RemixServer
context={remixContext}
url={request.url}
abortDelay={ABORT_DELAY}
/>,
{
[readyOption]() {
shellRendered = true;
...
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the way you cleaned this up. I've also refactored it so that the readyOption
var is defined as close to its usage as possible so it's a bit clearer what's going on, and also added a type annotation to ensure it's a valid option name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😍
This reverts commit dbf2e3d.
🤖 Hello there, We just published version Thanks! |
This refactor is based on the example from the official React docs: https://react.dev/reference/react-dom/server/renderToPipeableStream#waiting-for-all-content-to-load-for-crawlers-and-static-generation.
I've also been able to simplify usage of
isbot
since we can use this major release to prompt anyone using v3 to upgrade to v4.