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: clone methods correctly handle teeing body stream #17

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 15 additions & 0 deletions src/Body.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ class Body {
this._bodyInit = body;

if (!body) {
this._bodyNull = true;
this._bodyText = "";
return this;
}
Expand Down Expand Up @@ -179,6 +180,10 @@ class Body {
}

get body() {
if (this._bodyNull) {
return null;
}

if (this._bodyReadableStream) {
return this._bodyReadableStream;
}
Expand Down Expand Up @@ -228,6 +233,16 @@ class Body {
},
});
}

clone() {
Copy link
Author

@slightlytyler slightlytyler Jun 7, 2023

Choose a reason for hiding this comment

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

if (this._bodyReadableStream) {
const [stream1, stream2] = this._bodyReadableStream.tee();
this._bodyReadableStream = stream1;
return new Body(stream2);
} else {
return new Body(this._bodyInit);
}
}
}

export default Body;
11 changes: 9 additions & 2 deletions src/Request.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class Request {
this.signal = request.signal;
this.headers = new Headers(options.headers ?? request.headers);

if (!options.body && request._body._bodyInit) {
if (options.body === undefined && request._body._bodyInit) {
Copy link
Author

Choose a reason for hiding this comment

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

need to avoid explicit null body triggering this case I think

this._body = new Body(request._body._bodyInit);
request._body.bodyUsed = true;
}
Expand Down Expand Up @@ -85,7 +85,14 @@ class Request {
}

clone() {
Copy link
Author

Choose a reason for hiding this comment

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

return new Request(this, { body: this._body._bodyInit });
if (this.bodyUsed) {
throw new TypeError("Already read");
Copy link
Author

Choose a reason for hiding this comment

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

}

const newRequest = new Request(this, { body: null });
newRequest._body = this._body.clone();

return newRequest;
}

blob() {
Expand Down
9 changes: 8 additions & 1 deletion src/Response.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,19 @@ class Response {
}

clone() {
Copy link
Author

Choose a reason for hiding this comment

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

return new Response(this._body._bodyInit, {
if (this.bodyUsed) {
throw new TypeError("Already read");
Copy link
Author

Choose a reason for hiding this comment

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

}

const newResponse = new Response(null, {
status: this.status,
statusText: this.statusText,
headers: new Headers(this.headers),
url: this.url,
});
newResponse._body = this._body.clone();

return newResponse;
}

blob() {
Expand Down
53 changes: 41 additions & 12 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,7 @@ test("request", (t) => {
t.isNot(clone.headers, req.headers);
t.notOk(req.bodyUsed);

const bodies = await Promise.all([clone.text(), req.clone().text()]);
const bodies = await Promise.all([clone.text(), req.text()]);

t.eq(bodies, ["I work out", "I work out"]);
});
Expand Down Expand Up @@ -1864,14 +1864,27 @@ test("fetch method", (t) => {
reactNative: { textStreaming: true },
});
const clone = res.clone();
const stream = await clone.body;
const text = new TextDecoder().decode(await drainStream(stream));

const resStream = await res.body;
const cloneStream = await clone.body;
const resText = new TextDecoder().decode(
await drainStream(resStream)
);
const cloneText = new TextDecoder().decode(
await drainStream(cloneStream)
);

t.ok(
stream instanceof ReadableStream,
resStream instanceof ReadableStream,
"Response implements streaming body"
);
t.eq(text, "Hello world!");
t.eq(resText, "Hello world!");

t.ok(
cloneStream instanceof ReadableStream,
"Response implements streaming body"
);
t.eq(cloneText, "Hello world!");
});

t.test("cloning blob response", async (t) => {
Expand All @@ -1882,8 +1895,11 @@ test("fetch method", (t) => {
},
});
const clone = res.clone();
const json = await clone.json();
t.eq(json.headers.accept, "application/json");
const resJson = await res.json();
const cloneJson = await clone.json();

t.eq(resJson.headers.accept, "application/json");
t.eq(cloneJson.headers.accept, "application/json");
});

t.test("cloning array buffer response", async (t) => {
Expand All @@ -1894,15 +1910,28 @@ test("fetch method", (t) => {
},
});
const clone = res.clone();
const buf = await clone.arrayBuffer();
const resBuf = await res.arrayBuffer();
const cloneBuf = await clone.arrayBuffer();

t.ok(buf instanceof ArrayBuffer, "buf is an ArrayBuffer instance");
t.eq(buf.byteLength, 256, "buf.byteLength is correct");
t.ok(
resBuf instanceof ArrayBuffer,
"buf is an ArrayBuffer instance"
);
t.eq(resBuf.byteLength, 256, "buf.byteLength is correct");

t.ok(
cloneBuf instanceof ArrayBuffer,
"buf is an ArrayBuffer instance"
);
t.eq(cloneBuf.byteLength, 256, "buf.byteLength is correct");

const expected = Array.from({ length: 256 }, (_, i) => i);
const actual = Array.from(new Uint8Array(buf));

t.eq(actual, expected);
const resActual = Array.from(new Uint8Array(resBuf));
const cloneActual = Array.from(new Uint8Array(cloneBuf));

t.eq(resActual, expected);
t.eq(cloneActual, expected);
});
});
});