From b32f150b4a448814e8b1d6e12f70180ca1de0ee9 Mon Sep 17 00:00:00 2001 From: Gefei Hou Date: Mon, 11 May 2026 02:21:10 -0700 Subject: [PATCH] Updated fetch fix test and sdk --- .../codegen/client-generator.test.ts | 171 ++++++++++++++++-- sdk/constructive-cli/src/admin/orm/client.ts | 2 +- sdk/constructive-cli/src/auth/orm/client.ts | 2 +- .../src/objects/orm/client.ts | 2 +- sdk/constructive-cli/src/public/orm/client.ts | 2 +- .../src/admin/orm/client.ts | 2 +- sdk/constructive-react/src/auth/orm/client.ts | 2 +- .../src/objects/orm/client.ts | 2 +- .../src/public/orm/client.ts | 2 +- sdk/constructive-sdk/src/admin/orm/client.ts | 2 +- sdk/constructive-sdk/src/auth/orm/client.ts | 2 +- .../src/objects/orm/client.ts | 2 +- sdk/constructive-sdk/src/public/orm/client.ts | 2 +- sdk/migrate-client/src/migrate/orm/client.ts | 2 +- 14 files changed, 167 insertions(+), 30 deletions(-) diff --git a/graphql/codegen/src/__tests__/codegen/client-generator.test.ts b/graphql/codegen/src/__tests__/codegen/client-generator.test.ts index 2badd5af4..9b06f6332 100644 --- a/graphql/codegen/src/__tests__/codegen/client-generator.test.ts +++ b/graphql/codegen/src/__tests__/codegen/client-generator.test.ts @@ -77,32 +77,169 @@ describe('client-generator', () => { expect(result.content).toContain('createFetch()'); }); - it('binds fetchFn to globalThis to avoid Illegal invocation in browsers', () => { + it('FetchAdapter constructor binds the selected fetch function to globalThis', () => { const result = generateOrmClientFile(); - // The generated FetchAdapter must .bind(globalThis) to prevent - // "Illegal invocation" when native window.fetch is stored as - // an instance property (which detaches it from its original this). - expect(result.content).toContain('.bind(globalThis)'); + // Must match the exact constructor assignment with correct precedence: + // this.fetchFn = (fetchFn ?? createFetch()).bind(globalThis); + // Guards against a stray .bind(globalThis) in a comment or wrong + // precedence like fetchFn ?? createFetch().bind(globalThis). + expect(result.content).toMatch( + /this\.fetchFn\s*=\s*\(\s*fetchFn\s*\?\?\s*createFetch\(\)\s*\)\.bind\(globalThis\)\s*;/, + ); + expect(result.content).not.toMatch( + /fetchFn\s*\?\?\s*createFetch\(\)\.bind\(globalThis\)/, + ); + }); + + it('FetchAdapter can execute with a this-sensitive native fetch', async () => { + // Simulate browser window.fetch: rejects with TypeError when this + // is not the original Window (Chrome rejects asynchronously). + function strictFetch(this: unknown): Promise { + if (this !== globalThis) { + return Promise.reject(new TypeError('Illegal invocation')); + } + return Promise.resolve( + new Response(JSON.stringify({ data: { answer: 42 } })), + ); + } + + // Replicate the exact constructor pattern generated in FetchAdapter. + class TestFetchAdapter { + private fetchFn: typeof globalThis.fetch; + constructor(fetchFn?: typeof globalThis.fetch) { + this.fetchFn = (fetchFn ?? strictFetch).bind(globalThis); + } + async callFetch(): Promise { + return this.fetchFn('http://test', { method: 'POST' } as RequestInit); + } + } + + // Without .bind(globalThis) this would reject with TypeError. + const adapter = new TestFetchAdapter(strictFetch); + const response = await adapter.callFetch(); + expect(response.ok).toBe(true); + const json = (await response.json()) as { data: { answer: number } }; + expect(json.data).toEqual({ answer: 42 }); + }); + + it('FetchAdapter from generated output binds fetch to globalThis at runtime', async () => { + // Simulate browser window.fetch: Chrome returns a Promise that rejects + // with TypeError when `this` is not the original Window. + function strictFetch(this: unknown): Promise { + if (this !== globalThis) { + return Promise.reject(new TypeError('Illegal invocation')); + } + return Promise.resolve( + new Response(JSON.stringify({ data: { answer: 99 } })), + ); + } + + // Mock createFetch so the default path also uses strictFetch. + const mockCreateFetch = () => strictFetch; + + // ------------------------------------------------------------------ + // Extract the ACTUAL fetchFn assignment expression from generated code + // (not a hand-copied replica). This closes the drift gap: if the + // template changes, the extraction fails or the runtime test breaks. + // ------------------------------------------------------------------ + const generated = generateOrmClientFile().content; + const assignmentMatch = generated.match( + /this\.fetchFn\s*=\s*([^;]+);/, + ); + expect(assignmentMatch).not.toBeNull(); + const assignmentRhs = assignmentMatch![1].trim(); + // e.g. "(fetchFn ?? createFetch()).bind(globalThis)" + + // Build a minimal FetchAdapter using the EXTRACTED assignment from + // the generated source. new Function() gives us a JS-evaluable class + // whose constructor body is driven by the real template output. + const EvalFetchAdapter = new Function( + 'createFetch', + ` + return class FetchAdapter { + constructor(endpoint, headers, fetchFn) { + this.fetchFn = ${assignmentRhs}; + } + callFetch() { + return this.fetchFn('http://test', { method: 'POST' }); + } + }; + `, + )(mockCreateFetch); + + // --- Case 1: explicit strictFetch passed in --- + // With correct precedence (fetchFn ?? createFetch()).bind(globalThis), + // the provided strictFetch is bound to globalThis and succeeds. + const adapter = new (EvalFetchAdapter as new ( + endpoint: string, + headers: Record | undefined, + fetchFn?: typeof globalThis.fetch, + ) => { fetchFn: typeof globalThis.fetch; callFetch: () => Promise })( + 'http://test', + {}, + strictFetch, + ); + const response = await adapter.callFetch(); + expect(response.ok).toBe(true); + const json = (await response.json()) as { data: { answer: number } }; + expect(json.data).toEqual({ answer: 99 }); + + // --- Case 2: no custom fetch (falls back to createFetch()) --- + const adapterDefault = new (EvalFetchAdapter as new ( + endpoint: string, + headers: Record | undefined, + fetchFn?: typeof globalThis.fetch, + ) => { fetchFn: typeof globalThis.fetch; callFetch: () => Promise })( + 'http://test', + {}, + undefined, + ); + const responseDefault = await adapterDefault.callFetch(); + expect(responseDefault.ok).toBe(true); }); - it('bind(globalThis) prevents Illegal invocation for this-sensitive fetch', () => { - // Simulate a native browser fetch that requires this === globalThis - // (calling window.fetch with any other this throws TypeError) - function thisSensitiveFetch(this: unknown): Promise { + it('wrong precedence fetchFn ?? createFetch().bind(globalThis) fails with strict fetch', async () => { + // This test proves the precedence matters at runtime. + // If the generated code used the wrong precedence + // fetchFn ?? createFetch().bind(globalThis) + // then a provided fetchFn would NOT be bound to globalThis. + function strictFetch(this: unknown): Promise { if (this !== globalThis) { - throw new TypeError('Illegal invocation'); + return Promise.reject(new TypeError('Illegal invocation')); } - return Promise.resolve(new Response(JSON.stringify({ data: null }))); + return Promise.resolve(new Response('ok')); } - // Without bind: storing on an object and calling detaches this - const broken = { fetchFn: thisSensitiveFetch }; - expect(() => broken.fetchFn()).toThrow('Illegal invocation'); + const mockCreateFetch = () => strictFetch; + + // WRONG precedence: .bind(globalThis) only applies to createFetch(), + // not to the provided fetchFn. + const WrongPrecedenceAdapter = new Function( + 'createFetch', + ` + return class { + constructor(endpoint, headers, fetchFn) { + this.fetchFn = fetchFn ?? createFetch().bind(globalThis); + } + callFetch() { + return this.fetchFn('http://test', { method: 'POST' }); + } + }; + `, + )(mockCreateFetch); + + const adapter = new (WrongPrecedenceAdapter as new ( + endpoint: string, + headers: Record | undefined, + fetchFn?: typeof globalThis.fetch, + ) => { callFetch: () => Promise })( + 'http://test', + {}, + strictFetch, + ); - // With bind: the pattern used in FetchAdapter keeps correct this - const fixed = { fetchFn: thisSensitiveFetch.bind(globalThis) }; - expect(() => fixed.fetchFn()).not.toThrow(); + await expect(adapter.callFetch()).rejects.toThrow('Illegal invocation'); }); }); diff --git a/sdk/constructive-cli/src/admin/orm/client.ts b/sdk/constructive-cli/src/admin/orm/client.ts index ccfdebd36..16e683c71 100644 --- a/sdk/constructive-cli/src/admin/orm/client.ts +++ b/sdk/constructive-cli/src/admin/orm/client.ts @@ -58,7 +58,7 @@ export class FetchAdapter implements GraphQLAdapter { fetchFn?: typeof globalThis.fetch ) { this.headers = headers ?? {}; - this.fetchFn = fetchFn ?? createFetch(); + this.fetchFn = (fetchFn ?? createFetch()).bind(globalThis); } async execute(document: string, variables?: Record): Promise> { diff --git a/sdk/constructive-cli/src/auth/orm/client.ts b/sdk/constructive-cli/src/auth/orm/client.ts index ccfdebd36..16e683c71 100644 --- a/sdk/constructive-cli/src/auth/orm/client.ts +++ b/sdk/constructive-cli/src/auth/orm/client.ts @@ -58,7 +58,7 @@ export class FetchAdapter implements GraphQLAdapter { fetchFn?: typeof globalThis.fetch ) { this.headers = headers ?? {}; - this.fetchFn = fetchFn ?? createFetch(); + this.fetchFn = (fetchFn ?? createFetch()).bind(globalThis); } async execute(document: string, variables?: Record): Promise> { diff --git a/sdk/constructive-cli/src/objects/orm/client.ts b/sdk/constructive-cli/src/objects/orm/client.ts index ccfdebd36..16e683c71 100644 --- a/sdk/constructive-cli/src/objects/orm/client.ts +++ b/sdk/constructive-cli/src/objects/orm/client.ts @@ -58,7 +58,7 @@ export class FetchAdapter implements GraphQLAdapter { fetchFn?: typeof globalThis.fetch ) { this.headers = headers ?? {}; - this.fetchFn = fetchFn ?? createFetch(); + this.fetchFn = (fetchFn ?? createFetch()).bind(globalThis); } async execute(document: string, variables?: Record): Promise> { diff --git a/sdk/constructive-cli/src/public/orm/client.ts b/sdk/constructive-cli/src/public/orm/client.ts index ccfdebd36..16e683c71 100644 --- a/sdk/constructive-cli/src/public/orm/client.ts +++ b/sdk/constructive-cli/src/public/orm/client.ts @@ -58,7 +58,7 @@ export class FetchAdapter implements GraphQLAdapter { fetchFn?: typeof globalThis.fetch ) { this.headers = headers ?? {}; - this.fetchFn = fetchFn ?? createFetch(); + this.fetchFn = (fetchFn ?? createFetch()).bind(globalThis); } async execute(document: string, variables?: Record): Promise> { diff --git a/sdk/constructive-react/src/admin/orm/client.ts b/sdk/constructive-react/src/admin/orm/client.ts index ccfdebd36..16e683c71 100644 --- a/sdk/constructive-react/src/admin/orm/client.ts +++ b/sdk/constructive-react/src/admin/orm/client.ts @@ -58,7 +58,7 @@ export class FetchAdapter implements GraphQLAdapter { fetchFn?: typeof globalThis.fetch ) { this.headers = headers ?? {}; - this.fetchFn = fetchFn ?? createFetch(); + this.fetchFn = (fetchFn ?? createFetch()).bind(globalThis); } async execute(document: string, variables?: Record): Promise> { diff --git a/sdk/constructive-react/src/auth/orm/client.ts b/sdk/constructive-react/src/auth/orm/client.ts index ccfdebd36..16e683c71 100644 --- a/sdk/constructive-react/src/auth/orm/client.ts +++ b/sdk/constructive-react/src/auth/orm/client.ts @@ -58,7 +58,7 @@ export class FetchAdapter implements GraphQLAdapter { fetchFn?: typeof globalThis.fetch ) { this.headers = headers ?? {}; - this.fetchFn = fetchFn ?? createFetch(); + this.fetchFn = (fetchFn ?? createFetch()).bind(globalThis); } async execute(document: string, variables?: Record): Promise> { diff --git a/sdk/constructive-react/src/objects/orm/client.ts b/sdk/constructive-react/src/objects/orm/client.ts index ccfdebd36..16e683c71 100644 --- a/sdk/constructive-react/src/objects/orm/client.ts +++ b/sdk/constructive-react/src/objects/orm/client.ts @@ -58,7 +58,7 @@ export class FetchAdapter implements GraphQLAdapter { fetchFn?: typeof globalThis.fetch ) { this.headers = headers ?? {}; - this.fetchFn = fetchFn ?? createFetch(); + this.fetchFn = (fetchFn ?? createFetch()).bind(globalThis); } async execute(document: string, variables?: Record): Promise> { diff --git a/sdk/constructive-react/src/public/orm/client.ts b/sdk/constructive-react/src/public/orm/client.ts index ccfdebd36..16e683c71 100644 --- a/sdk/constructive-react/src/public/orm/client.ts +++ b/sdk/constructive-react/src/public/orm/client.ts @@ -58,7 +58,7 @@ export class FetchAdapter implements GraphQLAdapter { fetchFn?: typeof globalThis.fetch ) { this.headers = headers ?? {}; - this.fetchFn = fetchFn ?? createFetch(); + this.fetchFn = (fetchFn ?? createFetch()).bind(globalThis); } async execute(document: string, variables?: Record): Promise> { diff --git a/sdk/constructive-sdk/src/admin/orm/client.ts b/sdk/constructive-sdk/src/admin/orm/client.ts index ccfdebd36..16e683c71 100644 --- a/sdk/constructive-sdk/src/admin/orm/client.ts +++ b/sdk/constructive-sdk/src/admin/orm/client.ts @@ -58,7 +58,7 @@ export class FetchAdapter implements GraphQLAdapter { fetchFn?: typeof globalThis.fetch ) { this.headers = headers ?? {}; - this.fetchFn = fetchFn ?? createFetch(); + this.fetchFn = (fetchFn ?? createFetch()).bind(globalThis); } async execute(document: string, variables?: Record): Promise> { diff --git a/sdk/constructive-sdk/src/auth/orm/client.ts b/sdk/constructive-sdk/src/auth/orm/client.ts index ccfdebd36..16e683c71 100644 --- a/sdk/constructive-sdk/src/auth/orm/client.ts +++ b/sdk/constructive-sdk/src/auth/orm/client.ts @@ -58,7 +58,7 @@ export class FetchAdapter implements GraphQLAdapter { fetchFn?: typeof globalThis.fetch ) { this.headers = headers ?? {}; - this.fetchFn = fetchFn ?? createFetch(); + this.fetchFn = (fetchFn ?? createFetch()).bind(globalThis); } async execute(document: string, variables?: Record): Promise> { diff --git a/sdk/constructive-sdk/src/objects/orm/client.ts b/sdk/constructive-sdk/src/objects/orm/client.ts index ccfdebd36..16e683c71 100644 --- a/sdk/constructive-sdk/src/objects/orm/client.ts +++ b/sdk/constructive-sdk/src/objects/orm/client.ts @@ -58,7 +58,7 @@ export class FetchAdapter implements GraphQLAdapter { fetchFn?: typeof globalThis.fetch ) { this.headers = headers ?? {}; - this.fetchFn = fetchFn ?? createFetch(); + this.fetchFn = (fetchFn ?? createFetch()).bind(globalThis); } async execute(document: string, variables?: Record): Promise> { diff --git a/sdk/constructive-sdk/src/public/orm/client.ts b/sdk/constructive-sdk/src/public/orm/client.ts index ccfdebd36..16e683c71 100644 --- a/sdk/constructive-sdk/src/public/orm/client.ts +++ b/sdk/constructive-sdk/src/public/orm/client.ts @@ -58,7 +58,7 @@ export class FetchAdapter implements GraphQLAdapter { fetchFn?: typeof globalThis.fetch ) { this.headers = headers ?? {}; - this.fetchFn = fetchFn ?? createFetch(); + this.fetchFn = (fetchFn ?? createFetch()).bind(globalThis); } async execute(document: string, variables?: Record): Promise> { diff --git a/sdk/migrate-client/src/migrate/orm/client.ts b/sdk/migrate-client/src/migrate/orm/client.ts index ccfdebd36..16e683c71 100644 --- a/sdk/migrate-client/src/migrate/orm/client.ts +++ b/sdk/migrate-client/src/migrate/orm/client.ts @@ -58,7 +58,7 @@ export class FetchAdapter implements GraphQLAdapter { fetchFn?: typeof globalThis.fetch ) { this.headers = headers ?? {}; - this.fetchFn = fetchFn ?? createFetch(); + this.fetchFn = (fetchFn ?? createFetch()).bind(globalThis); } async execute(document: string, variables?: Record): Promise> {