Adding an option to disable HTTP streaming #3777
Conversation
🦋 Changeset detectedLatest commit: 14952ca The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
pkg: astro
tony-sull
changed the title
packages/astro/src/@types/astro.ts
Outdated
| * } | ||
| * ``` | ||
| */ | ||
| streaming?: boolean; |
There was a problem hiding this comment.
Think I can actually remove this completely if we don't want dev or preview support!
| let init = result.response; | ||
| let headers = new Headers(init.headers); | ||
| let body: BodyInit; | ||
| if (streaming) { |
There was a problem hiding this comment.
Bah, I wish CF supported ReadableStream. I'm still on a mission to reduce forking code paths, so in that spirit I'd love to use ReadableStream in both cases and then just pipe it to a string as a smaller, additional step in the non-streaming code path.
I can't think of any good way to remove the copy-paste code between the two if-else code paths, so I guess this might not be actionable. Just venting I guess :)
There was a problem hiding this comment.
That was actually my first solution! We can't use ReadableStream in CF at all unfortunately, the constructor throws an error immediately. Even worse, the error is basically silent if you don't add an extra try/catch around the entire SSR handler and logs are accessible yet for Cloudflare Pages Functions 🙃
|
@tony-sull big fan of starting as an option to |
* Adding a flag to disable HTTP streaming * refactor: adding support for SSG builds * handling string responses in the server runtime, adding tests * removing streaming CLI flag * removing import.meta.env.STREAMING * include Content-Length header when streaming is disabled * Verifying content-length header in dev * fix: default streaming to enabled in the base App server * TEMP: disabling the production test to investigate the test-adapter * re-enabling the test with an adapter option to disable streaming for the test * fix: use the existing TextEncoder to get the body's byte length * moving config to build.streaming, ignoring it in `dev` * fixing dev test to expect response streaming * chore: add changsets * removing the new config option all together 🎉 * remove temp debug log * Updating astro changeset now that streaming isn't a config option
Closes #3764
Changes
This adds an option to Astro's
appserver to disable HTTP streaming (enabled by default)Cloudflare Page Functions don't fully support
ReadableStreamyet - it is defined so a polyfill wouldn't really do the trick, but the server will throw an error when theReadableStreamconstructor is called. This change updates the Cloudflare adapter to always disable streamingTesting
Docs
Doc comments added for new config