From cdf7b39a815fdc45f6626fc3feb8a32eb78efe72 Mon Sep 17 00:00:00 2001 From: Stephen Rosa Date: Tue, 28 Jan 2025 15:41:18 -0800 Subject: [PATCH 01/20] add custom headers to track web frameworks usage --- packages/data-connect/src/api/DataConnect.ts | 26 ++++++++++++++- packages/data-connect/src/network/fetch.ts | 33 +++++++++++++++++-- .../src/network/transport/index.ts | 5 ++- .../src/network/transport/rest.ts | 15 +++++++-- packages/data-connect/test/unit/fetch.test.ts | 6 ++++ 5 files changed, 78 insertions(+), 7 deletions(-) diff --git a/packages/data-connect/src/api/DataConnect.ts b/packages/data-connect/src/api/DataConnect.ts index 53eb0c97ed5..169feaf5389 100644 --- a/packages/data-connect/src/api/DataConnect.ts +++ b/packages/data-connect/src/api/DataConnect.ts @@ -92,6 +92,9 @@ export class DataConnect { private _transportOptions?: TransportOptions; private _authTokenProvider?: AuthTokenProvider; _isUsingGeneratedSdk: boolean = false; + _isUsingTanStackSdk: boolean = false; + _isUsingReactSdk: boolean = false; + _isUsingAngularFireSdk: boolean = false; private _appCheckTokenProvider?: AppCheckTokenProvider; // @internal constructor( @@ -116,6 +119,24 @@ export class DataConnect { this._isUsingGeneratedSdk = true; } } + // @internal + _useTanStackSdk(): void { + if (!this._isUsingTanStackSdk) { + this._isUsingTanStackSdk = true; + } + } + // @internal + _useReactSdk(): void { + if (!this._isUsingReactSdk) { + this._isUsingReactSdk = true; + } + } + // @internal + _useAngularFireSdk(): void { + if (!this._isUsingAngularFireSdk) { + this._isUsingAngularFireSdk = true; + } + } _delete(): Promise { _removeServiceInstance( this.app, @@ -164,7 +185,10 @@ export class DataConnect { this._authTokenProvider, this._appCheckTokenProvider, undefined, - this._isUsingGeneratedSdk + this._isUsingGeneratedSdk, + this._isUsingTanStackSdk, + this._isUsingReactSdk, + this._isUsingAngularFireSdk ); if (this._transportOptions) { this._transport.useEmulator( diff --git a/packages/data-connect/src/network/fetch.ts b/packages/data-connect/src/network/fetch.ts index d5d2a439432..10522bd6b8f 100644 --- a/packages/data-connect/src/network/fetch.ts +++ b/packages/data-connect/src/network/fetch.ts @@ -30,6 +30,27 @@ function getGoogApiClientValue(_isUsingGen: boolean): string { } return str; } +function getWebFrameworkValue( + _isUsingTanStack: boolean, + _isUsingReact: boolean, + _isUsingAngularFire: boolean +): string { + let str = ''; + if (_isUsingTanStack) { + str += ' tanstack/'; + } + if (_isUsingReact) { + str += ' react/'; + } + if (_isUsingAngularFire) { + str += ' angularfire/'; + } + // no framework SDK used + if (str === '') { + str = 'vanilla/'; + } + return str.trim(); +} export interface DataConnectFetchBody { name: string; operationName: string; @@ -42,14 +63,22 @@ export function dcFetch( appId: string | null, accessToken: string | null, appCheckToken: string | null, - _isUsingGen: boolean + _isUsingGen: boolean, + _isUsingTanStack: boolean, + _isUsingReact: boolean, + _isUsingAngularFire: boolean ): Promise<{ data: T; errors: Error[] }> { if (!connectFetch) { throw new DataConnectError(Code.OTHER, 'No Fetch Implementation detected!'); } const headers: HeadersInit = { 'Content-Type': 'application/json', - 'X-Goog-Api-Client': getGoogApiClientValue(_isUsingGen) + 'X-Goog-Api-Client': getGoogApiClientValue(_isUsingGen), + 'X-Firebase-DataConnect-Web-Frameworks': getWebFrameworkValue( + _isUsingTanStack, + _isUsingReact, + _isUsingAngularFire + ) }; if (accessToken) { headers['X-Firebase-Auth-Token'] = accessToken; diff --git a/packages/data-connect/src/network/transport/index.ts b/packages/data-connect/src/network/transport/index.ts index f4bb801f9b3..515dc6f81d0 100644 --- a/packages/data-connect/src/network/transport/index.ts +++ b/packages/data-connect/src/network/transport/index.ts @@ -45,5 +45,8 @@ export type TransportClass = new ( authProvider?: AuthTokenProvider, appCheckProvider?: AppCheckTokenProvider, transportOptions?: TransportOptions, - _isUsingGen?: boolean + _isUsingGen?: boolean, + _isUsingTanStack?: boolean, + _isUsingReact?: boolean, + _isUsingAngularFire?: boolean ) => DataConnectTransport; diff --git a/packages/data-connect/src/network/transport/rest.ts b/packages/data-connect/src/network/transport/rest.ts index 7c8500b733d..8d3a5bae613 100644 --- a/packages/data-connect/src/network/transport/rest.ts +++ b/packages/data-connect/src/network/transport/rest.ts @@ -43,7 +43,10 @@ export class RESTTransport implements DataConnectTransport { private authProvider?: AuthTokenProvider | undefined, private appCheckProvider?: AppCheckTokenProvider | undefined, transportOptions?: TransportOptions | undefined, - private _isUsingGen = false + private _isUsingGen = false, + private _isUsingReact = false, + private _isUsingAngularFire = false, + private _isUsingTanStack = false ) { if (transportOptions) { if (typeof transportOptions.port === 'number') { @@ -180,7 +183,10 @@ export class RESTTransport implements DataConnectTransport { this.appId, this._accessToken, this._appCheckToken, - this._isUsingGen + this._isUsingGen, + this._isUsingTanStack, + this._isUsingReact, + this._isUsingAngularFire ) ); return withAuth; @@ -205,7 +211,10 @@ export class RESTTransport implements DataConnectTransport { this.appId, this._accessToken, this._appCheckToken, - this._isUsingGen + this._isUsingGen, + this._isUsingTanStack, + this._isUsingReact, + this._isUsingAngularFire ); }); return taskResult; diff --git a/packages/data-connect/test/unit/fetch.test.ts b/packages/data-connect/test/unit/fetch.test.ts index 3d9a9b04523..3f16d02ac20 100644 --- a/packages/data-connect/test/unit/fetch.test.ts +++ b/packages/data-connect/test/unit/fetch.test.ts @@ -51,6 +51,9 @@ describe('fetch', () => { null, null, null, + false, + false, + false, false ) ).to.eventually.be.rejectedWith(message); @@ -74,6 +77,9 @@ describe('fetch', () => { null, null, null, + false, + false, + false, false ) ).to.eventually.be.rejectedWith(JSON.stringify(json)); From fb276876dfda9e90a908ff2c5abf42822dfdf15c Mon Sep 17 00:00:00 2001 From: Stephen Rosa Date: Thu, 30 Jan 2025 14:41:31 -0800 Subject: [PATCH 02/20] add transport methods to update framework usage flags after transport initialization --- packages/data-connect/src/api/DataConnect.ts | 19 +++++++++++----- packages/data-connect/src/network/fetch.ts | 8 +++---- .../src/network/transport/index.ts | 5 ++++- .../src/network/transport/rest.ts | 22 ++++++++++++++++--- 4 files changed, 41 insertions(+), 13 deletions(-) diff --git a/packages/data-connect/src/api/DataConnect.ts b/packages/data-connect/src/api/DataConnect.ts index 169feaf5389..ce72f2627aa 100644 --- a/packages/data-connect/src/api/DataConnect.ts +++ b/packages/data-connect/src/api/DataConnect.ts @@ -94,7 +94,7 @@ export class DataConnect { _isUsingGeneratedSdk: boolean = false; _isUsingTanStackSdk: boolean = false; _isUsingReactSdk: boolean = false; - _isUsingAngularFireSdk: boolean = false; + _isUsingAngularSdk: boolean = false; private _appCheckTokenProvider?: AppCheckTokenProvider; // @internal constructor( @@ -124,17 +124,26 @@ export class DataConnect { if (!this._isUsingTanStackSdk) { this._isUsingTanStackSdk = true; } + if (this._transport) { + this._transport._useTanStack(); + } } // @internal _useReactSdk(): void { if (!this._isUsingReactSdk) { this._isUsingReactSdk = true; } + if (this._transport) { + this._transport._useReact(); + } } // @internal - _useAngularFireSdk(): void { - if (!this._isUsingAngularFireSdk) { - this._isUsingAngularFireSdk = true; + _useAngularSdk(): void { + if (!this._isUsingAngularSdk) { + this._isUsingAngularSdk = true; + } + if (this._transport) { + this._transport._useAngular(); } } _delete(): Promise { @@ -188,7 +197,7 @@ export class DataConnect { this._isUsingGeneratedSdk, this._isUsingTanStackSdk, this._isUsingReactSdk, - this._isUsingAngularFireSdk + this._isUsingAngularSdk ); if (this._transportOptions) { this._transport.useEmulator( diff --git a/packages/data-connect/src/network/fetch.ts b/packages/data-connect/src/network/fetch.ts index 10522bd6b8f..e37c9b5692f 100644 --- a/packages/data-connect/src/network/fetch.ts +++ b/packages/data-connect/src/network/fetch.ts @@ -33,7 +33,7 @@ function getGoogApiClientValue(_isUsingGen: boolean): string { function getWebFrameworkValue( _isUsingTanStack: boolean, _isUsingReact: boolean, - _isUsingAngularFire: boolean + _isUsingAngular: boolean ): string { let str = ''; if (_isUsingTanStack) { @@ -42,7 +42,7 @@ function getWebFrameworkValue( if (_isUsingReact) { str += ' react/'; } - if (_isUsingAngularFire) { + if (_isUsingAngular) { str += ' angularfire/'; } // no framework SDK used @@ -66,7 +66,7 @@ export function dcFetch( _isUsingGen: boolean, _isUsingTanStack: boolean, _isUsingReact: boolean, - _isUsingAngularFire: boolean + _isUsingAngular: boolean ): Promise<{ data: T; errors: Error[] }> { if (!connectFetch) { throw new DataConnectError(Code.OTHER, 'No Fetch Implementation detected!'); @@ -77,7 +77,7 @@ export function dcFetch( 'X-Firebase-DataConnect-Web-Frameworks': getWebFrameworkValue( _isUsingTanStack, _isUsingReact, - _isUsingAngularFire + _isUsingAngular ) }; if (accessToken) { diff --git a/packages/data-connect/src/network/transport/index.ts b/packages/data-connect/src/network/transport/index.ts index 515dc6f81d0..2894045c87a 100644 --- a/packages/data-connect/src/network/transport/index.ts +++ b/packages/data-connect/src/network/transport/index.ts @@ -33,6 +33,9 @@ export interface DataConnectTransport { ): Promise<{ data: T; errors: Error[] }>; useEmulator(host: string, port?: number, sslEnabled?: boolean): void; onTokenChanged: (token: string | null) => void; + _useTanStack(): void; + _useReact(): void; + _useAngular(): void; } /** @@ -48,5 +51,5 @@ export type TransportClass = new ( _isUsingGen?: boolean, _isUsingTanStack?: boolean, _isUsingReact?: boolean, - _isUsingAngularFire?: boolean + _isUsingAngular?: boolean ) => DataConnectTransport; diff --git a/packages/data-connect/src/network/transport/rest.ts b/packages/data-connect/src/network/transport/rest.ts index 8d3a5bae613..c078db9c96f 100644 --- a/packages/data-connect/src/network/transport/rest.ts +++ b/packages/data-connect/src/network/transport/rest.ts @@ -45,7 +45,7 @@ export class RESTTransport implements DataConnectTransport { transportOptions?: TransportOptions | undefined, private _isUsingGen = false, private _isUsingReact = false, - private _isUsingAngularFire = false, + private _isUsingAngular = false, private _isUsingTanStack = false ) { if (transportOptions) { @@ -186,7 +186,7 @@ export class RESTTransport implements DataConnectTransport { this._isUsingGen, this._isUsingTanStack, this._isUsingReact, - this._isUsingAngularFire + this._isUsingAngular ) ); return withAuth; @@ -214,9 +214,25 @@ export class RESTTransport implements DataConnectTransport { this._isUsingGen, this._isUsingTanStack, this._isUsingReact, - this._isUsingAngularFire + this._isUsingAngular ); }); return taskResult; }; + + _useTanStack(): void { + if (!this._isUsingTanStack) { + this._isUsingTanStack = true; + } + } + _useReact(): void { + if (!this._isUsingReact) { + this._isUsingReact = true; + } + } + _useAngular(): void { + if (!this._isUsingAngular) { + this._isUsingAngular = true; + } + } } From 6f27cbd63026fc17424227776061ce7f5223b98b Mon Sep 17 00:00:00 2001 From: Stephen Rosa Date: Thu, 30 Jan 2025 14:41:31 -0800 Subject: [PATCH 03/20] add transport methods to update framework usage flags after transport initialization --- packages/data-connect/src/network/fetch.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/data-connect/src/network/fetch.ts b/packages/data-connect/src/network/fetch.ts index e37c9b5692f..43ef1891ca6 100644 --- a/packages/data-connect/src/network/fetch.ts +++ b/packages/data-connect/src/network/fetch.ts @@ -43,7 +43,7 @@ function getWebFrameworkValue( str += ' react/'; } if (_isUsingAngular) { - str += ' angularfire/'; + str += ' angular/'; } // no framework SDK used if (str === '') { From 40a8c0bff827dc6d3b52edd68ebbd11ee1be254e Mon Sep 17 00:00:00 2001 From: Stephen Rosa Date: Fri, 31 Jan 2025 14:49:07 -0800 Subject: [PATCH 04/20] redesign framework/gen flags into CallerSdkType enum --- common/api-review/data-connect.api.md | 13 ++++++ packages/data-connect/src/api/DataConnect.ts | 45 +++---------------- packages/data-connect/src/network/fetch.ts | 40 +++++------------ .../src/network/transport/index.ts | 30 ++++++++++--- .../src/network/transport/rest.ts | 34 +++----------- packages/data-connect/test/unit/fetch.test.ts | 11 ++--- 6 files changed, 61 insertions(+), 112 deletions(-) diff --git a/common/api-review/data-connect.api.md b/common/api-review/data-connect.api.md index 952d8b4dc10..51b23096c2c 100644 --- a/common/api-review/data-connect.api.md +++ b/common/api-review/data-connect.api.md @@ -11,6 +11,19 @@ import { FirebaseError } from '@firebase/util'; import { LogLevelString } from '@firebase/logger'; import { Provider } from '@firebase/component'; +// @public +export type CallerSdkType = 'Base' | 'Generated' | 'TanstackReactCore' | 'GeneratedReact' | 'TanstackAngularCore' | 'GeneratedAngular'; + +// @public (undocumented) +export const CallerSdkTypeEnum: Readonly<{ + readonly Base: "Base"; + readonly Generated: "Generated"; + readonly TanstackReactCore: "TanstackReactCore"; + readonly GeneratedReact: "GeneratedReact"; + readonly TanstackAngularCore: "TanstackAngularCore"; + readonly GeneratedAngular: "GeneratedAngular"; +}>; + // @public export function connectDataConnectEmulator(dc: DataConnect, host: string, port?: number, sslEnabled?: boolean): void; diff --git a/packages/data-connect/src/api/DataConnect.ts b/packages/data-connect/src/api/DataConnect.ts index ce72f2627aa..9e4bebc8a37 100644 --- a/packages/data-connect/src/api/DataConnect.ts +++ b/packages/data-connect/src/api/DataConnect.ts @@ -33,7 +33,7 @@ import { } from '../core/FirebaseAuthProvider'; import { QueryManager } from '../core/QueryManager'; import { logDebug, logError } from '../logger'; -import { DataConnectTransport, TransportClass } from '../network'; +import { CallerSdkType, CallerSdkTypeEnum, DataConnectTransport, TransportClass } from '../network'; import { RESTTransport } from '../network/transport/rest'; import { MutationManager } from './Mutation'; @@ -91,10 +91,7 @@ export class DataConnect { private _transportClass: TransportClass | undefined; private _transportOptions?: TransportOptions; private _authTokenProvider?: AuthTokenProvider; - _isUsingGeneratedSdk: boolean = false; - _isUsingTanStackSdk: boolean = false; - _isUsingReactSdk: boolean = false; - _isUsingAngularSdk: boolean = false; + _callerSdkType: CallerSdkType = CallerSdkTypeEnum.Base; private _appCheckTokenProvider?: AppCheckTokenProvider; // @internal constructor( @@ -114,37 +111,8 @@ export class DataConnect { } } // @internal - _useGeneratedSdk(): void { - if (!this._isUsingGeneratedSdk) { - this._isUsingGeneratedSdk = true; - } - } - // @internal - _useTanStackSdk(): void { - if (!this._isUsingTanStackSdk) { - this._isUsingTanStackSdk = true; - } - if (this._transport) { - this._transport._useTanStack(); - } - } - // @internal - _useReactSdk(): void { - if (!this._isUsingReactSdk) { - this._isUsingReactSdk = true; - } - if (this._transport) { - this._transport._useReact(); - } - } - // @internal - _useAngularSdk(): void { - if (!this._isUsingAngularSdk) { - this._isUsingAngularSdk = true; - } - if (this._transport) { - this._transport._useAngular(); - } + _setCallerSdkType(callerSdkType: CallerSdkType): void { + this._callerSdkType = callerSdkType; } _delete(): Promise { _removeServiceInstance( @@ -194,10 +162,7 @@ export class DataConnect { this._authTokenProvider, this._appCheckTokenProvider, undefined, - this._isUsingGeneratedSdk, - this._isUsingTanStackSdk, - this._isUsingReactSdk, - this._isUsingAngularSdk + this._callerSdkType ); if (this._transportOptions) { this._transport.useEmulator( diff --git a/packages/data-connect/src/network/fetch.ts b/packages/data-connect/src/network/fetch.ts index 43ef1891ca6..2aea2ab93dc 100644 --- a/packages/data-connect/src/network/fetch.ts +++ b/packages/data-connect/src/network/fetch.ts @@ -19,37 +19,23 @@ import { Code, DataConnectError } from '../core/error'; import { SDK_VERSION } from '../core/version'; import { logDebug, logError } from '../logger'; +import { CallerSdkType, CallerSdkTypeEnum } from './transport'; + let connectFetch: typeof fetch | null = globalThis.fetch; export function initializeFetch(fetchImpl: typeof fetch): void { connectFetch = fetchImpl; } -function getGoogApiClientValue(_isUsingGen: boolean): string { +function getGoogApiClientValue(_callerSdkType: CallerSdkType): string { let str = 'gl-js/ fire/' + SDK_VERSION; - if (_isUsingGen) { + if (_callerSdkType !== CallerSdkTypeEnum.Base) { str += ' js/gen'; } return str; } function getWebFrameworkValue( - _isUsingTanStack: boolean, - _isUsingReact: boolean, - _isUsingAngular: boolean + _callerSdkType: CallerSdkType ): string { - let str = ''; - if (_isUsingTanStack) { - str += ' tanstack/'; - } - if (_isUsingReact) { - str += ' react/'; - } - if (_isUsingAngular) { - str += ' angular/'; - } - // no framework SDK used - if (str === '') { - str = 'vanilla/'; - } - return str.trim(); + return _callerSdkType + "/"; } export interface DataConnectFetchBody { name: string; @@ -63,22 +49,16 @@ export function dcFetch( appId: string | null, accessToken: string | null, appCheckToken: string | null, - _isUsingGen: boolean, - _isUsingTanStack: boolean, - _isUsingReact: boolean, - _isUsingAngular: boolean + _callerSdkType: CallerSdkType ): Promise<{ data: T; errors: Error[] }> { if (!connectFetch) { throw new DataConnectError(Code.OTHER, 'No Fetch Implementation detected!'); } const headers: HeadersInit = { 'Content-Type': 'application/json', - 'X-Goog-Api-Client': getGoogApiClientValue(_isUsingGen), - 'X-Firebase-DataConnect-Web-Frameworks': getWebFrameworkValue( - _isUsingTanStack, - _isUsingReact, - _isUsingAngular - ) + 'X-Goog-Api-Client': getGoogApiClientValue(_callerSdkType), + 'X-Firebase-DataConnect-Web-Frameworks': + getWebFrameworkValue(_callerSdkType) }; if (accessToken) { headers['X-Firebase-Auth-Token'] = accessToken; diff --git a/packages/data-connect/src/network/transport/index.ts b/packages/data-connect/src/network/transport/index.ts index 2894045c87a..5b3c05e1049 100644 --- a/packages/data-connect/src/network/transport/index.ts +++ b/packages/data-connect/src/network/transport/index.ts @@ -19,6 +19,26 @@ import { DataConnectOptions, TransportOptions } from '../../api/DataConnect'; import { AppCheckTokenProvider } from '../../core/AppCheckTokenProvider'; import { AuthTokenProvider } from '../../core/FirebaseAuthProvider'; +/** + * enum representing different flavors of the SDK used by developers + * use the CallerSdkType for type-checking, and the CallerSdkTypeEnum for value-checking/assigning + */ +export type CallerSdkType = + | 'Base' // Core JS SDK + | 'Generated' // Generated JS SDK + | 'TanstackReactCore' // Tanstack non-generated React SDK + | 'GeneratedReact' // Generated React SDK + | 'TanstackAngularCore' // Tanstack non-generated Angular SDK + | 'GeneratedAngular'; // Generated Angular SDK +export const CallerSdkTypeEnum = Object.freeze({ + Base: 'Base', // Core JS SDK + Generated: 'Generated', // Generated JS SDK + TanstackReactCore: 'TanstackReactCore', // Tanstack non-generated React SDK + GeneratedReact: 'GeneratedReact', // Tanstack non-generated Angular SDK + TanstackAngularCore: 'TanstackAngularCore', // Tanstack non-generated Angular SDK + GeneratedAngular: 'GeneratedAngular' // Generated Angular SDK +} as const); + /** * @internal */ @@ -33,9 +53,8 @@ export interface DataConnectTransport { ): Promise<{ data: T; errors: Error[] }>; useEmulator(host: string, port?: number, sslEnabled?: boolean): void; onTokenChanged: (token: string | null) => void; - _useTanStack(): void; - _useReact(): void; - _useAngular(): void; + // @internal + _setCallerSdkType(callerSdkType: CallerSdkType): void; } /** @@ -48,8 +67,5 @@ export type TransportClass = new ( authProvider?: AuthTokenProvider, appCheckProvider?: AppCheckTokenProvider, transportOptions?: TransportOptions, - _isUsingGen?: boolean, - _isUsingTanStack?: boolean, - _isUsingReact?: boolean, - _isUsingAngular?: boolean + _callerSdkType?: CallerSdkType ) => DataConnectTransport; diff --git a/packages/data-connect/src/network/transport/rest.ts b/packages/data-connect/src/network/transport/rest.ts index c078db9c96f..a32f24f5df6 100644 --- a/packages/data-connect/src/network/transport/rest.ts +++ b/packages/data-connect/src/network/transport/rest.ts @@ -23,7 +23,7 @@ import { logDebug } from '../../logger'; import { addToken, urlBuilder } from '../../util/url'; import { dcFetch } from '../fetch'; -import { DataConnectTransport } from '.'; +import { CallerSdkType, CallerSdkTypeEnum, DataConnectTransport } from '.'; export class RESTTransport implements DataConnectTransport { private _host = ''; @@ -43,10 +43,7 @@ export class RESTTransport implements DataConnectTransport { private authProvider?: AuthTokenProvider | undefined, private appCheckProvider?: AppCheckTokenProvider | undefined, transportOptions?: TransportOptions | undefined, - private _isUsingGen = false, - private _isUsingReact = false, - private _isUsingAngular = false, - private _isUsingTanStack = false + private _callerSdkType: CallerSdkType = CallerSdkTypeEnum.Base, ) { if (transportOptions) { if (typeof transportOptions.port === 'number') { @@ -183,10 +180,7 @@ export class RESTTransport implements DataConnectTransport { this.appId, this._accessToken, this._appCheckToken, - this._isUsingGen, - this._isUsingTanStack, - this._isUsingReact, - this._isUsingAngular + this._callerSdkType ) ); return withAuth; @@ -211,28 +205,14 @@ export class RESTTransport implements DataConnectTransport { this.appId, this._accessToken, this._appCheckToken, - this._isUsingGen, - this._isUsingTanStack, - this._isUsingReact, - this._isUsingAngular + this._callerSdkType ); }); return taskResult; }; - _useTanStack(): void { - if (!this._isUsingTanStack) { - this._isUsingTanStack = true; - } - } - _useReact(): void { - if (!this._isUsingReact) { - this._isUsingReact = true; - } - } - _useAngular(): void { - if (!this._isUsingAngular) { - this._isUsingAngular = true; - } + // @internal + _setCallerSdkType(callerSdkType: CallerSdkType): void { + this._callerSdkType = callerSdkType; } } diff --git a/packages/data-connect/test/unit/fetch.test.ts b/packages/data-connect/test/unit/fetch.test.ts index 3f16d02ac20..e9fddc03cb9 100644 --- a/packages/data-connect/test/unit/fetch.test.ts +++ b/packages/data-connect/test/unit/fetch.test.ts @@ -20,6 +20,7 @@ import chaiAsPromised from 'chai-as-promised'; import * as sinon from 'sinon'; import { dcFetch, initializeFetch } from '../../src/network/fetch'; +import { CallerSdkTypeEnum } from '../../src/network/transport'; use(chaiAsPromised); function mockFetch(json: object): void { const fakeFetchImpl = sinon.stub().returns( @@ -51,10 +52,7 @@ describe('fetch', () => { null, null, null, - false, - false, - false, - false + CallerSdkTypeEnum.Base ) ).to.eventually.be.rejectedWith(message); }); @@ -77,10 +75,7 @@ describe('fetch', () => { null, null, null, - false, - false, - false, - false + CallerSdkTypeEnum.Base ) ).to.eventually.be.rejectedWith(JSON.stringify(json)); }); From 5e397d003fb4fc102ea9536269f80dba24db62cf Mon Sep 17 00:00:00 2001 From: Stephen Rosa Date: Fri, 31 Jan 2025 15:35:14 -0800 Subject: [PATCH 05/20] update custom headers assignment logic and values; add gen flag functionality back for future deprecation --- packages/data-connect/src/api/DataConnect.ts | 15 ++++++++++++++- packages/data-connect/src/network/fetch.ts | 14 +++++--------- .../data-connect/src/network/transport/index.ts | 3 ++- .../data-connect/src/network/transport/rest.ts | 3 +++ packages/data-connect/test/unit/fetch.test.ts | 2 ++ 5 files changed, 26 insertions(+), 11 deletions(-) diff --git a/packages/data-connect/src/api/DataConnect.ts b/packages/data-connect/src/api/DataConnect.ts index 9e4bebc8a37..f736464f361 100644 --- a/packages/data-connect/src/api/DataConnect.ts +++ b/packages/data-connect/src/api/DataConnect.ts @@ -33,7 +33,12 @@ import { } from '../core/FirebaseAuthProvider'; import { QueryManager } from '../core/QueryManager'; import { logDebug, logError } from '../logger'; -import { CallerSdkType, CallerSdkTypeEnum, DataConnectTransport, TransportClass } from '../network'; +import { + CallerSdkType, + CallerSdkTypeEnum, + DataConnectTransport, + TransportClass +} from '../network'; import { RESTTransport } from '../network/transport/rest'; import { MutationManager } from './Mutation'; @@ -91,6 +96,7 @@ export class DataConnect { private _transportClass: TransportClass | undefined; private _transportOptions?: TransportOptions; private _authTokenProvider?: AuthTokenProvider; + _isUsingGeneratedSdk: boolean = false; _callerSdkType: CallerSdkType = CallerSdkTypeEnum.Base; private _appCheckTokenProvider?: AppCheckTokenProvider; // @internal @@ -111,6 +117,12 @@ export class DataConnect { } } // @internal + _useGeneratedSdk(): void { + if (!this._isUsingGeneratedSdk) { + this._isUsingGeneratedSdk = true; + } + } + // @internal _setCallerSdkType(callerSdkType: CallerSdkType): void { this._callerSdkType = callerSdkType; } @@ -162,6 +174,7 @@ export class DataConnect { this._authTokenProvider, this._appCheckTokenProvider, undefined, + this._isUsingGeneratedSdk, this._callerSdkType ); if (this._transportOptions) { diff --git a/packages/data-connect/src/network/fetch.ts b/packages/data-connect/src/network/fetch.ts index 2aea2ab93dc..6a77b315ac6 100644 --- a/packages/data-connect/src/network/fetch.ts +++ b/packages/data-connect/src/network/fetch.ts @@ -27,16 +27,13 @@ export function initializeFetch(fetchImpl: typeof fetch): void { } function getGoogApiClientValue(_callerSdkType: CallerSdkType): string { let str = 'gl-js/ fire/' + SDK_VERSION; - if (_callerSdkType !== CallerSdkTypeEnum.Base) { + if (_callerSdkType === CallerSdkTypeEnum.Generated) { str += ' js/gen'; + } else if (_callerSdkType !== CallerSdkTypeEnum.Base) { + str += ' js/' + _callerSdkType.toLowerCase(); } return str; } -function getWebFrameworkValue( - _callerSdkType: CallerSdkType -): string { - return _callerSdkType + "/"; -} export interface DataConnectFetchBody { name: string; operationName: string; @@ -49,6 +46,7 @@ export function dcFetch( appId: string | null, accessToken: string | null, appCheckToken: string | null, + _isUsingGen: boolean, _callerSdkType: CallerSdkType ): Promise<{ data: T; errors: Error[] }> { if (!connectFetch) { @@ -56,9 +54,7 @@ export function dcFetch( } const headers: HeadersInit = { 'Content-Type': 'application/json', - 'X-Goog-Api-Client': getGoogApiClientValue(_callerSdkType), - 'X-Firebase-DataConnect-Web-Frameworks': - getWebFrameworkValue(_callerSdkType) + 'X-Goog-Api-Client': getGoogApiClientValue(_callerSdkType) }; if (accessToken) { headers['X-Firebase-Auth-Token'] = accessToken; diff --git a/packages/data-connect/src/network/transport/index.ts b/packages/data-connect/src/network/transport/index.ts index 5b3c05e1049..ddc401877d8 100644 --- a/packages/data-connect/src/network/transport/index.ts +++ b/packages/data-connect/src/network/transport/index.ts @@ -67,5 +67,6 @@ export type TransportClass = new ( authProvider?: AuthTokenProvider, appCheckProvider?: AppCheckTokenProvider, transportOptions?: TransportOptions, - _callerSdkType?: CallerSdkType + _isUsingGen?: boolean, + _callerSdkType?: CallerSdkType ) => DataConnectTransport; diff --git a/packages/data-connect/src/network/transport/rest.ts b/packages/data-connect/src/network/transport/rest.ts index a32f24f5df6..b58ef9e621a 100644 --- a/packages/data-connect/src/network/transport/rest.ts +++ b/packages/data-connect/src/network/transport/rest.ts @@ -43,6 +43,7 @@ export class RESTTransport implements DataConnectTransport { private authProvider?: AuthTokenProvider | undefined, private appCheckProvider?: AppCheckTokenProvider | undefined, transportOptions?: TransportOptions | undefined, + private _isUsingGen = false, private _callerSdkType: CallerSdkType = CallerSdkTypeEnum.Base, ) { if (transportOptions) { @@ -180,6 +181,7 @@ export class RESTTransport implements DataConnectTransport { this.appId, this._accessToken, this._appCheckToken, + this._isUsingGen, this._callerSdkType ) ); @@ -205,6 +207,7 @@ export class RESTTransport implements DataConnectTransport { this.appId, this._accessToken, this._appCheckToken, + this._isUsingGen, this._callerSdkType ); }); diff --git a/packages/data-connect/test/unit/fetch.test.ts b/packages/data-connect/test/unit/fetch.test.ts index e9fddc03cb9..07ac1742f48 100644 --- a/packages/data-connect/test/unit/fetch.test.ts +++ b/packages/data-connect/test/unit/fetch.test.ts @@ -52,6 +52,7 @@ describe('fetch', () => { null, null, null, + false, CallerSdkTypeEnum.Base ) ).to.eventually.be.rejectedWith(message); @@ -75,6 +76,7 @@ describe('fetch', () => { null, null, null, + false, CallerSdkTypeEnum.Base ) ).to.eventually.be.rejectedWith(JSON.stringify(json)); From 1d504bfad1be9c118da1e0dd1fb479988cd79b58 Mon Sep 17 00:00:00 2001 From: Stephen Rosa Date: Fri, 31 Jan 2025 15:37:25 -0800 Subject: [PATCH 06/20] unfreeze CallerSdkTypeEnum --- common/api-review/data-connect.api.md | 4 ++-- packages/data-connect/src/network/transport/index.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/common/api-review/data-connect.api.md b/common/api-review/data-connect.api.md index 51b23096c2c..1a698c229b4 100644 --- a/common/api-review/data-connect.api.md +++ b/common/api-review/data-connect.api.md @@ -15,14 +15,14 @@ import { Provider } from '@firebase/component'; export type CallerSdkType = 'Base' | 'Generated' | 'TanstackReactCore' | 'GeneratedReact' | 'TanstackAngularCore' | 'GeneratedAngular'; // @public (undocumented) -export const CallerSdkTypeEnum: Readonly<{ +export const CallerSdkTypeEnum: { readonly Base: "Base"; readonly Generated: "Generated"; readonly TanstackReactCore: "TanstackReactCore"; readonly GeneratedReact: "GeneratedReact"; readonly TanstackAngularCore: "TanstackAngularCore"; readonly GeneratedAngular: "GeneratedAngular"; -}>; +}; // @public export function connectDataConnectEmulator(dc: DataConnect, host: string, port?: number, sslEnabled?: boolean): void; diff --git a/packages/data-connect/src/network/transport/index.ts b/packages/data-connect/src/network/transport/index.ts index ddc401877d8..13927c3200a 100644 --- a/packages/data-connect/src/network/transport/index.ts +++ b/packages/data-connect/src/network/transport/index.ts @@ -30,14 +30,14 @@ export type CallerSdkType = | 'GeneratedReact' // Generated React SDK | 'TanstackAngularCore' // Tanstack non-generated Angular SDK | 'GeneratedAngular'; // Generated Angular SDK -export const CallerSdkTypeEnum = Object.freeze({ +export const CallerSdkTypeEnum = { Base: 'Base', // Core JS SDK Generated: 'Generated', // Generated JS SDK TanstackReactCore: 'TanstackReactCore', // Tanstack non-generated React SDK GeneratedReact: 'GeneratedReact', // Tanstack non-generated Angular SDK TanstackAngularCore: 'TanstackAngularCore', // Tanstack non-generated Angular SDK GeneratedAngular: 'GeneratedAngular' // Generated Angular SDK -} as const); +} as const; /** * @internal From f45b282bd4b24009d7894bb82ab56a33b9658a3c Mon Sep 17 00:00:00 2001 From: Stephen Rosa Date: Fri, 31 Jan 2025 15:51:26 -0800 Subject: [PATCH 07/20] add checking of _isUsingGen back to custom headers logic --- packages/data-connect/src/network/fetch.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/data-connect/src/network/fetch.ts b/packages/data-connect/src/network/fetch.ts index 6a77b315ac6..bca0320996e 100644 --- a/packages/data-connect/src/network/fetch.ts +++ b/packages/data-connect/src/network/fetch.ts @@ -25,9 +25,9 @@ let connectFetch: typeof fetch | null = globalThis.fetch; export function initializeFetch(fetchImpl: typeof fetch): void { connectFetch = fetchImpl; } -function getGoogApiClientValue(_callerSdkType: CallerSdkType): string { +function getGoogApiClientValue(_isUsingGen: boolean, _callerSdkType: CallerSdkType): string { let str = 'gl-js/ fire/' + SDK_VERSION; - if (_callerSdkType === CallerSdkTypeEnum.Generated) { + if (_isUsingGen || _callerSdkType === CallerSdkTypeEnum.Generated) { str += ' js/gen'; } else if (_callerSdkType !== CallerSdkTypeEnum.Base) { str += ' js/' + _callerSdkType.toLowerCase(); @@ -54,7 +54,7 @@ export function dcFetch( } const headers: HeadersInit = { 'Content-Type': 'application/json', - 'X-Goog-Api-Client': getGoogApiClientValue(_callerSdkType) + 'X-Goog-Api-Client': getGoogApiClientValue(_isUsingGen, _callerSdkType) }; if (accessToken) { headers['X-Firebase-Auth-Token'] = accessToken; From b307ae5d7021cccba8f904f235347cb03b638215 Mon Sep 17 00:00:00 2001 From: Stephen Rosa Date: Mon, 3 Feb 2025 16:16:33 -0800 Subject: [PATCH 08/20] add call to _setCallerSdkType() on transport class --- packages/data-connect/src/api/DataConnect.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/data-connect/src/api/DataConnect.ts b/packages/data-connect/src/api/DataConnect.ts index f736464f361..0f24401001b 100644 --- a/packages/data-connect/src/api/DataConnect.ts +++ b/packages/data-connect/src/api/DataConnect.ts @@ -125,6 +125,7 @@ export class DataConnect { // @internal _setCallerSdkType(callerSdkType: CallerSdkType): void { this._callerSdkType = callerSdkType; + this._transport._setCallerSdkType(callerSdkType); } _delete(): Promise { _removeServiceInstance( From 112a60583dc10e2d77dc3c5248298f644c015abd Mon Sep 17 00:00:00 2001 From: Stephen Rosa Date: Mon, 3 Feb 2025 16:57:33 -0800 Subject: [PATCH 09/20] add tests for fetch with custom headers based on dcFetch _callerSdkType argument --- packages/data-connect/test/unit/fetch.test.ts | 93 +++++++++++++++++-- 1 file changed, 85 insertions(+), 8 deletions(-) diff --git a/packages/data-connect/test/unit/fetch.test.ts b/packages/data-connect/test/unit/fetch.test.ts index 07ac1742f48..ac9cea2a0e6 100644 --- a/packages/data-connect/test/unit/fetch.test.ts +++ b/packages/data-connect/test/unit/fetch.test.ts @@ -20,26 +20,30 @@ import chaiAsPromised from 'chai-as-promised'; import * as sinon from 'sinon'; import { dcFetch, initializeFetch } from '../../src/network/fetch'; -import { CallerSdkTypeEnum } from '../../src/network/transport'; +import { CallerSdkType, CallerSdkTypeEnum } from '../../src/network/transport'; use(chaiAsPromised); -function mockFetch(json: object): void { +function mockFetch(json: object, reject: boolean): sinon.SinonStub { const fakeFetchImpl = sinon.stub().returns( Promise.resolve({ json: () => { return Promise.resolve(json); }, - status: 401 + status: reject ? 401 : 200 } as Response) ); initializeFetch(fakeFetchImpl); + return fakeFetchImpl; } describe('fetch', () => { it('should throw an error with just the message when the server responds with an error with a message property in the body', async () => { const message = 'Failed to connect to Postgres instance'; - mockFetch({ - code: 401, - message - }); + mockFetch( + { + code: 401, + message + }, + true + ); await expect( dcFetch( 'http://localhost', @@ -63,7 +67,7 @@ describe('fetch', () => { code: 401, message1: message }; - mockFetch(json); + mockFetch(json, true); await expect( dcFetch( 'http://localhost', @@ -81,4 +85,77 @@ describe('fetch', () => { ) ).to.eventually.be.rejectedWith(JSON.stringify(json)); }); + it('should assign different values to custom headers based on the _callerSdkType argument', async () => { + const json = { + code: 200, + message1: 'success' + }; + const fakeFetchImpl = mockFetch(json, false); + + const callerSdkTypesUsed: string[] = []; + + for (const callerSdkType in CallerSdkTypeEnum) { + if ( + Object.prototype.hasOwnProperty.call(CallerSdkTypeEnum, callerSdkType) + ) { + callerSdkTypesUsed.push(callerSdkType); + await dcFetch( + 'http://localhost', + { + name: 'n', + operationName: 'n', + variables: {} + }, + {} as AbortController, + null, + null, + null, + false, + callerSdkType as CallerSdkType + ); + } + } + + // an example of the args to fetch(): + // [ + // "http://localhost", + // { + // "body": "{\"name\":\"n\",\"operationName\":\"n\",\"variables\":{}}", + // "headers": { + // "Content-Type": "application/json", + // "X-Goog-Api-Client": "gl-js/ fire/11.2.0" + // }, + // "method": "POST", + // "signal": [undefined] + // } + // ] + for (let i = 0; i < fakeFetchImpl.args.length; i++) { + const args = fakeFetchImpl.args[i]; + expect(args.length).to.equal(2); + expect(Object.prototype.hasOwnProperty.call(args[1], 'headers')).to.be + .true; + expect( + Object.prototype.hasOwnProperty.call( + args[1]['headers'], + 'X-Goog-Api-Client' + ) + ).to.be.true; + expect(typeof args[1]['headers']['X-Goog-Api-Client']).to.equal('string'); + + const xGoogApiClientValue: string = + args[1]['headers']['X-Goog-Api-Client']; + // the sdk type headers are always of the form "js/xxx", where xxx is _callerSdkType.toLower() + // when the _callerSdkType is Base, we do not set any header + // when the _callerSdkType is Generated, we use "js/gen" instead of "js/generated" + if (callerSdkTypesUsed[i] === CallerSdkTypeEnum.Base) { + expect(xGoogApiClientValue).to.not.match(RegExp(`js\/w`)); + } else if (callerSdkTypesUsed[i] === CallerSdkTypeEnum.Generated) { + expect(xGoogApiClientValue).to.match(RegExp(`js\/gen`)); + } else { + expect(xGoogApiClientValue).to.match( + RegExp(`js\/${callerSdkTypesUsed[i].toLowerCase()}`) + ); + } + } + }); }); From 7755617aba013daa3198a2e368c93d08620134d3 Mon Sep 17 00:00:00 2001 From: Stephen Rosa Date: Mon, 3 Feb 2025 17:06:59 -0800 Subject: [PATCH 10/20] add changeset for PR --- .changeset/little-news-sniff.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/little-news-sniff.md diff --git a/.changeset/little-news-sniff.md b/.changeset/little-news-sniff.md new file mode 100644 index 00000000000..d6273d4440d --- /dev/null +++ b/.changeset/little-news-sniff.md @@ -0,0 +1,5 @@ +--- +'@firebase/data-connect': minor +--- + +Add custom request headers based on type of SDK calling Core SDK From aa56521adb237e15d2effb8cac0a5352a654e0a7 Mon Sep 17 00:00:00 2001 From: Stephen Rosa Date: Mon, 3 Feb 2025 17:18:54 -0800 Subject: [PATCH 11/20] flip header assignment logic to only use js/gen if _callerSdkType not set --- packages/data-connect/src/network/fetch.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/data-connect/src/network/fetch.ts b/packages/data-connect/src/network/fetch.ts index bca0320996e..5e170b91113 100644 --- a/packages/data-connect/src/network/fetch.ts +++ b/packages/data-connect/src/network/fetch.ts @@ -27,10 +27,10 @@ export function initializeFetch(fetchImpl: typeof fetch): void { } function getGoogApiClientValue(_isUsingGen: boolean, _callerSdkType: CallerSdkType): string { let str = 'gl-js/ fire/' + SDK_VERSION; - if (_isUsingGen || _callerSdkType === CallerSdkTypeEnum.Generated) { - str += ' js/gen'; - } else if (_callerSdkType !== CallerSdkTypeEnum.Base) { + if (_callerSdkType !== CallerSdkTypeEnum.Base && _callerSdkType !== CallerSdkTypeEnum.Generated) { str += ' js/' + _callerSdkType.toLowerCase(); + } else if (_isUsingGen || _callerSdkType === CallerSdkTypeEnum.Generated) { + str += ' js/gen'; } return str; } From 3339fde53bf92c01c378cf760dd14791aaabe298 Mon Sep 17 00:00:00 2001 From: Stephen Rosa Date: Mon, 3 Feb 2025 17:32:12 -0800 Subject: [PATCH 12/20] add test for custom header values when _isUsingGen is true --- packages/data-connect/test/unit/fetch.test.ts | 64 ++++++++++++++++++- 1 file changed, 62 insertions(+), 2 deletions(-) diff --git a/packages/data-connect/test/unit/fetch.test.ts b/packages/data-connect/test/unit/fetch.test.ts index ac9cea2a0e6..f789b6ed669 100644 --- a/packages/data-connect/test/unit/fetch.test.ts +++ b/packages/data-connect/test/unit/fetch.test.ts @@ -85,7 +85,7 @@ describe('fetch', () => { ) ).to.eventually.be.rejectedWith(JSON.stringify(json)); }); - it('should assign different values to custom headers based on the _callerSdkType argument', async () => { + it('should assign different values to custom headers based on the _callerSdkType argument (_isUsingGen is false)', async () => { const json = { code: 200, message1: 'success' @@ -110,7 +110,7 @@ describe('fetch', () => { null, null, null, - false, + false, // _isUsingGen is false callerSdkType as CallerSdkType ); } @@ -158,4 +158,64 @@ describe('fetch', () => { } } }); + it('should assign custom headers based on _callerSdkType before checking to-be-deprecated _isUsingGen', async () => { + const json = { + code: 200, + message1: 'success' + }; + const fakeFetchImpl = mockFetch(json, false); + + const callerSdkTypesUsed: string[] = []; + + for (const callerSdkType in CallerSdkTypeEnum) { + if ( + Object.prototype.hasOwnProperty.call(CallerSdkTypeEnum, callerSdkType) + ) { + callerSdkTypesUsed.push(callerSdkType); + await dcFetch( + 'http://localhost', + { + name: 'n', + operationName: 'n', + variables: {} + }, + {} as AbortController, + null, + null, + null, + true, // _isUsingGen is true + callerSdkType as CallerSdkType + ); + } + } + + for (let i = 0; i < fakeFetchImpl.args.length; i++) { + const args = fakeFetchImpl.args[i]; + expect(args.length).to.equal(2); + expect(Object.prototype.hasOwnProperty.call(args[1], 'headers')).to.be + .true; + expect( + Object.prototype.hasOwnProperty.call( + args[1]['headers'], + 'X-Goog-Api-Client' + ) + ).to.be.true; + expect(typeof args[1]['headers']['X-Goog-Api-Client']).to.equal('string'); + + const xGoogApiClientValue: string = + args[1]['headers']['X-Goog-Api-Client']; + // despite _isUsingGen being true, the headers should be based on _callerSdkType + // _isUsingGen should only take precedence when _callerSdkType is "Base" + if ( + callerSdkTypesUsed[i] === CallerSdkTypeEnum.Generated || + callerSdkTypesUsed[i] === CallerSdkTypeEnum.Base + ) { + expect(xGoogApiClientValue).to.match(RegExp(`js\/gen`)); + } else { + expect(xGoogApiClientValue).to.match( + RegExp(`js\/${callerSdkTypesUsed[i].toLowerCase()}`) + ); + } + } + }); }); From b77ad49375c0fb50150826a9078777023dcdac65 Mon Sep 17 00:00:00 2001 From: Stephen Rosa Date: Mon, 3 Feb 2025 17:34:09 -0800 Subject: [PATCH 13/20] add firebase minor version bump to changeset --- .changeset/little-news-sniff.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.changeset/little-news-sniff.md b/.changeset/little-news-sniff.md index d6273d4440d..6f69b582592 100644 --- a/.changeset/little-news-sniff.md +++ b/.changeset/little-news-sniff.md @@ -1,5 +1,6 @@ --- '@firebase/data-connect': minor +'firebase': minor --- Add custom request headers based on type of SDK calling Core SDK From 261909831fa774ec704b89746d02f6623c0b69d0 Mon Sep 17 00:00:00 2001 From: Stephen Rosa Date: Mon, 3 Feb 2025 18:09:48 -0800 Subject: [PATCH 14/20] fix formatting issues --- packages/data-connect/src/network/fetch.ts | 12 +++++++++--- packages/data-connect/src/network/transport/index.ts | 2 +- packages/data-connect/src/network/transport/rest.ts | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/data-connect/src/network/fetch.ts b/packages/data-connect/src/network/fetch.ts index 5e170b91113..166422ca14c 100644 --- a/packages/data-connect/src/network/fetch.ts +++ b/packages/data-connect/src/network/fetch.ts @@ -25,11 +25,17 @@ let connectFetch: typeof fetch | null = globalThis.fetch; export function initializeFetch(fetchImpl: typeof fetch): void { connectFetch = fetchImpl; } -function getGoogApiClientValue(_isUsingGen: boolean, _callerSdkType: CallerSdkType): string { +function getGoogApiClientValue( + _isUsingGen: boolean, + _callerSdkType: CallerSdkType +): string { let str = 'gl-js/ fire/' + SDK_VERSION; - if (_callerSdkType !== CallerSdkTypeEnum.Base && _callerSdkType !== CallerSdkTypeEnum.Generated) { + if ( + _callerSdkType !== CallerSdkTypeEnum.Base && + _callerSdkType !== CallerSdkTypeEnum.Generated + ) { str += ' js/' + _callerSdkType.toLowerCase(); - } else if (_isUsingGen || _callerSdkType === CallerSdkTypeEnum.Generated) { + } else if (_isUsingGen || _callerSdkType === CallerSdkTypeEnum.Generated) { str += ' js/gen'; } return str; diff --git a/packages/data-connect/src/network/transport/index.ts b/packages/data-connect/src/network/transport/index.ts index 13927c3200a..4da8312b708 100644 --- a/packages/data-connect/src/network/transport/index.ts +++ b/packages/data-connect/src/network/transport/index.ts @@ -68,5 +68,5 @@ export type TransportClass = new ( appCheckProvider?: AppCheckTokenProvider, transportOptions?: TransportOptions, _isUsingGen?: boolean, - _callerSdkType?: CallerSdkType + _callerSdkType?: CallerSdkType ) => DataConnectTransport; diff --git a/packages/data-connect/src/network/transport/rest.ts b/packages/data-connect/src/network/transport/rest.ts index b58ef9e621a..9b24021b3d1 100644 --- a/packages/data-connect/src/network/transport/rest.ts +++ b/packages/data-connect/src/network/transport/rest.ts @@ -44,7 +44,7 @@ export class RESTTransport implements DataConnectTransport { private appCheckProvider?: AppCheckTokenProvider | undefined, transportOptions?: TransportOptions | undefined, private _isUsingGen = false, - private _callerSdkType: CallerSdkType = CallerSdkTypeEnum.Base, + private _callerSdkType: CallerSdkType = CallerSdkTypeEnum.Base ) { if (transportOptions) { if (typeof transportOptions.port === 'number') { From 1e3db015c6c38b1846048d88a2f5c3a2874108d2 Mon Sep 17 00:00:00 2001 From: Stephen Rosa Date: Tue, 4 Feb 2025 10:06:19 -0800 Subject: [PATCH 15/20] update changelog --- .changeset/little-news-sniff.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/little-news-sniff.md b/.changeset/little-news-sniff.md index 6f69b582592..79ec737d119 100644 --- a/.changeset/little-news-sniff.md +++ b/.changeset/little-news-sniff.md @@ -3,4 +3,4 @@ 'firebase': minor --- -Add custom request headers based on type of SDK calling Core SDK +Add custom request headers based on the type of SDK (JS/TS, React, Angular, etc) that's invoking Data Connect requests. This will help us understand how users interact with Data Connect when using the Web SDK. \ No newline at end of file From 08accc6c95a61afbb12857bf4ec9e670d8b97b90 Mon Sep 17 00:00:00 2001 From: Stephen Rosa Date: Tue, 4 Feb 2025 12:20:10 -0800 Subject: [PATCH 16/20] only call transport's _setCallerSdkType() if DataConnect is initialized --- packages/data-connect/src/api/DataConnect.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/data-connect/src/api/DataConnect.ts b/packages/data-connect/src/api/DataConnect.ts index 0f24401001b..023efea3859 100644 --- a/packages/data-connect/src/api/DataConnect.ts +++ b/packages/data-connect/src/api/DataConnect.ts @@ -125,7 +125,9 @@ export class DataConnect { // @internal _setCallerSdkType(callerSdkType: CallerSdkType): void { this._callerSdkType = callerSdkType; - this._transport._setCallerSdkType(callerSdkType); + if (this._initialized) { + this._transport._setCallerSdkType(callerSdkType); + } } _delete(): Promise { _removeServiceInstance( From 784597c340629160d635e8edfba4a01a6bab1cce Mon Sep 17 00:00:00 2001 From: Stephen Rosa Date: Tue, 4 Feb 2025 15:25:33 -0800 Subject: [PATCH 17/20] simplify tests --- packages/data-connect/test/unit/fetch.test.ts | 108 ++++++------------ 1 file changed, 36 insertions(+), 72 deletions(-) diff --git a/packages/data-connect/test/unit/fetch.test.ts b/packages/data-connect/test/unit/fetch.test.ts index f789b6ed669..3f3b695baf4 100644 --- a/packages/data-connect/test/unit/fetch.test.ts +++ b/packages/data-connect/test/unit/fetch.test.ts @@ -113,48 +113,25 @@ describe('fetch', () => { false, // _isUsingGen is false callerSdkType as CallerSdkType ); - } - } - // an example of the args to fetch(): - // [ - // "http://localhost", - // { - // "body": "{\"name\":\"n\",\"operationName\":\"n\",\"variables\":{}}", - // "headers": { - // "Content-Type": "application/json", - // "X-Goog-Api-Client": "gl-js/ fire/11.2.0" - // }, - // "method": "POST", - // "signal": [undefined] - // } - // ] - for (let i = 0; i < fakeFetchImpl.args.length; i++) { - const args = fakeFetchImpl.args[i]; - expect(args.length).to.equal(2); - expect(Object.prototype.hasOwnProperty.call(args[1], 'headers')).to.be - .true; - expect( - Object.prototype.hasOwnProperty.call( - args[1]['headers'], - 'X-Goog-Api-Client' - ) - ).to.be.true; - expect(typeof args[1]['headers']['X-Goog-Api-Client']).to.equal('string'); - - const xGoogApiClientValue: string = - args[1]['headers']['X-Goog-Api-Client']; - // the sdk type headers are always of the form "js/xxx", where xxx is _callerSdkType.toLower() - // when the _callerSdkType is Base, we do not set any header - // when the _callerSdkType is Generated, we use "js/gen" instead of "js/generated" - if (callerSdkTypesUsed[i] === CallerSdkTypeEnum.Base) { - expect(xGoogApiClientValue).to.not.match(RegExp(`js\/w`)); - } else if (callerSdkTypesUsed[i] === CallerSdkTypeEnum.Generated) { - expect(xGoogApiClientValue).to.match(RegExp(`js\/gen`)); - } else { - expect(xGoogApiClientValue).to.match( - RegExp(`js\/${callerSdkTypesUsed[i].toLowerCase()}`) - ); + let expectedHeaderRegex: RegExp; + if (callerSdkType === CallerSdkTypeEnum.Base) { + // should not contain any "js/xxx" substring + expectedHeaderRegex = RegExp(`^((?!js\/\w).)*$`); + } else if (callerSdkType === CallerSdkTypeEnum.Generated) { + expectedHeaderRegex = RegExp(`js\/gen`); + } else { + expectedHeaderRegex = RegExp(`js\/${callerSdkType.toLowerCase()}`); + } + expect( + fakeFetchImpl.calledWithMatch( + 'http://localhost', + sinon.match.hasNested( + 'headers.X-Goog-Api-Client', + sinon.match(expectedHeaderRegex) + ) + ) + ).to.be.true; } } }); @@ -165,13 +142,10 @@ describe('fetch', () => { }; const fakeFetchImpl = mockFetch(json, false); - const callerSdkTypesUsed: string[] = []; - for (const callerSdkType in CallerSdkTypeEnum) { if ( Object.prototype.hasOwnProperty.call(CallerSdkTypeEnum, callerSdkType) ) { - callerSdkTypesUsed.push(callerSdkType); await dcFetch( 'http://localhost', { @@ -186,35 +160,25 @@ describe('fetch', () => { true, // _isUsingGen is true callerSdkType as CallerSdkType ); - } - } - - for (let i = 0; i < fakeFetchImpl.args.length; i++) { - const args = fakeFetchImpl.args[i]; - expect(args.length).to.equal(2); - expect(Object.prototype.hasOwnProperty.call(args[1], 'headers')).to.be - .true; - expect( - Object.prototype.hasOwnProperty.call( - args[1]['headers'], - 'X-Goog-Api-Client' - ) - ).to.be.true; - expect(typeof args[1]['headers']['X-Goog-Api-Client']).to.equal('string'); - const xGoogApiClientValue: string = - args[1]['headers']['X-Goog-Api-Client']; - // despite _isUsingGen being true, the headers should be based on _callerSdkType - // _isUsingGen should only take precedence when _callerSdkType is "Base" - if ( - callerSdkTypesUsed[i] === CallerSdkTypeEnum.Generated || - callerSdkTypesUsed[i] === CallerSdkTypeEnum.Base - ) { - expect(xGoogApiClientValue).to.match(RegExp(`js\/gen`)); - } else { - expect(xGoogApiClientValue).to.match( - RegExp(`js\/${callerSdkTypesUsed[i].toLowerCase()}`) - ); + let expectedHeaderRegex: RegExp; + if ( + callerSdkType === CallerSdkTypeEnum.Generated || + callerSdkType === CallerSdkTypeEnum.Base + ) { + expectedHeaderRegex = RegExp(`js\/gen`); + } else { + expectedHeaderRegex = RegExp(`js\/${callerSdkType.toLowerCase()}`); + } + expect( + fakeFetchImpl.calledWithMatch( + 'http://localhost', + sinon.match.hasNested( + 'headers.X-Goog-Api-Client', + sinon.match(expectedHeaderRegex) + ) + ) + ).to.be.true; } } }); From 9efd04092c9e713e405793e89c33dd6d9b655d7d Mon Sep 17 00:00:00 2001 From: Stephen Rosa Date: Tue, 4 Feb 2025 16:12:35 -0800 Subject: [PATCH 18/20] add comments explaining funky eslint logic --- packages/data-connect/test/unit/fetch.test.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/data-connect/test/unit/fetch.test.ts b/packages/data-connect/test/unit/fetch.test.ts index 3f3b695baf4..5dc9c4b4e77 100644 --- a/packages/data-connect/test/unit/fetch.test.ts +++ b/packages/data-connect/test/unit/fetch.test.ts @@ -92,13 +92,11 @@ describe('fetch', () => { }; const fakeFetchImpl = mockFetch(json, false); - const callerSdkTypesUsed: string[] = []; - for (const callerSdkType in CallerSdkTypeEnum) { + // this check is done to follow the best practices outlined by the "guard-for-in" eslint rule if ( Object.prototype.hasOwnProperty.call(CallerSdkTypeEnum, callerSdkType) ) { - callerSdkTypesUsed.push(callerSdkType); await dcFetch( 'http://localhost', { @@ -143,6 +141,7 @@ describe('fetch', () => { const fakeFetchImpl = mockFetch(json, false); for (const callerSdkType in CallerSdkTypeEnum) { + // this check is done to follow the best practices outlined by the "guard-for-in" eslint rule if ( Object.prototype.hasOwnProperty.call(CallerSdkTypeEnum, callerSdkType) ) { From 595fee331ea2bd3770bce5307a9e5f903f6be8ae Mon Sep 17 00:00:00 2001 From: Stephen Rosa Date: Tue, 4 Feb 2025 16:13:33 -0800 Subject: [PATCH 19/20] remove new, useless @internal comments --- packages/data-connect/src/api/DataConnect.ts | 1 - packages/data-connect/src/network/transport/index.ts | 1 - packages/data-connect/src/network/transport/rest.ts | 1 - 3 files changed, 3 deletions(-) diff --git a/packages/data-connect/src/api/DataConnect.ts b/packages/data-connect/src/api/DataConnect.ts index 023efea3859..dc170809143 100644 --- a/packages/data-connect/src/api/DataConnect.ts +++ b/packages/data-connect/src/api/DataConnect.ts @@ -122,7 +122,6 @@ export class DataConnect { this._isUsingGeneratedSdk = true; } } - // @internal _setCallerSdkType(callerSdkType: CallerSdkType): void { this._callerSdkType = callerSdkType; if (this._initialized) { diff --git a/packages/data-connect/src/network/transport/index.ts b/packages/data-connect/src/network/transport/index.ts index 4da8312b708..8b106b4d636 100644 --- a/packages/data-connect/src/network/transport/index.ts +++ b/packages/data-connect/src/network/transport/index.ts @@ -53,7 +53,6 @@ export interface DataConnectTransport { ): Promise<{ data: T; errors: Error[] }>; useEmulator(host: string, port?: number, sslEnabled?: boolean): void; onTokenChanged: (token: string | null) => void; - // @internal _setCallerSdkType(callerSdkType: CallerSdkType): void; } diff --git a/packages/data-connect/src/network/transport/rest.ts b/packages/data-connect/src/network/transport/rest.ts index 9b24021b3d1..0663bc026db 100644 --- a/packages/data-connect/src/network/transport/rest.ts +++ b/packages/data-connect/src/network/transport/rest.ts @@ -214,7 +214,6 @@ export class RESTTransport implements DataConnectTransport { return taskResult; }; - // @internal _setCallerSdkType(callerSdkType: CallerSdkType): void { this._callerSdkType = callerSdkType; } From 99cced0e2c1aa6159a9c7884e80c7fad270a133d Mon Sep 17 00:00:00 2001 From: Stephen Rosa Date: Tue, 4 Feb 2025 16:55:40 -0800 Subject: [PATCH 20/20] convert regex strings to regex literals --- packages/data-connect/test/unit/fetch.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/data-connect/test/unit/fetch.test.ts b/packages/data-connect/test/unit/fetch.test.ts index 5dc9c4b4e77..599260f8b10 100644 --- a/packages/data-connect/test/unit/fetch.test.ts +++ b/packages/data-connect/test/unit/fetch.test.ts @@ -115,9 +115,9 @@ describe('fetch', () => { let expectedHeaderRegex: RegExp; if (callerSdkType === CallerSdkTypeEnum.Base) { // should not contain any "js/xxx" substring - expectedHeaderRegex = RegExp(`^((?!js\/\w).)*$`); + expectedHeaderRegex = RegExp(/^((?!js\/\w).)*$/); } else if (callerSdkType === CallerSdkTypeEnum.Generated) { - expectedHeaderRegex = RegExp(`js\/gen`); + expectedHeaderRegex = RegExp(/js\/gen/); } else { expectedHeaderRegex = RegExp(`js\/${callerSdkType.toLowerCase()}`); }