-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
[fix] reading from same response body twice during prerender (#3473) #3521
[fix] reading from same response body twice during prerender (#3473) #3521
Conversation
✔️ Deploy Preview for kit-demo canceled. 🔨 Explore the source changes: 1cb1794 🔍 Inspect the deploy log: https://app.netlify.com/sites/kit-demo/deploys/61f19cb6c3d6000007dbf0f3 |
🦋 Changeset detectedLatest commit: 1cb1794 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
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.
Looks like cloning it has an impact on large responses (probably expected) and caused the tests to fail 🤔
it looks like we bumped into #591 here... |
…ender.js" This reverts commit 5e5f30b. Revert being stupid sveltejs#2
Thank you. I pushed a commit that changes the behaviour so that the prerenderer gets access to the same buffered body as the code that inlines responses into the HTML (i.e. without cloning the response), though as it was on its way to GitHub I realised that this would fail in the case where the data wasn't buffered during I guess we'll need a hybrid approach in which we use the buffered body if it's available, otherwise buffer it at prerender time. Working on it. |
…already_' error building the project removed SvelteKit 1.0.0-next.244 fixed [#3473](sveltejs/kit#3473) and [#3521](sveltejs/kit#3521). `clone()` on fetch response as workaround to avoid '_body used already_' error when building the project removed
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
This PR fixes the behavior described in #3473, where building the default template with adapter static fails as during the pre-rendering the same response body was read twice. As @bluwy pointed out in the original issue, this is related to fetch (and accordingly node-fetch) only allowing a single read of a response body and can be circumvented by using the clone() API that is provided for this purpose.
I also wrote a test that fails without this PR, and I think that would be generally useful to have.
Tests
pnpm test
and lint the project withpnpm lint
andpnpm check
test/test.js:953:2 › Load › handles large responses
fails due to problems discussed in #591Changesets
pnpx changeset
and following the prompts. All changesets should bepatch
until SvelteKit 1.0