-
Notifications
You must be signed in to change notification settings - Fork 50.5k
Explicitly set highWaterMark to 0 for ReadableStream
#24641
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
Conversation
This is because not all streaming implementations respect the default behavior of settings highWaterMark to 0 for byte streams. Being explicit guarantees the intended behavior across runtimes.
|
Comparing: 1328ff7...fff3966 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: (No significant changes) |
|
You're getting failed tests due the |
|
Ahh, Flow doesn't like any queuingStrategy without a I've updated with I tried implementing https://developer.mozilla.org/en-US/docs/Web/API/ByteLengthQueuingStrategy but Flow doesn't seem to recognize that as a valid thing, either. |
Summary
This is because not all streaming implementations (e.g. Cloudflare) respect the default behavior of settings
highWaterMarkto 0 for byte streams: 75662d6Related Hydrogen bug: Shopify/hydrogen#1383
Being explicit guarantees the intended behavior across runtimes. I've contacted Cloudflare, and they are working on fixing this in their implementation in the meantime.
How did you test this change?
highWaterMarkmanually in the compiled source code