Skip to content

fix: ESM compatibility - avoid direct Response object mutation #1430

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: ESM compatibility - avoid direct Response object mutation #1430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
26 changes: 23 additions & 3 deletions templates/base/http-clients/fetch-http-client.ejs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,29 @@ export class HttpClient<SecurityDataType = unknown> {
body: typeof body === "undefined" || body === null ? null : payloadFormatter(body),
}
).then(async (response) => {
const r = response as HttpResponse<T, E>;
r.data = (null as unknown) as T;
r.error = (null as unknown) as E;
// Create a wrapper object that doesn't mutate the Response
// This ensures compatibility with ESM environments where Response is read-only
const r = {
data: (null as unknown) as T,
error: (null as unknown) as E,
// Delegate Response properties
ok: response.ok,
status: response.status,
statusText: response.statusText,
headers: response.headers,
url: response.url,
redirected: response.redirected,
type: response.type,
body: response.body,
bodyUsed: response.bodyUsed,
// Delegate Response methods
Comment on lines +199 to +214
Copy link

Choose a reason for hiding this comment

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

P1 Badge Keep HttpResponse.bodyUsed in sync with underlying Response

The new wrapper records response.bodyUsed once and stores the boolean on the returned object, but the response body is consumed later in the function when responseFormat is defined. After a call resolves, data.bodyUsed still reports the initial value (false) even though the underlying Response has been read and json()/text() can no longer be called without throwing. Previously, because the original Response instance was returned, bodyUsed reflected the consumed state. Any client code that checks bodyUsed to determine whether the body can be re-read will now be misled and may attempt a second read that fails.

Useful? React with 👍 / 👎.

arrayBuffer: () => response.arrayBuffer(),
blob: () => response.blob(),
clone: () => response.clone(),
formData: () => response.formData(),
json: () => response.json(),
text: () => response.text(),
} as HttpResponse<T, E>;
Copy link

Choose a reason for hiding this comment

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

Bug: HttpResponse Wrapper Causes Compatibility Problems

The new HttpResponse wrapper object introduces several compatibility issues. It's a plain object, which breaks instanceof Response checks. Properties like body and bodyUsed are statically copied, leading to stale values, and the stream() method is missing from the delegated Response methods.

Fix in Cursor Fix in Web


const responseToParse = responseFormat ? response.clone() : response;
const data = !responseFormat ? r : await responseToParse[responseFormat]()
Expand Down
Loading