From d5d1470e81fda712527cf61c8c71c381da584249 Mon Sep 17 00:00:00 2001 From: Kyle Fang Date: Thu, 13 Aug 2020 12:20:27 +0800 Subject: [PATCH 1/6] fix: observer compatibility in firestore --- packages/firestore/lib/utils/index.js | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/packages/firestore/lib/utils/index.js b/packages/firestore/lib/utils/index.js index abd4752a5b..b1da65575a 100644 --- a/packages/firestore/lib/utils/index.js +++ b/packages/firestore/lib/utils/index.js @@ -141,6 +141,10 @@ export function parseSetOptions(options) { // }; // } +function isPartialObserver(input) { + return isFunction(input.next) || isFunction(input.error) || isFunction(input.complete); +} + export function parseSnapshotArgs(args) { if (args.length === 0) { throw new Error('expected at least one argument.'); @@ -174,19 +178,20 @@ export function parseSnapshotArgs(args) { /** * .onSnapshot({ complete: () => {}, error: (e) => {}, next: (snapshot) => {} }) */ - if (isObject(args[0]) && args[0].includeMetadataChanges === undefined) { - if (args[0].error) { - onError = args[0].error; + if (isObject(args[0]) && isPartialObserver(args[0])) { + const observer = args[0]; + if (observer.error) { + onError = observer.error.bind(observer); } - if (args[0].next) { - onNext = args[0].next; + if (observer.next) { + onNext = observer.next.bind(observer); } } /** * .onSnapshot(SnapshotListenOptions, ... */ - if (isObject(args[0]) && args[0].includeMetadataChanges !== undefined) { + if (isObject(args[0]) && !isPartialObserver(args[0])) { snapshotListenOptions.includeMetadataChanges = args[0].includeMetadataChanges; if (isFunction(args[1])) { /** @@ -208,11 +213,12 @@ export function parseSnapshotArgs(args) { /** * .onSnapshot(SnapshotListenOptions, { complete: () => {}, error: (e) => {}, next: (snapshot) => {} }); */ - if (isFunction(args[1].error)) { - onError = args[1].error; + const observer = args[1]; + if (isFunction(observer.error)) { + onError = observer.error.bind(observer); } - if (isFunction(args[1].next)) { - onNext = args[1].next; + if (isFunction(observer.next)) { + onNext = observer.next.bind(observer); } } } From b0eb68130618bbdf6358c135d7c393e46fe1756d Mon Sep 17 00:00:00 2001 From: Kyle Fang Date: Thu, 13 Aug 2020 12:37:05 +0800 Subject: [PATCH 2/6] fix: add observer compatibility to auth APIs --- packages/auth/lib/index.d.ts | 8 +++++--- packages/auth/lib/index.js | 15 ++++++++++++--- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/packages/auth/lib/index.d.ts b/packages/auth/lib/index.d.ts index 4eb53171d6..20673d73eb 100644 --- a/packages/auth/lib/index.d.ts +++ b/packages/auth/lib/index.d.ts @@ -1254,7 +1254,7 @@ export namespace FirebaseAuthTypes { * * @param listener A listener function which triggers when auth state changed (for example signing out). */ - onAuthStateChanged(listener: AuthListenerCallback): () => void; + onAuthStateChanged(listener: CallbackOrObserver): () => void; /** * Listen for changes in ID token. @@ -1276,7 +1276,7 @@ export namespace FirebaseAuthTypes { * * @param listener A listener function which triggers when the users ID token changes. */ - onIdTokenChanged(listener: AuthListenerCallback): () => void; + onIdTokenChanged(listener: CallbackOrObserver): () => void; /** * Adds a listener to observe changes to the User object. This is a superset of everything from @@ -1302,7 +1302,7 @@ export namespace FirebaseAuthTypes { * @react-native-firebase * @param listener A listener function which triggers when the users data changes. */ - onUserChanged(listener: AuthListenerCallback): () => void; + onUserChanged(listener: CallbackOrObserver): () => void; /** * Signs the user out. @@ -1634,6 +1634,8 @@ export namespace FirebaseAuthTypes { } } +type CallbackOrObserver any> = T | { next: T }; + declare module '@react-native-firebase/auth' { // tslint:disable-next-line:no-duplicate-imports required otherwise doesn't work import { ReactNativeFirebase } from '@react-native-firebase/app'; diff --git a/packages/auth/lib/index.js b/packages/auth/lib/index.js index 24110ce978..d020699dc6 100644 --- a/packages/auth/lib/index.js +++ b/packages/auth/lib/index.js @@ -129,7 +129,14 @@ class FirebaseAuthModule extends FirebaseModule { }; } - onAuthStateChanged(listener) { + _parseListener(listenerOrObserver) { + return typeof listenerOrObserver === 'object' + ? listenerOrObserver.next.bind(listenerOrObserver) + : listenerOrObserver; + } + + onAuthStateChanged(listenerOrObserver) { + const listener = this._parseListener(listenerOrObserver); const subscription = this.emitter.addListener( this.eventNameForApp('onAuthStateChanged'), listener, @@ -143,7 +150,8 @@ class FirebaseAuthModule extends FirebaseModule { return () => subscription.remove(); } - onIdTokenChanged(listener) { + onIdTokenChanged(listenerOrObserver) { + const listener = this._parseListener(listenerOrObserver); const subscription = this.emitter.addListener( this.eventNameForApp('onIdTokenChanged'), listener, @@ -157,7 +165,8 @@ class FirebaseAuthModule extends FirebaseModule { return () => subscription.remove(); } - onUserChanged(listener) { + onUserChanged(listenerOrObserver) { + const listener = this._parseListener(listenerOrObserver); const subscription = this.emitter.addListener(this.eventNameForApp('onUserChanged'), listener); if (this._authResult) { Promise.resolve().then(() => { From ed417e5853c7b51470afa20c5eab5814ef1e3190 Mon Sep 17 00:00:00 2001 From: Kyle Fang Date: Thu, 13 Aug 2020 13:19:39 +0800 Subject: [PATCH 3/6] fix: boolean --- packages/firestore/lib/utils/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/lib/utils/index.js b/packages/firestore/lib/utils/index.js index b1da65575a..ee1f660cb1 100644 --- a/packages/firestore/lib/utils/index.js +++ b/packages/firestore/lib/utils/index.js @@ -192,7 +192,7 @@ export function parseSnapshotArgs(args) { * .onSnapshot(SnapshotListenOptions, ... */ if (isObject(args[0]) && !isPartialObserver(args[0])) { - snapshotListenOptions.includeMetadataChanges = args[0].includeMetadataChanges; + snapshotListenOptions.includeMetadataChanges = Boolean(args[0].includeMetadataChanges); if (isFunction(args[1])) { /** * .onSnapshot(SnapshotListenOptions, Function); From e942d7de19d6a9319679a6634b396f5ccf180c06 Mon Sep 17 00:00:00 2001 From: Kyle Fang Date: Fri, 14 Aug 2020 13:21:57 +0800 Subject: [PATCH 4/6] fix: tests --- packages/firestore/lib/utils/index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/firestore/lib/utils/index.js b/packages/firestore/lib/utils/index.js index ee1f660cb1..7c7a071bfa 100644 --- a/packages/firestore/lib/utils/index.js +++ b/packages/firestore/lib/utils/index.js @@ -192,7 +192,8 @@ export function parseSnapshotArgs(args) { * .onSnapshot(SnapshotListenOptions, ... */ if (isObject(args[0]) && !isPartialObserver(args[0])) { - snapshotListenOptions.includeMetadataChanges = Boolean(args[0].includeMetadataChanges); + snapshotListenOptions.includeMetadataChanges = + args[0].includeMetadataChanges == null ? false : args[0].includeMetadataChanges; if (isFunction(args[1])) { /** * .onSnapshot(SnapshotListenOptions, Function); From c8cc169fb966286062dcba14550fd02a9d2d4f69 Mon Sep 17 00:00:00 2001 From: Kyle Fang Date: Fri, 14 Aug 2020 14:56:47 +0800 Subject: [PATCH 5/6] test: add auth test on onAuthStateChange also fix onSnapshot test --- packages/auth/e2e/auth.e2e.js | 36 +++++++++++++++++++++++++++ packages/firestore/lib/utils/index.js | 19 ++++++++------ 2 files changed, 47 insertions(+), 8 deletions(-) diff --git a/packages/auth/e2e/auth.e2e.js b/packages/auth/e2e/auth.e2e.js index c30d62c950..ad89059df8 100644 --- a/packages/auth/e2e/auth.e2e.js +++ b/packages/auth/e2e/auth.e2e.js @@ -187,6 +187,42 @@ describe('auth()', () => { unsubscribe(); }); + it('accept observer instead callback as well', async () => { + await firebase.auth().signInAnonymously(); + + await Utils.sleep(50); + + // Test + const observer = { + next(user) { + // Test this access + this.onNext(); + this.user = user + }, + }; + + let unsubscribe; + await new Promise(resolve => { + observer.onNext = resolve; + unsubscribe = firebase.auth().onAuthStateChanged(observer); + }); + should.exist(observer.user); + + // Sign out + + await firebase.auth().signOut(); + + // Assertions + + await Utils.sleep(50); + + should.not.exist(observer.user); + + // Tear down + + unsubscribe(); + }); + it('stops listening when unsubscribed', async () => { await firebase.auth().signInAnonymously(); diff --git a/packages/firestore/lib/utils/index.js b/packages/firestore/lib/utils/index.js index 7c7a071bfa..9e0154fd3d 100644 --- a/packages/firestore/lib/utils/index.js +++ b/packages/firestore/lib/utils/index.js @@ -142,7 +142,10 @@ export function parseSetOptions(options) { // } function isPartialObserver(input) { - return isFunction(input.next) || isFunction(input.error) || isFunction(input.complete); + if (input == null) { + return false; + } + return input.next != null || input.error != null || input.complete != null; } export function parseSnapshotArgs(args) { @@ -181,10 +184,10 @@ export function parseSnapshotArgs(args) { if (isObject(args[0]) && isPartialObserver(args[0])) { const observer = args[0]; if (observer.error) { - onError = observer.error.bind(observer); + onError = isFunction(observer.error) ? observer.error.bind(observer) : observer.error; } if (observer.next) { - onNext = observer.next.bind(observer); + onNext = isFunction(observer.next) ? observer.next.bind(observer) : observer.next; } } @@ -210,16 +213,16 @@ export function parseSnapshotArgs(args) { */ callback = args[1]; } - } else if (isObject(args[1])) { + } else if (isPartialObserver(args[1])) { /** * .onSnapshot(SnapshotListenOptions, { complete: () => {}, error: (e) => {}, next: (snapshot) => {} }); */ const observer = args[1]; - if (isFunction(observer.error)) { - onError = observer.error.bind(observer); + if (observer.error) { + onError = isFunction(observer.error) ? observer.error.bind(observer) : observer.error; } - if (isFunction(observer.next)) { - onNext = observer.next.bind(observer); + if (observer.next) { + onNext = isFunction(observer.next) ? observer.next.bind(observer) : observer.next; } } } From 8d20f3dcd1fc9020de7e1984432d45f6c95d7e8b Mon Sep 17 00:00:00 2001 From: Kyle Fang Date: Fri, 14 Aug 2020 15:10:12 +0800 Subject: [PATCH 6/6] fix: lint --- packages/auth/e2e/auth.e2e.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/auth/e2e/auth.e2e.js b/packages/auth/e2e/auth.e2e.js index ad89059df8..b6820bf001 100644 --- a/packages/auth/e2e/auth.e2e.js +++ b/packages/auth/e2e/auth.e2e.js @@ -197,7 +197,7 @@ describe('auth()', () => { next(user) { // Test this access this.onNext(); - this.user = user + this.user = user; }, };