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.

Sign up for GitHub

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

Jump to bottom

[BUG] Buffering HTML for bots is not working properly in workers environments #1383

Closed
frandiox opened this issue May 30, 2022 · 4 comments · Fixed by #1571
Closed

[BUG] Buffering HTML for bots is not working properly in workers environments #1383

frandiox opened this issue May 30, 2022 · 4 comments · Fixed by #1571
Labels
bug Something isn't working

Comments

@frandiox
Copy link
Contributor

When running in a worker environment using ?_bot, we get a response like this:

<body><div id="root"><!--$?--><template id="B:0"></template><!--/$--><!--$?--><template id="B:1">
...
</body></html><div hidden id="S:0"><!--$?--><template id="B:2"></template><header class="h-20 lg:h-32 max-w-screen text-gray-700">
...

Which means, even though we await for ssrReadable.allReady before buffering, React is still producing JS tags to assemble the HTML.

In Node, however, we get a proper response by starting to buffer after onAllReady is called:

<body><div id="root"><!--$--><!--$--><div class="absolute top-0 left-0"><a href="[#mainContent](http://localhost:3000/?_bot#mainContent)"
...
</body></html>
@frandiox frandiox added the bug Something isn't working label May 30, 2022
@jplhomer
Copy link
Contributor

Related Shopify/hydrogen#1309 (comment)

@frandiox frandiox mentioned this issue May 30, 2022
Merged
4 tasks
@jplhomer
Copy link
Contributor

jplhomer commented May 31, 2022 •
edited
Loading

While Cloudflare works on a fix, we can fix this temporarily by updating the ReadableStream constructor in react-dom-server-vite (update: we can't, because this is Fizz not Flight).

new ReadableStream({
  // ...
+ }, {
+  highWaterMark: 0,
});

I might also propose this upstream in React to be more explicit.

@jplhomer jplhomer mentioned this issue May 31, 2022
Merged
@jplhomer
Copy link
Contributor

jplhomer commented Jun 2, 2022

Since the React fix was merged upstream, developers can get this fix by using react@experimental react-dom@experimental. Otherwise, it will be fixed with Cloudflare ships the fix, or when React ships 18.2.

@jplhomer
Copy link
Contributor

jplhomer commented Jun 2, 2022

Update: Cloudflare will be shipping the fix next week (Week of June 7, 2022)

@frandiox frandiox mentioned this issue Jun 7, 2022
Merged
4 tasks
@jplhomer jplhomer mentioned this issue Jun 14, 2022
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Upgrade to React v18.2 Shopify/hydrogen-v1
2 participants
@jplhomer @frandiox