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.

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(ext/fetch): clone second branch chunks in Body.clone() #20057

Merged
merged 1 commit into from
Aug 15, 2023

Conversation

marcosc90
Copy link
Contributor

@marcosc90 marcosc90 commented Aug 4, 2023

This PR makes Body.clone() spec compliant: https://fetch.spec.whatwg.org/#concept-body-clone

1, Let « out1, out2 » be the result of teeing body’s stream.
...
To tee a ReadableStream stream, return ? ReadableStreamTee(stream, true).


Closes #10994

@@ -9,6 +9,7 @@
const core = globalThis.Deno.core;
const ops = core.ops;
import * as webidl from "ext:deno_webidl/00_webidl.js";
import { structuredClone } from "ext:deno_web/02_structured_clone.js";
Copy link
Contributor Author

@marcosc90 marcosc90 Aug 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we using 02_structured_clone or 13_message_port one?

function structuredClone(value, options) {

ref: #12655

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its fine to use 02

Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@crowlKats please take a look as you're more knowledgable about this part of the codebase

Copy link
Member

@crowlKats crowlKats left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@crowlKats crowlKats merged commit 0fc31d9 into denoland:main Aug 15, 2023
12 checks passed
@marcosc90 marcosc90 deleted the fix-body-clone branch August 15, 2023 08:00
littledivy pushed a commit to littledivy/deno that referenced this pull request Aug 21, 2023
littledivy pushed a commit that referenced this pull request Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use structuredClone for teed ReadableStreams in Response.clone
3 participants