Skip to content

Fix global fetch this issue#1128

Merged
pyramation merged 3 commits into
mainfrom
fix/global-this-fetch
May 11, 2026
Merged

Fix global fetch this issue#1128
pyramation merged 3 commits into
mainfrom
fix/global-this-fetch

Conversation

@NorOldBurden
Copy link
Copy Markdown
Contributor

@NorOldBurden NorOldBurden commented May 11, 2026

The Bug
In FetchAdapter, the generated code does this:
ts
this.fetchFn = fetchFn ?? createFetch(); // step 1: store fetch as property
// ...
const response = await this.fetchFn(url, opts); // step 2: call it
When createFetch() runs in the browser, it returns globalThis.fetch — which is window.fetch. The problem is step 2: calling this.fetchFn(...) invokes fetch with this === the FetchAdapter instance, but native window.fetch requires this === window. That's what "Illegal invocation" means — the browser rejects the call because this is wrong.In plain terms: storing a native function as an object property and then calling it detaches it from its original owner. Same reason "hello".trim works but const fn = "hello".trim; fn() throws.

@yyyyaaa
Copy link
Copy Markdown
Contributor

yyyyaaa commented May 11, 2026

  1. sdk/constructive-sdk/src/public/orm/client.ts:61 and the other 12 checked-in generated SDK clients still contain this.fetchFn = fetchFn ?? createFetch();.
    .ie need to regenerate all sdks in this monorepo

  2. The added regression tests are weak. The substring assertion at PR test lines 80-87 only proves .bind(globalThis) appears somewhere in generated text, not that FetchAdapter binds the selected fetch function correctly. It would pass with .bind(globalThis) in a comment or
    with the precedence bug fetchFn ?? createFetch().bind(globalThis). The second test at lines 89-105 does not instantiate FetchAdapter; it tests JavaScript bind() behavior in isolation.

  3. The second test also does not match native browser timing exactly. In Chrome, detached window.fetch returns a Promise and then rejects with TypeError: Failed to execute 'fetch' on 'Window': Illegal invocation; it does not synchronously throw. The production await
    this.fetchFn(...) still observes it as a failure, but the test is a simplified demo, not a browser-faithful regression.

@pyramation pyramation merged commit a8b20ca into main May 11, 2026
55 checks passed
@pyramation pyramation deleted the fix/global-this-fetch branch May 11, 2026 09:09
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.

3 participants