From 39fef1f416f70a84e0364e8852e887b36d0c6483 Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Tue, 16 Jul 2024 13:28:06 -0700 Subject: [PATCH 1/4] Handle JSDOM case in app-compat checks --- .changeset/spicy-dragons-pay.md | 5 +++++ packages/app-compat/src/index.ts | 7 ++++--- packages/util/src/environment.ts | 3 +++ 3 files changed, 12 insertions(+), 3 deletions(-) create mode 100644 .changeset/spicy-dragons-pay.md diff --git a/.changeset/spicy-dragons-pay.md b/.changeset/spicy-dragons-pay.md new file mode 100644 index 00000000000..799f0ff0890 --- /dev/null +++ b/.changeset/spicy-dragons-pay.md @@ -0,0 +1,5 @@ +--- +'@firebase/app-compat': patch +--- + +Handle JSDOM case in app-compat checks. diff --git a/packages/app-compat/src/index.ts b/packages/app-compat/src/index.ts index 42b5f5a154a..afed01069ec 100644 --- a/packages/app-compat/src/index.ts +++ b/packages/app-compat/src/index.ts @@ -16,21 +16,22 @@ */ import { FirebaseNamespace } from './public-types'; -import { isBrowser } from '@firebase/util'; +import { isBrowser, getGlobal } from '@firebase/util'; import { firebase as firebaseNamespace } from './firebaseNamespace'; import { logger } from './logger'; import { registerCoreComponents } from './registerCoreComponents'; // Firebase Lite detection // eslint-disable-next-line @typescript-eslint/no-explicit-any -if (isBrowser() && (self as any).firebase !== undefined) { +if (isBrowser() && (getGlobal() as any).firebase !== undefined) { logger.warn(` Warning: Firebase is already defined in the global scope. Please make sure Firebase library is only loaded once. `); // eslint-disable-next-line - const sdkVersion = ((self as any).firebase as FirebaseNamespace).SDK_VERSION; + const sdkVersion = ((getGlobal() as any).firebase as FirebaseNamespace) + .SDK_VERSION; if (sdkVersion && sdkVersion.indexOf('LITE') >= 0) { logger.warn(` Warning: You are trying to load Firebase while using Firebase Performance standalone script. diff --git a/packages/util/src/environment.ts b/packages/util/src/environment.ts index 30fc661bfc6..11eb26755db 100644 --- a/packages/util/src/environment.ts +++ b/packages/util/src/environment.ts @@ -80,6 +80,9 @@ export function isNode(): boolean { /** * Detect Browser Environment + * Note: This will return true for JSDOM (e.g. Jest) as it is mimicking + * a browser, and should not lead to assuming all browser APIs are + * available. */ export function isBrowser(): boolean { return typeof window !== 'undefined' || isWebWorker(); From cff12061d4f53b3920af6646d7347afb690d122f Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Fri, 19 Jul 2024 10:14:54 -0700 Subject: [PATCH 2/4] update changeset comment --- .changeset/spicy-dragons-pay.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/spicy-dragons-pay.md b/.changeset/spicy-dragons-pay.md index 799f0ff0890..280406da0f0 100644 --- a/.changeset/spicy-dragons-pay.md +++ b/.changeset/spicy-dragons-pay.md @@ -2,4 +2,4 @@ '@firebase/app-compat': patch --- -Handle JSDOM case in app-compat checks. +Properly handle the case in app-compat checks where `window` exists but `self` does not. (This occurs in Ionic Stencil's Jest preset.) From 3128ce706348fe76a8376bb4acbb77e26c50665d Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Fri, 19 Jul 2024 10:39:26 -0700 Subject: [PATCH 3/4] just do window --- packages/app-compat/src/index.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/app-compat/src/index.ts b/packages/app-compat/src/index.ts index afed01069ec..8d4a7834a7e 100644 --- a/packages/app-compat/src/index.ts +++ b/packages/app-compat/src/index.ts @@ -16,22 +16,27 @@ */ import { FirebaseNamespace } from './public-types'; -import { isBrowser, getGlobal } from '@firebase/util'; +import { isBrowser } from '@firebase/util'; import { firebase as firebaseNamespace } from './firebaseNamespace'; import { logger } from './logger'; import { registerCoreComponents } from './registerCoreComponents'; +declare global { + interface Window { + firebase: FirebaseNamespace; + } +} + // Firebase Lite detection // eslint-disable-next-line @typescript-eslint/no-explicit-any -if (isBrowser() && (getGlobal() as any).firebase !== undefined) { +if (isBrowser() && window.firebase !== undefined) { logger.warn(` Warning: Firebase is already defined in the global scope. Please make sure Firebase library is only loaded once. `); // eslint-disable-next-line - const sdkVersion = ((getGlobal() as any).firebase as FirebaseNamespace) - .SDK_VERSION; + const sdkVersion = (window.firebase as FirebaseNamespace).SDK_VERSION; if (sdkVersion && sdkVersion.indexOf('LITE') >= 0) { logger.warn(` Warning: You are trying to load Firebase while using Firebase Performance standalone script. From 77fb06802dc4f3b339cdd4068f5cc19eb8d5bde8 Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Fri, 19 Jul 2024 10:40:45 -0700 Subject: [PATCH 4/4] update comment --- packages/util/src/environment.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/util/src/environment.ts b/packages/util/src/environment.ts index 11eb26755db..998000308c3 100644 --- a/packages/util/src/environment.ts +++ b/packages/util/src/environment.ts @@ -80,8 +80,8 @@ export function isNode(): boolean { /** * Detect Browser Environment - * Note: This will return true for JSDOM (e.g. Jest) as it is mimicking - * a browser, and should not lead to assuming all browser APIs are + * Note: This will return true for certain test frameworks that are incompletely + * mimicking a browser, and should not lead to assuming all browser APIs are * available. */ export function isBrowser(): boolean {