From 348808ed52fc5702f2246b412e415e10e452a8b7 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Wed, 20 Nov 2024 15:20:39 +0000 Subject: [PATCH 01/29] chore(firestore): deprecation warning for v8 API ahead of future major release --- packages/app/lib/FirebaseApp.js | 4 ++-- packages/app/lib/common/index.js | 8 ++++++-- packages/firestore/lib/FirestoreQuery.js | 25 +++++++++++++++++++----- packages/firestore/lib/modular/query.js | 18 ++++++++--------- 4 files changed, 37 insertions(+), 18 deletions(-) diff --git a/packages/app/lib/FirebaseApp.js b/packages/app/lib/FirebaseApp.js index be7ea00120..449829a863 100644 --- a/packages/app/lib/FirebaseApp.js +++ b/packages/app/lib/FirebaseApp.js @@ -62,7 +62,7 @@ export default class FirebaseApp { extendApp(extendedProps) { // this method has no modular alternative, send true for param 'noAlternative' - warnIfNotModularCall(arguments, '', true); + warnIfNotModularCall(arguments); this._checkDestroyed(); Object.assign(this, extendedProps); } @@ -75,7 +75,7 @@ export default class FirebaseApp { toString() { // this method has no modular alternative, send true for param 'noAlternative' - warnIfNotModularCall(arguments, '', true); + warnIfNotModularCall(arguments); return this.name; } } diff --git a/packages/app/lib/common/index.js b/packages/app/lib/common/index.js index 68dcc755ed..2f24a5c14c 100644 --- a/packages/app/lib/common/index.js +++ b/packages/app/lib/common/index.js @@ -105,7 +105,11 @@ export function tryJSONStringify(data) { export const MODULAR_DEPRECATION_ARG = 'react-native-firebase-modular-method-call'; -export function warnIfNotModularCall(args, replacementMethodName, noAlternative) { +export function filterModularArgument(list) { + return list.filter(arg => arg !== MODULAR_DEPRECATION_ARG); +} + +export function warnIfNotModularCall(args, replacementMethodName = '') { for (let i = 0; i < args.length; i++) { if (args[i] === MODULAR_DEPRECATION_ARG) { return; @@ -115,7 +119,7 @@ export function warnIfNotModularCall(args, replacementMethodName, noAlternative) 'This v8 method is deprecated and will be removed in the next major release ' + 'as part of move to match Firebase Web modular v9 SDK API.'; - if (!noAlternative) { + if (replacementMethodName.length > 0) { message += ` Please use \`${replacementMethodName}\` instead.`; } diff --git a/packages/firestore/lib/FirestoreQuery.js b/packages/firestore/lib/FirestoreQuery.js index cd5f6664d6..c08dc662c0 100644 --- a/packages/firestore/lib/FirestoreQuery.js +++ b/packages/firestore/lib/FirestoreQuery.js @@ -21,6 +21,8 @@ import { isObject, isString, isUndefined, + filterModularArgument, + warnIfNotModularCall, } from '@react-native-firebase/app/lib/common'; import NativeError from '@react-native-firebase/app/lib/internal/NativeFirebaseError'; import { FirestoreAggregateQuery } from './FirestoreAggregate'; @@ -133,6 +135,7 @@ export default class FirestoreQuery { } count() { + warnIfNotModularCall(arguments, 'getCountFromServer()'); return new FirestoreAggregateQuery( this._firestore, this, @@ -142,28 +145,32 @@ export default class FirestoreQuery { } countFromServer() { + // deprecation warning called in count() return this.count(); } endAt(docOrField, ...fields) { + warnIfNotModularCall(arguments, 'endAt()'); return new FirestoreQuery( this._firestore, this._collectionPath, - this._handleQueryCursor('endAt', docOrField, fields), + this._handleQueryCursor('endAt', docOrField, filterModularArgument(fields)), this._queryName, ); } endBefore(docOrField, ...fields) { + warnIfNotModularCall(arguments, 'endBefore()'); return new FirestoreQuery( this._firestore, this._collectionPath, - this._handleQueryCursor('endBefore', docOrField, fields), + this._handleQueryCursor('endBefore', docOrField, filterModularArgument(fields)), this._queryName, ); } get(options) { + warnIfNotModularCall(arguments, 'getDocs()'); if (!isUndefined(options) && !isObject(options)) { throw new Error( "firebase.firestore().collection().get(*) 'options' must be an object is provided.", @@ -210,6 +217,7 @@ export default class FirestoreQuery { } isEqual(other) { + warnIfNotModularCall(arguments); if (!(other instanceof FirestoreQuery)) { throw new Error( "firebase.firestore().collection().isEqual(*) 'other' expected a Query instance.", @@ -242,6 +250,7 @@ export default class FirestoreQuery { } limit(limit) { + warnIfNotModularCall(arguments, 'limit()'); if (this._modifiers.isValidLimit(limit)) { throw new Error( "firebase.firestore().collection().limit(*) 'limit' must be a positive integer value.", @@ -254,6 +263,7 @@ export default class FirestoreQuery { } limitToLast(limitToLast) { + warnIfNotModularCall(arguments, 'limitToLast()'); if (this._modifiers.isValidLimitToLast(limitToLast)) { throw new Error( "firebase.firestore().collection().limitToLast(*) 'limitToLast' must be a positive integer value.", @@ -266,6 +276,7 @@ export default class FirestoreQuery { } onSnapshot(...args) { + warnIfNotModularCall(arguments, 'onSnapshot()'); let snapshotListenOptions; let callback; let onNext; @@ -274,7 +285,7 @@ export default class FirestoreQuery { this._modifiers.validatelimitToLast(); try { - const options = parseSnapshotArgs(args); + const options = parseSnapshotArgs(filterModularArgument(args)); snapshotListenOptions = options.snapshotListenOptions; callback = options.callback; onNext = options.onNext; @@ -342,6 +353,7 @@ export default class FirestoreQuery { } orderBy(fieldPath, directionStr) { + warnIfNotModularCall(arguments, 'orderBy()'); if (!isString(fieldPath) && !(fieldPath instanceof FirestoreFieldPath)) { throw new Error( "firebase.firestore().collection().orderBy(*) 'fieldPath' must be a string or instance of FieldPath.", @@ -390,24 +402,27 @@ export default class FirestoreQuery { } startAfter(docOrField, ...fields) { + warnIfNotModularCall(arguments, 'startAfter()'); return new FirestoreQuery( this._firestore, this._collectionPath, - this._handleQueryCursor('startAfter', docOrField, fields), + this._handleQueryCursor('startAfter', docOrField, filterModularArgument(fields)), this._queryName, ); } startAt(docOrField, ...fields) { + warnIfNotModularCall(arguments, 'startAt()'); return new FirestoreQuery( this._firestore, this._collectionPath, - this._handleQueryCursor('startAt', docOrField, fields), + this._handleQueryCursor('startAt', docOrField, filterModularArgument(fields)), this._queryName, ); } where(fieldPathOrFilter, opStr, value) { + warnIfNotModularCall(arguments, 'where()'); if ( !isString(fieldPathOrFilter) && !(fieldPathOrFilter instanceof FirestoreFieldPath) && diff --git a/packages/firestore/lib/modular/query.js b/packages/firestore/lib/modular/query.js index 6d906cec83..943ca0906c 100644 --- a/packages/firestore/lib/modular/query.js +++ b/packages/firestore/lib/modular/query.js @@ -15,7 +15,7 @@ * @typedef {import('./query').QueryOrderByConstraint} QueryOrderByConstraint * @typedef {import('./query').QueryStartAtConstraint} QueryStartAtConstraint */ - +import { MODULAR_DEPRECATION_ARG } from '@react-native-firebase/app/lib/common'; import { _Filter, Filter } from '../FirestoreFilter'; /** @@ -28,7 +28,7 @@ class QueryConstraint { } _apply(query) { - return query[this.type].apply(query, this._args); + return query[this.type].apply(query, this._args, MODULAR_DEPRECATION_ARG); } } @@ -174,7 +174,7 @@ export function limitToLast(limit) { * @returns {Promise} */ export function getDoc(reference) { - return reference.get({ source: 'default' }); + return reference.get.call(reference, { source: 'default' }, MODULAR_DEPRECATION_ARG); } /** @@ -182,7 +182,7 @@ export function getDoc(reference) { * @returns {Promise} */ export function getDocFromCache(reference) { - return reference.get({ source: 'cache' }); + return reference.get.call(reference, { source: 'cache' }, MODULAR_DEPRECATION_ARG); } /** @@ -190,7 +190,7 @@ export function getDocFromCache(reference) { * @returns {Promise} */ export function getDocFromServer(reference) { - return reference.get({ source: 'server' }); + return reference.get.call(reference, { source: 'server' }, MODULAR_DEPRECATION_ARG); } /** @@ -198,7 +198,7 @@ export function getDocFromServer(reference) { * @returns {Promise} */ export function getDocs(query) { - return query.get({ source: 'default' }); + return query.get.call(reference, { source: 'default' }, MODULAR_DEPRECATION_ARG); } /** @@ -206,7 +206,7 @@ export function getDocs(query) { * @returns {Promise} */ export function getDocsFromCache(query) { - return query.get({ source: 'cache' }); + return query.get.call(reference, { source: 'cache' }, MODULAR_DEPRECATION_ARG); } /** @@ -214,7 +214,7 @@ export function getDocsFromCache(query) { * @returns {Promise} */ export function getDocsFromServer(query) { - return query.get({ source: 'server' }); + return query.get.call(reference, { source: 'server' }, MODULAR_DEPRECATION_ARG); } /** @@ -222,5 +222,5 @@ export function getDocsFromServer(query) { * @returns {Promise} */ export function deleteDoc(reference) { - return reference.delete(); + return reference.delete.call(reference, MODULAR_DEPRECATION_ARG); } From 736c07b72395c79f66dafa11178016e8c0d245a9 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Fri, 29 Nov 2024 11:54:25 +0000 Subject: [PATCH 02/29] proof of concept --- packages/app/lib/common/index.js | 98 ++++++++++++++----- packages/app/lib/common/unitTestUtils.ts | 9 +- .../app/lib/internal/registry/namespace.js | 24 +---- .../crashlytics/__tests__/crashlytics.test.ts | 2 +- .../firestore/__tests__/firestore.test.ts | 55 ++++++++++- packages/firestore/lib/FirestoreQuery.js | 82 ++++++++-------- packages/firestore/lib/index.js | 19 ++-- packages/firestore/lib/modular/index.js | 3 +- 8 files changed, 193 insertions(+), 99 deletions(-) diff --git a/packages/app/lib/common/index.js b/packages/app/lib/common/index.js index be893eb92b..8d73fe077b 100644 --- a/packages/app/lib/common/index.js +++ b/packages/app/lib/common/index.js @@ -108,17 +108,25 @@ const NO_REPLACEMENT = true; const mapOfDeprecationReplacements = { crashlytics: { - checkForUnsentReports: 'checkForUnsentReports()', - crash: 'crash()', - deleteUnsentReports: 'deleteUnsentReports()', - didCrashOnPreviousExecution: 'didCrashOnPreviousExecution()', - log: 'log()', - setAttribute: 'setAttribute()', - setAttributes: 'setAttributes()', - setUserId: 'setUserId()', - recordError: 'recordError()', - sendUnsentReports: 'sendUnsentReports()', - setCrashlyticsCollectionEnabled: 'setCrashlyticsCollectionEnabled()', + default: { + checkForUnsentReports: 'checkForUnsentReports()', + crash: 'crash()', + deleteUnsentReports: 'deleteUnsentReports()', + didCrashOnPreviousExecution: 'didCrashOnPreviousExecution()', + log: 'log()', + setAttribute: 'setAttribute()', + setAttributes: 'setAttributes()', + setUserId: 'setUserId()', + recordError: 'recordError()', + sendUnsentReports: 'sendUnsentReports()', + setCrashlyticsCollectionEnabled: 'setCrashlyticsCollectionEnabled()', + }, + }, + firestore: { + FirestoreCollectionReference: { + count: 'getCountFromServer()', + countFromServer: 'getCountFromServer()', + }, }, }; @@ -126,15 +134,14 @@ const v8deprecationMessage = 'This v8 method is deprecated and will be removed in the next major release ' + 'as part of move to match Firebase Web modular v9 SDK API.'; -export function deprecationConsoleWarning(moduleName, methodName, isModularMethod) { +export function deprecationConsoleWarning(nameSpace, methodName, instanceName, isModularMethod) { if (!isModularMethod) { - const moduleMap = mapOfDeprecationReplacements[moduleName]; + const moduleMap = mapOfDeprecationReplacements[nameSpace]; if (moduleMap) { - const replacementMethodName = moduleMap[methodName]; - // only warn if it is mapped and purposefully deprecated - if (replacementMethodName) { - const message = createMessage(moduleName, methodName); - + const instanceMap = moduleMap[instanceName]; + const deprecatedMethod = instanceMap[methodName]; + if (instanceMap && deprecatedMethod) { + const message = createMessage(nameSpace, methodName, instanceName); // eslint-disable-next-line no-console console.warn(message); } @@ -142,16 +149,23 @@ export function deprecationConsoleWarning(moduleName, methodName, isModularMetho } } -export function createMessage(moduleName, methodName, uniqueMessage = '') { - if (uniqueMessage.length > 0) { +export function createMessage( + nameSpace, + methodName, + instanceName = 'default', + uniqueMessage = null, +) { + if (uniqueMessage) { // Unique deprecation message used for testing return uniqueMessage; } - const moduleMap = mapOfDeprecationReplacements[moduleName]; + const moduleMap = mapOfDeprecationReplacements[nameSpace]; if (moduleMap) { - const replacementMethodName = moduleMap[methodName]; - if (replacementMethodName) { + const instance = moduleMap[instanceName]; + if (instance) { + const replacementMethodName = instance[methodName]; + if (replacementMethodName !== NO_REPLACEMENT) { return v8deprecationMessage + ` Please use \`${replacementMethodName}\` instead.`; } else { @@ -161,6 +175,44 @@ export function createMessage(moduleName, methodName, uniqueMessage = '') { } } +function getNamespace(className) { + if ( + ['FirestoreAggregateQuery', 'FirebaseFirestoreModule', 'FirestoreCollectionReference'].includes( + className, + ) + ) { + return 'firestore'; + } + if (['FirebaseCrashlyticsModule'].includes(className)) { + return 'crashlytics'; + } +} + +export function createDeprecationProxy(instance) { + return new Proxy(instance, { + get(target, prop, receiver) { + const originalMethod = target[prop]; + + if (prop === 'constructor') { + return target.constructor; + } + + if (typeof originalMethod === 'function') { + return function (...args) { + const isModularMethod = args.includes(MODULAR_DEPRECATION_ARG); + const nameSpace = getNamespace(target.constructor.name); + const instanceName = !receiver._config ? target.constructor.name : 'default'; + + deprecationConsoleWarning(nameSpace, prop, instanceName, isModularMethod); + + return originalMethod.apply(target, args); + }; + } + return Reflect.get(target, prop, receiver); + }, + }); +} + export const MODULAR_DEPRECATION_ARG = 'react-native-firebase-modular-method-call'; export function filterModularArgument(list) { diff --git a/packages/app/lib/common/unitTestUtils.ts b/packages/app/lib/common/unitTestUtils.ts index 7196fd99b4..a2324901b4 100644 --- a/packages/app/lib/common/unitTestUtils.ts +++ b/packages/app/lib/common/unitTestUtils.ts @@ -22,20 +22,23 @@ export type CheckV9DeprecationFunction = ( uniqueMessage: string = '', ) => void; -export const createCheckV9Deprecation = (moduleName: string): CheckV9DeprecationFunction => { +export const createCheckV9Deprecation = (moduleNames: string[]): CheckV9DeprecationFunction => { return ( modularFunction: () => void, nonModularFunction: () => void, methodName: string, - uniqueMessage = '', + uniqueMessage: string?, ) => { + const moduleName = moduleNames[0]; // firestore, database, etc + const instanceName = moduleNames[1] || 'default'; // default, FirestoreCollectionReference, etc const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); consoleWarnSpy.mockReset(); + consoleWarnSpy.mockRestore(); modularFunction(); expect(consoleWarnSpy).not.toHaveBeenCalled(); consoleWarnSpy.mockReset(); const consoleWarnSpy2 = jest.spyOn(console, 'warn').mockImplementation(warnMessage => { - const message = createMessage(moduleName, methodName, uniqueMessage); + const message = createMessage(moduleName, methodName, instanceName, uniqueMessage); expect(message).toMatch(warnMessage); }); nonModularFunction(); diff --git a/packages/app/lib/internal/registry/namespace.js b/packages/app/lib/internal/registry/namespace.js index 6ee73bf20b..48013eadf4 100644 --- a/packages/app/lib/internal/registry/namespace.js +++ b/packages/app/lib/internal/registry/namespace.js @@ -15,7 +15,7 @@ * */ -import { isString, MODULAR_DEPRECATION_ARG, deprecationConsoleWarning } from '../../common'; +import { isString, createDeprecationProxy } from '../../common'; import FirebaseApp from '../../FirebaseApp'; import SDK_VERSION from '../../version'; import { DEFAULT_APP_NAME, KNOWN_NAMESPACES } from '../constants'; @@ -277,28 +277,6 @@ export function getFirebaseRoot() { return createFirebaseRoot(); } -function createDeprecationProxy(instance) { - return new Proxy(instance, { - get(target, prop, receiver) { - const originalMethod = target[prop]; - if (prop === 'constructor') { - return target.constructor; - } - if (typeof originalMethod === 'function') { - return function (...args) { - const isModularMethod = args.includes(MODULAR_DEPRECATION_ARG); - const moduleName = receiver._config.namespace; - - deprecationConsoleWarning(moduleName, prop, isModularMethod); - - return originalMethod.apply(target, args); - }; - } - return Reflect.get(target, prop, receiver); - }, - }); -} - /** * * @param options diff --git a/packages/crashlytics/__tests__/crashlytics.test.ts b/packages/crashlytics/__tests__/crashlytics.test.ts index 7e9775445e..b6a18aebd2 100644 --- a/packages/crashlytics/__tests__/crashlytics.test.ts +++ b/packages/crashlytics/__tests__/crashlytics.test.ts @@ -89,7 +89,7 @@ describe('Crashlytics', function () { let checkV9Deprecation: CheckV9DeprecationFunction; beforeEach(function () { - checkV9Deprecation = createCheckV9Deprecation('crashlytics'); + checkV9Deprecation = createCheckV9Deprecation(['crashlytics']); // @ts-ignore test jest.spyOn(FirebaseModule.prototype, 'native', 'get').mockImplementation(() => { diff --git a/packages/firestore/__tests__/firestore.test.ts b/packages/firestore/__tests__/firestore.test.ts index 29f661fe9b..a70081d259 100644 --- a/packages/firestore/__tests__/firestore.test.ts +++ b/packages/firestore/__tests__/firestore.test.ts @@ -1,4 +1,10 @@ -import { describe, expect, it } from '@jest/globals'; +import { describe, expect, it, jest, beforeEach } from '@jest/globals'; +// @ts-ignore test +import FirebaseModule from '../../app/lib/internal/FirebaseModule'; +import { + createCheckV9Deprecation, + CheckV9DeprecationFunction, +} from '../../app/lib/common/unitTestUtils'; import firestore, { firebase, @@ -696,4 +702,51 @@ describe('Firestore', function () { expect(nullIndexManagerModular).toBeNull(); }); }); + + describe('test `console.warn` is called for RNFB v8 API & not called for v9 API', function () { + // let firestoreV9Deprecation: CheckV9DeprecationFunction; + let collectionRefV9Deprecation: CheckV9DeprecationFunction; + + beforeEach(function () { + // firestoreV9Deprecation = createCheckV9Deprecation(['firestore']); + collectionRefV9Deprecation = createCheckV9Deprecation([ + 'firestore', + 'FirestoreCollectionReference', + ]); + + // @ts-ignore test + jest.spyOn(FirebaseModule.prototype, 'native', 'get').mockImplementation(() => { + return new Proxy( + {}, + { + get: () => jest.fn(), + }, + ); + }); + }); + + it('count', function () { + const firestore = getFirestore(); + + const query = collection(firestore, 'test'); + + collectionRefV9Deprecation( + () => getCountFromServer(query), + () => query.count(), + 'count', + ); + }); + + it('countFromServer', function () { + const firestore = getFirestore(); + + const query = collection(firestore, 'test'); + + collectionRefV9Deprecation( + () => getCountFromServer(query), + () => query.countFromServer(), + 'count', + ); + }); + }); }); diff --git a/packages/firestore/lib/FirestoreQuery.js b/packages/firestore/lib/FirestoreQuery.js index c08dc662c0..6f1e462be8 100644 --- a/packages/firestore/lib/FirestoreQuery.js +++ b/packages/firestore/lib/FirestoreQuery.js @@ -22,6 +22,7 @@ import { isString, isUndefined, filterModularArgument, + createDeprecationProxy, warnIfNotModularCall, } from '@react-native-firebase/app/lib/common'; import NativeError from '@react-native-firebase/app/lib/internal/NativeFirebaseError'; @@ -135,12 +136,8 @@ export default class FirestoreQuery { } count() { - warnIfNotModularCall(arguments, 'getCountFromServer()'); - return new FirestoreAggregateQuery( - this._firestore, - this, - this._collectionPath, - this._modifiers, + return createDeprecationProxy( + new FirestoreAggregateQuery(this._firestore, this, this._collectionPath, this._modifiers), ); } @@ -150,27 +147,28 @@ export default class FirestoreQuery { } endAt(docOrField, ...fields) { - warnIfNotModularCall(arguments, 'endAt()'); - return new FirestoreQuery( - this._firestore, - this._collectionPath, - this._handleQueryCursor('endAt', docOrField, filterModularArgument(fields)), - this._queryName, + return createDeprecationProxy( + new FirestoreQuery( + this._firestore, + this._collectionPath, + this._handleQueryCursor('endAt', docOrField, filterModularArgument(fields)), + this._queryName, + ), ); } endBefore(docOrField, ...fields) { - warnIfNotModularCall(arguments, 'endBefore()'); - return new FirestoreQuery( - this._firestore, - this._collectionPath, - this._handleQueryCursor('endBefore', docOrField, filterModularArgument(fields)), - this._queryName, + return createDeprecationProxy( + new FirestoreQuery( + this._firestore, + this._collectionPath, + this._handleQueryCursor('endBefore', docOrField, filterModularArgument(fields)), + this._queryName, + ), ); } get(options) { - warnIfNotModularCall(arguments, 'getDocs()'); if (!isUndefined(options) && !isObject(options)) { throw new Error( "firebase.firestore().collection().get(*) 'options' must be an object is provided.", @@ -217,7 +215,6 @@ export default class FirestoreQuery { } isEqual(other) { - warnIfNotModularCall(arguments); if (!(other instanceof FirestoreQuery)) { throw new Error( "firebase.firestore().collection().isEqual(*) 'other' expected a Query instance.", @@ -250,7 +247,6 @@ export default class FirestoreQuery { } limit(limit) { - warnIfNotModularCall(arguments, 'limit()'); if (this._modifiers.isValidLimit(limit)) { throw new Error( "firebase.firestore().collection().limit(*) 'limit' must be a positive integer value.", @@ -259,11 +255,12 @@ export default class FirestoreQuery { const modifiers = this._modifiers._copy().limit(limit); - return new FirestoreQuery(this._firestore, this._collectionPath, modifiers, this._queryName); + return createDeprecationProxy( + new FirestoreQuery(this._firestore, this._collectionPath, modifiers, this._queryName), + ); } limitToLast(limitToLast) { - warnIfNotModularCall(arguments, 'limitToLast()'); if (this._modifiers.isValidLimitToLast(limitToLast)) { throw new Error( "firebase.firestore().collection().limitToLast(*) 'limitToLast' must be a positive integer value.", @@ -272,11 +269,12 @@ export default class FirestoreQuery { const modifiers = this._modifiers._copy().limitToLast(limitToLast); - return new FirestoreQuery(this._firestore, this._collectionPath, modifiers, this._queryName); + return createDeprecationProxy( + new FirestoreQuery(this._firestore, this._collectionPath, modifiers, this._queryName), + ); } onSnapshot(...args) { - warnIfNotModularCall(arguments, 'onSnapshot()'); let snapshotListenOptions; let callback; let onNext; @@ -398,31 +396,35 @@ export default class FirestoreQuery { throw new Error(`firebase.firestore().collection().orderBy() ${e.message}`); } - return new FirestoreQuery(this._firestore, this._collectionPath, modifiers, this._queryName); + return createDeprecationProxy( + new FirestoreQuery(this._firestore, this._collectionPath, modifiers, this._queryName), + ); } startAfter(docOrField, ...fields) { warnIfNotModularCall(arguments, 'startAfter()'); - return new FirestoreQuery( - this._firestore, - this._collectionPath, - this._handleQueryCursor('startAfter', docOrField, filterModularArgument(fields)), - this._queryName, + return createDeprecationProxy( + new FirestoreQuery( + this._firestore, + this._collectionPath, + this._handleQueryCursor('startAfter', docOrField, filterModularArgument(fields)), + this._queryName, + ), ); } startAt(docOrField, ...fields) { - warnIfNotModularCall(arguments, 'startAt()'); - return new FirestoreQuery( - this._firestore, - this._collectionPath, - this._handleQueryCursor('startAt', docOrField, filterModularArgument(fields)), - this._queryName, + return createDeprecationProxy( + new FirestoreQuery( + this._firestore, + this._collectionPath, + this._handleQueryCursor('startAt', docOrField, filterModularArgument(fields)), + this._queryName, + ), ); } where(fieldPathOrFilter, opStr, value) { - warnIfNotModularCall(arguments, 'where()'); if ( !isString(fieldPathOrFilter) && !(fieldPathOrFilter instanceof FirestoreFieldPath) && @@ -502,6 +504,8 @@ export default class FirestoreQuery { throw new Error(`firebase.firestore().collection().where() ${e.message}`); } - return new FirestoreQuery(this._firestore, this._collectionPath, modifiers, this._queryName); + return createDeprecationProxy( + new FirestoreQuery(this._firestore, this._collectionPath, modifiers, this._queryName), + ); } } diff --git a/packages/firestore/lib/index.js b/packages/firestore/lib/index.js index 08a5cfafea..e95cb29713 100644 --- a/packages/firestore/lib/index.js +++ b/packages/firestore/lib/index.js @@ -23,6 +23,7 @@ import { isString, isUndefined, isAndroid, + createDeprecationProxy, } from '@react-native-firebase/app/lib/common'; import { setReactNativeModule } from '@react-native-firebase/app/lib/internal/nativeModule'; import { @@ -175,7 +176,7 @@ class FirebaseFirestoreModule extends FirebaseModule { ); } - return new FirestoreCollectionReference(this, path); + return createDeprecationProxy(new FirestoreCollectionReference(this, path)); } collectionGroup(collectionId) { @@ -197,11 +198,13 @@ class FirebaseFirestoreModule extends FirebaseModule { ); } - return new FirestoreQuery( - this, - this._referencePath.child(collectionId), - new FirestoreQueryModifiers().asCollectionGroupQuery(), - undefined, + return createDeprecationProxy( + new FirestoreQuery( + this, + this._referencePath.child(collectionId), + new FirestoreQueryModifiers().asCollectionGroupQuery(), + undefined, + ), ); } @@ -224,7 +227,7 @@ class FirebaseFirestoreModule extends FirebaseModule { throw new Error("firebase.firestore().doc(*) 'documentPath' must point to a document."); } - return new FirestoreDocumentReference(this, path); + return createDeprecationProxy(new FirestoreDocumentReference(this, path)); } async enableNetwork() { @@ -377,7 +380,7 @@ class FirebaseFirestoreModule extends FirebaseModule { if (this._settings.persistence === false) { return null; } - return new FirestorePersistentCacheIndexManager(this); + return createDeprecationProxy(new FirestorePersistentCacheIndexManager(this)); } } diff --git a/packages/firestore/lib/modular/index.js b/packages/firestore/lib/modular/index.js index 4242e983eb..70618d13b0 100644 --- a/packages/firestore/lib/modular/index.js +++ b/packages/firestore/lib/modular/index.js @@ -21,6 +21,7 @@ import { fieldPathFromArgument, } from '../FirestoreAggregate'; import FirestoreQuery from '../FirestoreQuery'; +import { MODULAR_DEPRECATION_ARG } from '../../../app/lib/common'; /** * @param {FirebaseApp?} app @@ -197,7 +198,7 @@ export function runTransaction(firestore, updateFunction) { * @returns {Promise} */ export function getCountFromServer(query) { - return query.count().get(); + return query.count().get.call(query, MODULAR_DEPRECATION_ARG); } export function getAggregateFromServer(query, aggregateSpec) { From 15f4baf7aaaae36b4e51480e0630c6461d5f656b Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Fri, 29 Nov 2024 12:02:52 +0000 Subject: [PATCH 03/29] fix mock resolve value --- packages/firestore/__tests__/firestore.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/__tests__/firestore.test.ts b/packages/firestore/__tests__/firestore.test.ts index a70081d259..5e881ca92b 100644 --- a/packages/firestore/__tests__/firestore.test.ts +++ b/packages/firestore/__tests__/firestore.test.ts @@ -719,7 +719,7 @@ describe('Firestore', function () { return new Proxy( {}, { - get: () => jest.fn(), + get: () => jest.fn().mockResolvedValue(null as never), }, ); }); From 47317b4ec0bd64f9258f9efac021ff5ed5e7179c Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Fri, 29 Nov 2024 13:50:41 +0000 Subject: [PATCH 04/29] tests --- packages/app/lib/common/unitTestUtils.ts | 1 + .../firestore/__tests__/firestore.test.ts | 61 ++++++++++++++++++- packages/firestore/lib/modular/query.js | 6 +- 3 files changed, 64 insertions(+), 4 deletions(-) diff --git a/packages/app/lib/common/unitTestUtils.ts b/packages/app/lib/common/unitTestUtils.ts index a2324901b4..59fa06d871 100644 --- a/packages/app/lib/common/unitTestUtils.ts +++ b/packages/app/lib/common/unitTestUtils.ts @@ -37,6 +37,7 @@ export const createCheckV9Deprecation = (moduleNames: string[]): CheckV9Deprecat modularFunction(); expect(consoleWarnSpy).not.toHaveBeenCalled(); consoleWarnSpy.mockReset(); + consoleWarnSpy.mockRestore(); const consoleWarnSpy2 = jest.spyOn(console, 'warn').mockImplementation(warnMessage => { const message = createMessage(moduleName, methodName, instanceName, uniqueMessage); expect(message).toMatch(warnMessage); diff --git a/packages/firestore/__tests__/firestore.test.ts b/packages/firestore/__tests__/firestore.test.ts index 5e881ca92b..7509c9230c 100644 --- a/packages/firestore/__tests__/firestore.test.ts +++ b/packages/firestore/__tests__/firestore.test.ts @@ -1,6 +1,9 @@ import { describe, expect, it, jest, beforeEach } from '@jest/globals'; // @ts-ignore test import FirebaseModule from '../../app/lib/internal/FirebaseModule'; +// @ts-ignore test +import FirestoreQuery from '../lib/FirestoreQuery'; + import { createCheckV9Deprecation, CheckV9DeprecationFunction, @@ -706,6 +709,7 @@ describe('Firestore', function () { describe('test `console.warn` is called for RNFB v8 API & not called for v9 API', function () { // let firestoreV9Deprecation: CheckV9DeprecationFunction; let collectionRefV9Deprecation: CheckV9DeprecationFunction; + // let queryV9Deprecation: CheckV9DeprecationFunction; beforeEach(function () { // firestoreV9Deprecation = createCheckV9Deprecation(['firestore']); @@ -714,15 +718,34 @@ describe('Firestore', function () { 'FirestoreCollectionReference', ]); + // queryV9Deprecation = createCheckV9Deprecation(['firestore', 'FirestoreQuery']); + // @ts-ignore test jest.spyOn(FirebaseModule.prototype, 'native', 'get').mockImplementation(() => { return new Proxy( {}, { - get: () => jest.fn().mockResolvedValue(null as never), + get: () => + jest.fn().mockResolvedValue({ + source: 'cache', + changes: [], + documents: [], + metadata: {}, + } as never), }, ); }); + + jest + .spyOn(FirestoreQuery.prototype, '_handleQueryCursor') + // @ts-ignore test + .mockImplementation((cursor, docOrField, fields) => { + // Mock implementation returning an empty array or any other desired value + return []; // or any other mock value you want to return + }); + + // jest.spyOn(FirestoreQuerySnapshot.prototype, 'constructor'); + // @ts-ignore test }); it('count', function () { @@ -748,5 +771,41 @@ describe('Firestore', function () { 'count', ); }); + + it('endAt', function () { + const firestore = getFirestore(); + + const query = collection(firestore, 'test'); + + collectionRefV9Deprecation( + () => endAt('foo'), + () => query.endAt('foo'), + 'endAt', + ); + }); + + it('endBefore', function () { + const firestore = getFirestore(); + + const query = collection(firestore, 'test'); + + collectionRefV9Deprecation( + () => endBefore('foo'), + () => query.endBefore('foo'), + 'endBefore', + ); + }); + + it('get', function () { + const firestore = getFirestore(); + + const query = collection(firestore, 'test'); + + collectionRefV9Deprecation( + () => getDocs(query), + () => query.get(), + 'get', + ); + }); }); }); diff --git a/packages/firestore/lib/modular/query.js b/packages/firestore/lib/modular/query.js index 943ca0906c..a1629ab207 100644 --- a/packages/firestore/lib/modular/query.js +++ b/packages/firestore/lib/modular/query.js @@ -198,7 +198,7 @@ export function getDocFromServer(reference) { * @returns {Promise} */ export function getDocs(query) { - return query.get.call(reference, { source: 'default' }, MODULAR_DEPRECATION_ARG); + return query.get.call(query, { source: 'default' }, MODULAR_DEPRECATION_ARG); } /** @@ -206,7 +206,7 @@ export function getDocs(query) { * @returns {Promise} */ export function getDocsFromCache(query) { - return query.get.call(reference, { source: 'cache' }, MODULAR_DEPRECATION_ARG); + return query.get.call(query, { source: 'cache' }, MODULAR_DEPRECATION_ARG); } /** @@ -214,7 +214,7 @@ export function getDocsFromCache(query) { * @returns {Promise} */ export function getDocsFromServer(query) { - return query.get.call(reference, { source: 'server' }, MODULAR_DEPRECATION_ARG); + return query.get.call(query, { source: 'server' }, MODULAR_DEPRECATION_ARG); } /** From 3d9aa5e7d323a3a99ace5da7b2befa59de44a7a2 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Fri, 29 Nov 2024 13:51:02 +0000 Subject: [PATCH 05/29] update common functions --- packages/app/lib/common/index.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/app/lib/common/index.js b/packages/app/lib/common/index.js index 8d73fe077b..c2524084ea 100644 --- a/packages/app/lib/common/index.js +++ b/packages/app/lib/common/index.js @@ -126,7 +126,11 @@ const mapOfDeprecationReplacements = { FirestoreCollectionReference: { count: 'getCountFromServer()', countFromServer: 'getCountFromServer()', + endAt: 'endAt()', + endBefore: 'endBefore()', + get: 'getDocs()', }, + FirestoreQuery: {}, }, }; @@ -177,9 +181,12 @@ export function createMessage( function getNamespace(className) { if ( - ['FirestoreAggregateQuery', 'FirebaseFirestoreModule', 'FirestoreCollectionReference'].includes( - className, - ) + [ + 'FirestoreAggregateQuery', + 'FirebaseFirestoreModule', + 'FirestoreCollectionReference', + 'FirestoreQuery', + ].includes(className) ) { return 'firestore'; } From 38b018b5fa5fa27938a307f13a380be1cd69929e Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Fri, 29 Nov 2024 14:13:08 +0000 Subject: [PATCH 06/29] remove warnings --- packages/firestore/lib/FirestoreQuery.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/firestore/lib/FirestoreQuery.js b/packages/firestore/lib/FirestoreQuery.js index 6f1e462be8..0f6132368e 100644 --- a/packages/firestore/lib/FirestoreQuery.js +++ b/packages/firestore/lib/FirestoreQuery.js @@ -351,7 +351,6 @@ export default class FirestoreQuery { } orderBy(fieldPath, directionStr) { - warnIfNotModularCall(arguments, 'orderBy()'); if (!isString(fieldPath) && !(fieldPath instanceof FirestoreFieldPath)) { throw new Error( "firebase.firestore().collection().orderBy(*) 'fieldPath' must be a string or instance of FieldPath.", @@ -402,7 +401,6 @@ export default class FirestoreQuery { } startAfter(docOrField, ...fields) { - warnIfNotModularCall(arguments, 'startAfter()'); return createDeprecationProxy( new FirestoreQuery( this._firestore, From d1ad2807b3d9f31d0ab053ceb76da1569ae0f385 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Fri, 29 Nov 2024 14:13:27 +0000 Subject: [PATCH 07/29] tweak test name --- packages/app/lib/common/unitTestUtils.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/app/lib/common/unitTestUtils.ts b/packages/app/lib/common/unitTestUtils.ts index 59fa06d871..7753da7205 100644 --- a/packages/app/lib/common/unitTestUtils.ts +++ b/packages/app/lib/common/unitTestUtils.ts @@ -18,7 +18,7 @@ export const checkV9Deprecation = (modularFunction: () => void, nonModularFuncti export type CheckV9DeprecationFunction = ( modularFunction: () => void, nonModularFunction: () => void, - methodName: string, + methodNameKey: string, uniqueMessage: string = '', ) => void; @@ -26,7 +26,7 @@ export const createCheckV9Deprecation = (moduleNames: string[]): CheckV9Deprecat return ( modularFunction: () => void, nonModularFunction: () => void, - methodName: string, + methodNameKey: string, uniqueMessage: string?, ) => { const moduleName = moduleNames[0]; // firestore, database, etc @@ -39,7 +39,7 @@ export const createCheckV9Deprecation = (moduleNames: string[]): CheckV9Deprecat consoleWarnSpy.mockReset(); consoleWarnSpy.mockRestore(); const consoleWarnSpy2 = jest.spyOn(console, 'warn').mockImplementation(warnMessage => { - const message = createMessage(moduleName, methodName, instanceName, uniqueMessage); + const message = createMessage(moduleName, methodNameKey, instanceName, uniqueMessage); expect(message).toMatch(warnMessage); }); nonModularFunction(); From 4a5c5ea6407aeb216d8bf25c68f81545c80a422c Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Fri, 29 Nov 2024 14:14:50 +0000 Subject: [PATCH 08/29] query unit tests --- packages/app/lib/common/index.js | 8 ++ .../firestore/__tests__/firestore.test.ts | 97 +++++++++++++++++++ 2 files changed, 105 insertions(+) diff --git a/packages/app/lib/common/index.js b/packages/app/lib/common/index.js index c2524084ea..f8548ab4aa 100644 --- a/packages/app/lib/common/index.js +++ b/packages/app/lib/common/index.js @@ -129,6 +129,14 @@ const mapOfDeprecationReplacements = { endAt: 'endAt()', endBefore: 'endBefore()', get: 'getDocs()', + isEqual: NO_REPLACEMENT, + limit: 'limit()', + limitToLast: 'limitToLast()', + onSnapshot: 'onSnapshot()', + orderBy: 'orderBy()', + startAfter: 'startAfter()', + startAt: 'startAt()', + where: 'where()', }, FirestoreQuery: {}, }, diff --git a/packages/firestore/__tests__/firestore.test.ts b/packages/firestore/__tests__/firestore.test.ts index 7509c9230c..61de138d15 100644 --- a/packages/firestore/__tests__/firestore.test.ts +++ b/packages/firestore/__tests__/firestore.test.ts @@ -807,5 +807,102 @@ describe('Firestore', function () { 'get', ); }); + + it('isEqual', function () { + const firestore = getFirestore(); + + const query = collection(firestore, 'test'); + + collectionRefV9Deprecation( + // no equivalent method + () => {}, + () => query.isEqual(query), + 'isEqual', + ); + }); + + it('limit', function () { + const firestore = getFirestore(); + + const query = collection(firestore, 'test'); + + collectionRefV9Deprecation( + () => limit(9), + () => query.limit(9), + 'limit', + ); + }); + + it('limitToLast', function () { + const firestore = getFirestore(); + + const query = collection(firestore, 'test'); + + collectionRefV9Deprecation( + () => limitToLast(9), + () => query.limitToLast(9), + 'limitToLast', + ); + }); + + it('onSnapshot', function () { + const firestore = getFirestore(); + + const query = collection(firestore, 'test'); + + collectionRefV9Deprecation( + () => onSnapshot(query, () => {}), + () => query.onSnapshot(() => {}), + 'onSnapshot', + ); + }); + + it('orderBy', function () { + const firestore = getFirestore(); + + const query = collection(firestore, 'test'); + + collectionRefV9Deprecation( + () => orderBy('foo', 'asc'), + () => query.orderBy('foo', 'asc'), + 'orderBy', + ); + }); + + it('startAfter', function () { + const firestore = getFirestore(); + + const query = collection(firestore, 'test'); + + collectionRefV9Deprecation( + () => startAfter('foo'), + () => query.startAfter('foo'), + 'startAfter', + ); + }); + + it('startAt', function () { + const firestore = getFirestore(); + + const query = collection(firestore, 'test'); + + collectionRefV9Deprecation( + () => startAt('foo'), + () => query.startAt('foo'), + 'startAt', + ); + }); + + it('where', function () { + const firestore = getFirestore(); + + const query = collection(firestore, 'test'); + + collectionRefV9Deprecation( + () => where('foo', '==', 'bar'), + () => query.where('foo', '==', 'bar'), + 'where', + ); + }); }); }); From 21302d033b58467ff733b5ad6091dc0d6f634749 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Fri, 29 Nov 2024 16:28:06 +0000 Subject: [PATCH 09/29] firestore unit tests --- packages/app/lib/common/index.js | 17 +- .../firestore/__tests__/firestore.test.ts | 166 ++++++++++++++++-- .../firestore/lib/FirestoreDocumentChange.js | 6 +- .../lib/FirestoreDocumentReference.js | 14 +- packages/firestore/lib/FirestoreQuery.js | 1 - .../firestore/lib/FirestoreQuerySnapshot.js | 5 +- .../firestore/lib/FirestoreTransaction.js | 4 +- 7 files changed, 187 insertions(+), 26 deletions(-) diff --git a/packages/app/lib/common/index.js b/packages/app/lib/common/index.js index f8548ab4aa..1cde712130 100644 --- a/packages/app/lib/common/index.js +++ b/packages/app/lib/common/index.js @@ -137,8 +137,21 @@ const mapOfDeprecationReplacements = { startAfter: 'startAfter()', startAt: 'startAt()', where: 'where()', + add: 'addDoc()', + doc: 'doc()', + }, + FirestoreDocumentReference: { + collection: 'collection()', + delete: 'deleteDoc()', + get: 'getDoc()', + isEqual: NO_REPLACEMENT, + onSnapshot: 'onSnapshot()', + set: 'setDoc()', + update: 'updateDoc()', + }, + FirestoreDocumentSnapshot: { + isEqual: NO_REPLACEMENT, }, - FirestoreQuery: {}, }, }; @@ -194,6 +207,8 @@ function getNamespace(className) { 'FirebaseFirestoreModule', 'FirestoreCollectionReference', 'FirestoreQuery', + 'FirestoreDocumentReference', + 'FirestoreDocumentSnapshot', ].includes(className) ) { return 'firestore'; diff --git a/packages/firestore/__tests__/firestore.test.ts b/packages/firestore/__tests__/firestore.test.ts index 61de138d15..9d00bd932c 100644 --- a/packages/firestore/__tests__/firestore.test.ts +++ b/packages/firestore/__tests__/firestore.test.ts @@ -1,8 +1,12 @@ import { describe, expect, it, jest, beforeEach } from '@jest/globals'; // @ts-ignore test +import { createDeprecationProxy } from '../../app/lib/common'; +// @ts-ignore test import FirebaseModule from '../../app/lib/internal/FirebaseModule'; // @ts-ignore test import FirestoreQuery from '../lib/FirestoreQuery'; +// @ts-ignore test +import FirestoreDocumentSnapshot from '../lib/FirestoreDocumentSnapshot'; import { createCheckV9Deprecation, @@ -709,7 +713,7 @@ describe('Firestore', function () { describe('test `console.warn` is called for RNFB v8 API & not called for v9 API', function () { // let firestoreV9Deprecation: CheckV9DeprecationFunction; let collectionRefV9Deprecation: CheckV9DeprecationFunction; - // let queryV9Deprecation: CheckV9DeprecationFunction; + let docRefV9Deprecation: CheckV9DeprecationFunction; beforeEach(function () { // firestoreV9Deprecation = createCheckV9Deprecation(['firestore']); @@ -718,6 +722,8 @@ describe('Firestore', function () { 'FirestoreCollectionReference', ]); + docRefV9Deprecation = createCheckV9Deprecation(['firestore', 'FirestoreDocumentReference']); + // queryV9Deprecation = createCheckV9Deprecation(['firestore', 'FirestoreQuery']); // @ts-ignore test @@ -731,6 +737,7 @@ describe('Firestore', function () { changes: [], documents: [], metadata: {}, + path: 'foo', } as never), }, ); @@ -748,7 +755,7 @@ describe('Firestore', function () { // @ts-ignore test }); - it('count', function () { + it('CollectionReference.count()', function () { const firestore = getFirestore(); const query = collection(firestore, 'test'); @@ -760,7 +767,7 @@ describe('Firestore', function () { ); }); - it('countFromServer', function () { + it('CollectionReference.countFromServer()', function () { const firestore = getFirestore(); const query = collection(firestore, 'test'); @@ -772,7 +779,7 @@ describe('Firestore', function () { ); }); - it('endAt', function () { + it('CollectionReference.endAt()', function () { const firestore = getFirestore(); const query = collection(firestore, 'test'); @@ -784,7 +791,7 @@ describe('Firestore', function () { ); }); - it('endBefore', function () { + it('CollectionReference.endBefore()', function () { const firestore = getFirestore(); const query = collection(firestore, 'test'); @@ -796,7 +803,7 @@ describe('Firestore', function () { ); }); - it('get', function () { + it('CollectionReference.get()', function () { const firestore = getFirestore(); const query = collection(firestore, 'test'); @@ -808,7 +815,7 @@ describe('Firestore', function () { ); }); - it('isEqual', function () { + it('CollectionReference.isEqual()', function () { const firestore = getFirestore(); const query = collection(firestore, 'test'); @@ -821,7 +828,7 @@ describe('Firestore', function () { ); }); - it('limit', function () { + it('CollectionReference.limit()', function () { const firestore = getFirestore(); const query = collection(firestore, 'test'); @@ -833,7 +840,7 @@ describe('Firestore', function () { ); }); - it('limitToLast', function () { + it('CollectionReference.limitToLast()', function () { const firestore = getFirestore(); const query = collection(firestore, 'test'); @@ -845,7 +852,7 @@ describe('Firestore', function () { ); }); - it('onSnapshot', function () { + it('CollectionReference.onSnapshot()', function () { const firestore = getFirestore(); const query = collection(firestore, 'test'); @@ -857,7 +864,7 @@ describe('Firestore', function () { ); }); - it('orderBy', function () { + it('CollectionReference.orderBy()', function () { const firestore = getFirestore(); const query = collection(firestore, 'test'); @@ -869,7 +876,7 @@ describe('Firestore', function () { ); }); - it('startAfter', function () { + it('CollectionReference.startAfter()', function () { const firestore = getFirestore(); const query = collection(firestore, 'test'); @@ -881,7 +888,7 @@ describe('Firestore', function () { ); }); - it('startAt', function () { + it('CollectionReference.startAt()', function () { const firestore = getFirestore(); const query = collection(firestore, 'test'); @@ -893,7 +900,7 @@ describe('Firestore', function () { ); }); - it('where', function () { + it('CollectionReference.where()', function () { const firestore = getFirestore(); const query = collection(firestore, 'test'); @@ -904,5 +911,136 @@ describe('Firestore', function () { 'where', ); }); + + it('CollectionReference.add()', function () { + const firestore = getFirestore(); + + const query = collection(firestore, 'test'); + + collectionRefV9Deprecation( + () => addDoc(query, { foo: 'bar' }), + () => query.add({ foo: 'bar' }), + 'add', + ); + }); + + it('CollectionReference.doc()', function () { + const firestore = getFirestore(); + + const query = collection(firestore, 'test'); + + collectionRefV9Deprecation( + () => doc(query, 'bar'), + () => query.doc('foo'), + 'doc', + ); + }); + + it('DocumentReference.collection()', function () { + const firestore = getFirestore(); + + const docRef = firestore.doc('some/foo'); + + docRefV9Deprecation( + () => collection(firestore, 'bar'), + () => docRef.collection('bar'), + 'collection', + ); + }); + + it('DocumentReference.delete()', function () { + const firestore = getFirestore(); + + const docRef = firestore.doc('some/foo'); + + docRefV9Deprecation( + () => deleteDoc(docRef), + () => docRef.delete(), + 'delete', + ); + }); + + it('DocumentReference.get()', function () { + const firestore = getFirestore(); + + const docRef = firestore.doc('some/foo'); + + docRefV9Deprecation( + () => getDoc(docRef), + () => docRef.get(), + 'get', + ); + }); + + it('DocumentReference.isEqual()', function () { + const firestore = getFirestore(); + + const docRef = firestore.doc('some/foo'); + + docRefV9Deprecation( + // no equivalent method + () => {}, + () => docRef.isEqual(docRef), + 'isEqual', + ); + }); + + it('DocumentReference.onSnapshot()', function () { + const firestore = getFirestore(); + + const docRef = firestore.doc('some/foo'); + + docRefV9Deprecation( + () => onSnapshot(docRef, () => {}), + () => docRef.onSnapshot(() => {}), + 'onSnapshot', + ); + }); + + it('DocumentReference.set()', function () { + const firestore = getFirestore(); + + const docRef = firestore.doc('some/foo'); + + docRefV9Deprecation( + () => setDoc(docRef, { foo: 'bar' }), + () => docRef.set({ foo: 'bar' }), + 'set', + ); + }); + + it('DocumentReference.update()', function () { + const firestore = getFirestore(); + + const docRef = firestore.doc('some/foo'); + + docRefV9Deprecation( + () => updateDoc(docRef, { foo: 'bar' }), + () => docRef.update({ foo: 'bar' }), + 'update', + ); + }); + + it('FirestoreDocumentSnapshot.isEqual()', function () { + const firestore = getFirestore(); + // Every `FirestoreDocumentSnapshot` has been wrapped in deprecation proxy, so we use constructor directly + // for ease of mocking + const snapshot = createDeprecationProxy( + new FirestoreDocumentSnapshot(firestore, { + source: 'cache', + changes: [], + documents: [], + metadata: {}, + path: 'foo', + }), + ); + + docRefV9Deprecation( + // no equivalent method + () => {}, + () => snapshot.isEqual(snapshot), + 'isEqual', + ); + }); }); }); diff --git a/packages/firestore/lib/FirestoreDocumentChange.js b/packages/firestore/lib/FirestoreDocumentChange.js index e2cb2e7c02..9aa66833b6 100644 --- a/packages/firestore/lib/FirestoreDocumentChange.js +++ b/packages/firestore/lib/FirestoreDocumentChange.js @@ -14,7 +14,7 @@ * limitations under the License. * */ - +import { createDeprecationProxy } from '@react-native-firebase/app/lib/common'; import FirestoreDocumentSnapshot from './FirestoreDocumentSnapshot'; const TYPE_MAP = { @@ -31,7 +31,9 @@ export default class FirestoreDocumentChange { } get doc() { - return new FirestoreDocumentSnapshot(this._firestore, this._nativeData.doc); + return createDeprecationProxy( + new FirestoreDocumentSnapshot(this._firestore, this._nativeData.doc), + ); } get newIndex() { diff --git a/packages/firestore/lib/FirestoreDocumentReference.js b/packages/firestore/lib/FirestoreDocumentReference.js index 0ffe715f95..141eb63e70 100644 --- a/packages/firestore/lib/FirestoreDocumentReference.js +++ b/packages/firestore/lib/FirestoreDocumentReference.js @@ -15,7 +15,12 @@ * */ -import { isObject, isString, isUndefined } from '@react-native-firebase/app/lib/common'; +import { + isObject, + isString, + isUndefined, + createDeprecationProxy, +} from '@react-native-firebase/app/lib/common'; import NativeError from '@react-native-firebase/app/lib/internal/NativeFirebaseError'; import { parseSetOptions, parseSnapshotArgs, parseUpdateArgs } from './utils'; import { buildNativeMap, provideDocumentReferenceClass } from './utils/serialize'; @@ -103,7 +108,7 @@ export default class FirestoreDocumentReference { return this._firestore.native .documentGet(this.path, options) - .then(data => new FirestoreDocumentSnapshot(this._firestore, data)); + .then(data => createDeprecationProxy(new FirestoreDocumentSnapshot(this._firestore, data))); } isEqual(other) { @@ -154,9 +159,8 @@ export default class FirestoreDocumentReference { if (event.body.error) { handleError(NativeError.fromEvent(event.body.error, 'firestore')); } else { - const documentSnapshot = new FirestoreDocumentSnapshot( - this._firestore, - event.body.snapshot, + const documentSnapshot = createDeprecationProxy( + new FirestoreDocumentSnapshot(this._firestore, event.body.snapshot), ); handleSuccess(documentSnapshot); } diff --git a/packages/firestore/lib/FirestoreQuery.js b/packages/firestore/lib/FirestoreQuery.js index 0f6132368e..a8de1042dd 100644 --- a/packages/firestore/lib/FirestoreQuery.js +++ b/packages/firestore/lib/FirestoreQuery.js @@ -23,7 +23,6 @@ import { isUndefined, filterModularArgument, createDeprecationProxy, - warnIfNotModularCall, } from '@react-native-firebase/app/lib/common'; import NativeError from '@react-native-firebase/app/lib/internal/NativeFirebaseError'; import { FirestoreAggregateQuery } from './FirestoreAggregate'; diff --git a/packages/firestore/lib/FirestoreQuerySnapshot.js b/packages/firestore/lib/FirestoreQuerySnapshot.js index 1ea59b9d0e..a730038ac7 100644 --- a/packages/firestore/lib/FirestoreQuerySnapshot.js +++ b/packages/firestore/lib/FirestoreQuerySnapshot.js @@ -20,6 +20,7 @@ import { isFunction, isObject, isUndefined, + createDeprecationProxy, } from '@react-native-firebase/app/lib/common'; import FirestoreDocumentChange from './FirestoreDocumentChange'; import FirestoreDocumentSnapshot from './FirestoreDocumentSnapshot'; @@ -31,7 +32,9 @@ export default class FirestoreQuerySnapshot { this._source = nativeData.source; this._excludesMetadataChanges = nativeData.excludesMetadataChanges; this._changes = nativeData.changes.map($ => new FirestoreDocumentChange(firestore, $)); - this._docs = nativeData.documents.map($ => new FirestoreDocumentSnapshot(firestore, $)); + this._docs = nativeData.documents.map($ => + createDeprecationProxy(new FirestoreDocumentSnapshot(firestore, $)), + ); this._metadata = new FirestoreSnapshotMetadata(nativeData.metadata); } diff --git a/packages/firestore/lib/FirestoreTransaction.js b/packages/firestore/lib/FirestoreTransaction.js index d6d710b974..f496ddb6b8 100644 --- a/packages/firestore/lib/FirestoreTransaction.js +++ b/packages/firestore/lib/FirestoreTransaction.js @@ -15,7 +15,7 @@ * */ -import { isObject } from '@react-native-firebase/app/lib/common'; +import { isObject, createDeprecationProxy } from '@react-native-firebase/app/lib/common'; import FirestoreDocumentReference from './FirestoreDocumentReference'; import FirestoreDocumentSnapshot from './FirestoreDocumentSnapshot'; import { parseSetOptions, parseUpdateArgs } from './utils'; @@ -52,7 +52,7 @@ export default class FirestoreTransaction { this._calledGetCount++; return this._firestore.native .transactionGetDocument(this._meta.id, documentRef.path) - .then(data => new FirestoreDocumentSnapshot(this._firestore, data)); + .then(data => createDeprecationProxy(new FirestoreDocumentSnapshot(this._firestore, data))); } /** From 3c728e241ef1bc3c7d08125335d82806f0136a81 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Fri, 29 Nov 2024 17:08:49 +0000 Subject: [PATCH 10/29] refactor how to get namespace --- packages/app/lib/common/index.js | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/packages/app/lib/common/index.js b/packages/app/lib/common/index.js index 1cde712130..3276088aa5 100644 --- a/packages/app/lib/common/index.js +++ b/packages/app/lib/common/index.js @@ -201,21 +201,11 @@ export function createMessage( } function getNamespace(className) { - if ( - [ - 'FirestoreAggregateQuery', - 'FirebaseFirestoreModule', - 'FirestoreCollectionReference', - 'FirestoreQuery', - 'FirestoreDocumentReference', - 'FirestoreDocumentSnapshot', - ].includes(className) - ) { - return 'firestore'; - } - if (['FirebaseCrashlyticsModule'].includes(className)) { - return 'crashlytics'; - } + return Object.keys(mapOfDeprecationReplacements).find(key => { + if (mapOfDeprecationReplacements[key][className]) { + return key; + } + }); } export function createDeprecationProxy(instance) { @@ -230,7 +220,10 @@ export function createDeprecationProxy(instance) { if (typeof originalMethod === 'function') { return function (...args) { const isModularMethod = args.includes(MODULAR_DEPRECATION_ARG); - const nameSpace = getNamespace(target.constructor.name); + const nameSpace = receiver._config + ? receiver._config.namespace + : getNamespace(target.constructor.name); + const instanceName = !receiver._config ? target.constructor.name : 'default'; deprecationConsoleWarning(nameSpace, prop, instanceName, isModularMethod); From da7d5103b72294488ec1f477cae8af56fcef4981 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Fri, 29 Nov 2024 17:52:17 +0000 Subject: [PATCH 11/29] FieldValue tests --- packages/app/lib/common/index.js | 26 +++++++++-- .../firestore/__tests__/firestore.test.ts | 43 ++++++++++++++++++- packages/firestore/lib/FirestoreStatics.js | 3 +- 3 files changed, 66 insertions(+), 6 deletions(-) diff --git a/packages/app/lib/common/index.js b/packages/app/lib/common/index.js index 3276088aa5..33a402394f 100644 --- a/packages/app/lib/common/index.js +++ b/packages/app/lib/common/index.js @@ -152,6 +152,13 @@ const mapOfDeprecationReplacements = { FirestoreDocumentSnapshot: { isEqual: NO_REPLACEMENT, }, + FirestoreFieldValue: { + arrayRemove: 'arrayRemove()', + arrayUnion: 'arrayUnion()', + delete: 'deleteField()', + increment: 'increment()', + serverTimestamp: 'serverTimestamp()', + }, }, }; @@ -208,6 +215,19 @@ function getNamespace(className) { }); } +function getInstanceName(target) { + if (target._config) { + // module class instance, we use default to store map of deprecated methods + return 'default'; + } + if (target.name) { + // It's a function which has a name property unlike classes + return target.name; + } + // It's a class instance + return target.constructor.name; +} + export function createDeprecationProxy(instance) { return new Proxy(instance, { get(target, prop, receiver) { @@ -220,11 +240,9 @@ export function createDeprecationProxy(instance) { if (typeof originalMethod === 'function') { return function (...args) { const isModularMethod = args.includes(MODULAR_DEPRECATION_ARG); - const nameSpace = receiver._config - ? receiver._config.namespace - : getNamespace(target.constructor.name); + const instanceName = getInstanceName(target); + const nameSpace = getNamespace(instanceName); - const instanceName = !receiver._config ? target.constructor.name : 'default'; deprecationConsoleWarning(nameSpace, prop, instanceName, isModularMethod); diff --git a/packages/firestore/__tests__/firestore.test.ts b/packages/firestore/__tests__/firestore.test.ts index 9d00bd932c..afc54c395a 100644 --- a/packages/firestore/__tests__/firestore.test.ts +++ b/packages/firestore/__tests__/firestore.test.ts @@ -714,6 +714,7 @@ describe('Firestore', function () { // let firestoreV9Deprecation: CheckV9DeprecationFunction; let collectionRefV9Deprecation: CheckV9DeprecationFunction; let docRefV9Deprecation: CheckV9DeprecationFunction; + let fieldValueV9Deprecation: CheckV9DeprecationFunction; beforeEach(function () { // firestoreV9Deprecation = createCheckV9Deprecation(['firestore']); @@ -724,7 +725,7 @@ describe('Firestore', function () { docRefV9Deprecation = createCheckV9Deprecation(['firestore', 'FirestoreDocumentReference']); - // queryV9Deprecation = createCheckV9Deprecation(['firestore', 'FirestoreQuery']); + fieldValueV9Deprecation = createCheckV9Deprecation(['firestore', 'FirestoreFieldValue']); // @ts-ignore test jest.spyOn(FirebaseModule.prototype, 'native', 'get').mockImplementation(() => { @@ -1042,5 +1043,45 @@ describe('Firestore', function () { 'isEqual', ); }); + + it('FirestoreFieldValue.delete()', function () { + fieldValueV9Deprecation( + () => deleteField(), + () => firestore.FieldValue.delete(), + 'delete', + ); + }); + + it('FirestoreFieldValue.increment()', function () { + fieldValueV9Deprecation( + () => increment(3), + () => firestore.FieldValue.increment(4), + 'increment', + ); + }); + + it('FirestoreFieldValue.serverTimestamp()', function () { + fieldValueV9Deprecation( + () => serverTimestamp(), + () => firestore.FieldValue.serverTimestamp(), + 'serverTimestamp', + ); + }); + + it('FirestoreFieldValue.arrayUnion()', function () { + fieldValueV9Deprecation( + () => arrayUnion('foo'), + () => firestore.FieldValue.arrayUnion('bar'), + 'arrayUnion', + ); + }); + + it('FirestoreFieldValue.arrayRemove()', function () { + fieldValueV9Deprecation( + () => arrayRemove('foo'), + () => firestore.FieldValue.arrayRemove('bar'), + 'arrayRemove', + ); + }); }); }); diff --git a/packages/firestore/lib/FirestoreStatics.js b/packages/firestore/lib/FirestoreStatics.js index 8f98c714a8..fefd74b0a8 100644 --- a/packages/firestore/lib/FirestoreStatics.js +++ b/packages/firestore/lib/FirestoreStatics.js @@ -15,6 +15,7 @@ * */ +import { createDeprecationProxy } from '@react-native-firebase/app/lib/common'; import { getReactNativeModule } from '@react-native-firebase/app/lib/internal/nativeModule'; import FirestoreBlob from './FirestoreBlob'; import FirestoreFieldPath from './FirestoreFieldPath'; @@ -25,7 +26,7 @@ import { Filter } from './FirestoreFilter'; export default { Blob: FirestoreBlob, FieldPath: FirestoreFieldPath, - FieldValue: FirestoreFieldValue, + FieldValue: createDeprecationProxy(FirestoreFieldValue), GeoPoint: FirestoreGeoPoint, Timestamp: FirestoreTimestamp, Filter: Filter, From 0d752d3a6d8c3a41494eea1b087249b2588b4043 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Fri, 6 Dec 2024 11:27:28 +0000 Subject: [PATCH 12/29] rm code comments --- packages/firestore/__tests__/firestore.test.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/firestore/__tests__/firestore.test.ts b/packages/firestore/__tests__/firestore.test.ts index afc54c395a..b7bafed7b5 100644 --- a/packages/firestore/__tests__/firestore.test.ts +++ b/packages/firestore/__tests__/firestore.test.ts @@ -711,13 +711,11 @@ describe('Firestore', function () { }); describe('test `console.warn` is called for RNFB v8 API & not called for v9 API', function () { - // let firestoreV9Deprecation: CheckV9DeprecationFunction; let collectionRefV9Deprecation: CheckV9DeprecationFunction; let docRefV9Deprecation: CheckV9DeprecationFunction; let fieldValueV9Deprecation: CheckV9DeprecationFunction; beforeEach(function () { - // firestoreV9Deprecation = createCheckV9Deprecation(['firestore']); collectionRefV9Deprecation = createCheckV9Deprecation([ 'firestore', 'FirestoreCollectionReference', @@ -748,12 +746,8 @@ describe('Firestore', function () { .spyOn(FirestoreQuery.prototype, '_handleQueryCursor') // @ts-ignore test .mockImplementation((cursor, docOrField, fields) => { - // Mock implementation returning an empty array or any other desired value - return []; // or any other mock value you want to return + return []; }); - - // jest.spyOn(FirestoreQuerySnapshot.prototype, 'constructor'); - // @ts-ignore test }); it('CollectionReference.count()', function () { From b42375660a83c2df3f431d4d667dabd0572105d1 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Fri, 6 Dec 2024 13:51:11 +0000 Subject: [PATCH 13/29] FirestoreFilter --- packages/app/lib/common/index.js | 8 ++++++ .../firestore/__tests__/firestore.test.ts | 26 +++++++++++++++++++ packages/firestore/lib/FirestoreStatics.js | 2 +- 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/packages/app/lib/common/index.js b/packages/app/lib/common/index.js index 33a402394f..90f86b6650 100644 --- a/packages/app/lib/common/index.js +++ b/packages/app/lib/common/index.js @@ -159,6 +159,10 @@ const mapOfDeprecationReplacements = { increment: 'increment()', serverTimestamp: 'serverTimestamp()', }, + Filter: { + or: 'or()', + and: 'and()', + }, }, }; @@ -243,6 +247,10 @@ export function createDeprecationProxy(instance) { const instanceName = getInstanceName(target); const nameSpace = getNamespace(instanceName); + console.log('Prop', prop); + console.log('modularMethod', isModularMethod); + console.log('nameSpace', nameSpace); + console.log('instanceName', instanceName); deprecationConsoleWarning(nameSpace, prop, instanceName, isModularMethod); diff --git a/packages/firestore/__tests__/firestore.test.ts b/packages/firestore/__tests__/firestore.test.ts index b7bafed7b5..33cb7ba054 100644 --- a/packages/firestore/__tests__/firestore.test.ts +++ b/packages/firestore/__tests__/firestore.test.ts @@ -714,6 +714,7 @@ describe('Firestore', function () { let collectionRefV9Deprecation: CheckV9DeprecationFunction; let docRefV9Deprecation: CheckV9DeprecationFunction; let fieldValueV9Deprecation: CheckV9DeprecationFunction; + let filterV9Deprecation: CheckV9DeprecationFunction; beforeEach(function () { collectionRefV9Deprecation = createCheckV9Deprecation([ @@ -724,6 +725,7 @@ describe('Firestore', function () { docRefV9Deprecation = createCheckV9Deprecation(['firestore', 'FirestoreDocumentReference']); fieldValueV9Deprecation = createCheckV9Deprecation(['firestore', 'FirestoreFieldValue']); + filterV9Deprecation = createCheckV9Deprecation(['firestore', 'Filter']); // @ts-ignore test jest.spyOn(FirebaseModule.prototype, 'native', 'get').mockImplementation(() => { @@ -1077,5 +1079,29 @@ describe('Firestore', function () { 'arrayRemove', ); }); + + it('Filter.or()', function () { + filterV9Deprecation( + () => {}, + () => + firestore.Filter.or( + firestore.Filter('foo', '==', 'bar'), + firestore.Filter('baz', '==', 'qux'), + ), + 'or', + ); + }); + + it('Filter.and()', function () { + filterV9Deprecation( + () => {}, + () => + firestore.Filter.and( + firestore.Filter('foo', '==', 'bar'), + firestore.Filter('baz', '==', 'qux'), + ), + 'and', + ); + }); }); }); diff --git a/packages/firestore/lib/FirestoreStatics.js b/packages/firestore/lib/FirestoreStatics.js index fefd74b0a8..bac6b402d3 100644 --- a/packages/firestore/lib/FirestoreStatics.js +++ b/packages/firestore/lib/FirestoreStatics.js @@ -29,7 +29,7 @@ export default { FieldValue: createDeprecationProxy(FirestoreFieldValue), GeoPoint: FirestoreGeoPoint, Timestamp: FirestoreTimestamp, - Filter: Filter, + Filter: createDeprecationProxy(Filter), CACHE_SIZE_UNLIMITED: -1, From 3658b6c94024043bfb3398c9eb3ed34c924c15e6 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Fri, 6 Dec 2024 14:02:26 +0000 Subject: [PATCH 14/29] where query --- packages/firestore/__tests__/firestore.test.ts | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/firestore/__tests__/firestore.test.ts b/packages/firestore/__tests__/firestore.test.ts index 33cb7ba054..9a22d3d29d 100644 --- a/packages/firestore/__tests__/firestore.test.ts +++ b/packages/firestore/__tests__/firestore.test.ts @@ -1082,7 +1082,7 @@ describe('Firestore', function () { it('Filter.or()', function () { filterV9Deprecation( - () => {}, + () => or(where('foo.bar', '==', null), where('foo.bar', '==', null)), () => firestore.Filter.or( firestore.Filter('foo', '==', 'bar'), @@ -1094,7 +1094,7 @@ describe('Firestore', function () { it('Filter.and()', function () { filterV9Deprecation( - () => {}, + () => and(where('foo.bar', '==', null), where('foo.bar', '==', null)), () => firestore.Filter.and( firestore.Filter('foo', '==', 'bar'), @@ -1103,5 +1103,17 @@ describe('Firestore', function () { 'and', ); }); + + // it('FirestoreGeoPoint.and()', function () { + // filterV9Deprecation( + // () => or(firestore.Filter('foo.bar', '==', null), firestore.Filter('foo.bar', '==', null)), + // () => + // firestore.Filter.and( + // firestore.Filter('foo', '==', 'bar'), + // firestore.Filter('baz', '==', 'qux'), + // ), + // 'and', + // ); + // }); }); }); From 323143c7d13f094f02b3ba3f41bd86d45498df24 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Thu, 12 Dec 2024 12:11:12 +0000 Subject: [PATCH 15/29] update default firestore --- packages/app/lib/common/index.js | 16 +++++++++------- packages/app/lib/internal/registry/namespace.js | 8 ++++---- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/packages/app/lib/common/index.js b/packages/app/lib/common/index.js index 90f86b6650..b1602fa4c4 100644 --- a/packages/app/lib/common/index.js +++ b/packages/app/lib/common/index.js @@ -123,6 +123,9 @@ const mapOfDeprecationReplacements = { }, }, firestore: { + default: { + batch: 'writeBatch()', + }, FirestoreCollectionReference: { count: 'getCountFromServer()', countFromServer: 'getCountFromServer()', @@ -211,7 +214,11 @@ export function createMessage( } } -function getNamespace(className) { +function getNamespace(target) { + if (target._config && target._config.namespace) { + return target._config.namespace; + } + const className = target.name ? target.name : target.constructor.name; return Object.keys(mapOfDeprecationReplacements).find(key => { if (mapOfDeprecationReplacements[key][className]) { return key; @@ -245,12 +252,7 @@ export function createDeprecationProxy(instance) { return function (...args) { const isModularMethod = args.includes(MODULAR_DEPRECATION_ARG); const instanceName = getInstanceName(target); - const nameSpace = getNamespace(instanceName); - - console.log('Prop', prop); - console.log('modularMethod', isModularMethod); - console.log('nameSpace', nameSpace); - console.log('instanceName', instanceName); + const nameSpace = getNamespace(target); deprecationConsoleWarning(nameSpace, prop, instanceName, isModularMethod); diff --git a/packages/app/lib/internal/registry/namespace.js b/packages/app/lib/internal/registry/namespace.js index 48013eadf4..6665be71c3 100644 --- a/packages/app/lib/internal/registry/namespace.js +++ b/packages/app/lib/internal/registry/namespace.js @@ -114,11 +114,11 @@ function getOrCreateModuleForApp(app, moduleNamespace) { } if (!APP_MODULE_INSTANCE[app.name][key]) { - APP_MODULE_INSTANCE[app.name][key] = new ModuleClass( - app, - NAMESPACE_REGISTRY[moduleNamespace], - customUrlOrRegionOrDatabaseId, + const module = createDeprecationProxy( + new ModuleClass(app, NAMESPACE_REGISTRY[moduleNamespace], customUrlOrRegionOrDatabaseId), ); + + APP_MODULE_INSTANCE[app.name][key] = module; } return APP_MODULE_INSTANCE[app.name][key]; From af75b1311cd5db83d60f085587c8f89c0c9358a2 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Thu, 12 Dec 2024 13:00:56 +0000 Subject: [PATCH 16/29] fix broken firestore tests --- packages/app/lib/common/index.js | 6 +++- packages/app/lib/common/unitTestUtils.ts | 2 +- .../firestore/__tests__/firestore.test.ts | 29 +++++++++++++----- packages/firestore/lib/modular/index.js | 30 +++++++++++-------- packages/firestore/lib/modular/snapshot.js | 4 ++- 5 files changed, 48 insertions(+), 23 deletions(-) diff --git a/packages/app/lib/common/index.js b/packages/app/lib/common/index.js index b1602fa4c4..aaddb1b952 100644 --- a/packages/app/lib/common/index.js +++ b/packages/app/lib/common/index.js @@ -256,7 +256,11 @@ export function createDeprecationProxy(instance) { deprecationConsoleWarning(nameSpace, prop, instanceName, isModularMethod); - return originalMethod.apply(target, args); + return originalMethod.apply( + target, + // Remove the modular deprecation argument from the arguments list + args.filter(arg => arg !== MODULAR_DEPRECATION_ARG), + ); }; } return Reflect.get(target, prop, receiver); diff --git a/packages/app/lib/common/unitTestUtils.ts b/packages/app/lib/common/unitTestUtils.ts index 7753da7205..13532b6737 100644 --- a/packages/app/lib/common/unitTestUtils.ts +++ b/packages/app/lib/common/unitTestUtils.ts @@ -32,8 +32,8 @@ export const createCheckV9Deprecation = (moduleNames: string[]): CheckV9Deprecat const moduleName = moduleNames[0]; // firestore, database, etc const instanceName = moduleNames[1] || 'default'; // default, FirestoreCollectionReference, etc const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + // Do not call `mockRestore()` as it removes the spy consoleWarnSpy.mockReset(); - consoleWarnSpy.mockRestore(); modularFunction(); expect(consoleWarnSpy).not.toHaveBeenCalled(); consoleWarnSpy.mockReset(); diff --git a/packages/firestore/__tests__/firestore.test.ts b/packages/firestore/__tests__/firestore.test.ts index 9a22d3d29d..8c7b6bc41a 100644 --- a/packages/firestore/__tests__/firestore.test.ts +++ b/packages/firestore/__tests__/firestore.test.ts @@ -715,8 +715,11 @@ describe('Firestore', function () { let docRefV9Deprecation: CheckV9DeprecationFunction; let fieldValueV9Deprecation: CheckV9DeprecationFunction; let filterV9Deprecation: CheckV9DeprecationFunction; + // let persistentCacheIndexManagerV9Deprecation: CheckV9DeprecationFunction; + let firestoreRefV9Deprecation: CheckV9DeprecationFunction; beforeEach(function () { + firestoreRefV9Deprecation = createCheckV9Deprecation(['firestore']); collectionRefV9Deprecation = createCheckV9Deprecation([ 'firestore', 'FirestoreCollectionReference', @@ -726,6 +729,10 @@ describe('Firestore', function () { fieldValueV9Deprecation = createCheckV9Deprecation(['firestore', 'FirestoreFieldValue']); filterV9Deprecation = createCheckV9Deprecation(['firestore', 'Filter']); + // persistentCacheIndexManagerV9Deprecation = createCheckV9Deprecation([ + // 'firestore', + // 'FirestorePersistentCacheIndexManager', + // ]); // @ts-ignore test jest.spyOn(FirebaseModule.prototype, 'native', 'get').mockImplementation(() => { @@ -752,6 +759,15 @@ describe('Firestore', function () { }); }); + it('firestore.batch()', function () { + const firestore = getFirestore(); + firestoreRefV9Deprecation( + () => writeBatch(firestore), + () => firestore.batch(), + 'batch', + ); + }); + it('CollectionReference.count()', function () { const firestore = getFirestore(); @@ -1104,14 +1120,11 @@ describe('Firestore', function () { ); }); - // it('FirestoreGeoPoint.and()', function () { - // filterV9Deprecation( - // () => or(firestore.Filter('foo.bar', '==', null), firestore.Filter('foo.bar', '==', null)), - // () => - // firestore.Filter.and( - // firestore.Filter('foo', '==', 'bar'), - // firestore.Filter('baz', '==', 'qux'), - // ), + // it.only('FirestorePersistentCacheIndexManager.enableIndexAutoCreation()', function () { + // const firestore = getFirestore(); + // persistentCacheIndexManagerV9Deprecation( + // () => firestore.persistentCacheIndexManager(), + // () => firestore.GeoPoint, // 'and', // ); // }); diff --git a/packages/firestore/lib/modular/index.js b/packages/firestore/lib/modular/index.js index 70618d13b0..db506dfef2 100644 --- a/packages/firestore/lib/modular/index.js +++ b/packages/firestore/lib/modular/index.js @@ -54,7 +54,7 @@ export function doc(parent, path, ...pathSegments) { path = path + '/' + pathSegments.map(e => e.replace(/^\/|\/$/g, '')).join('/'); } - return parent.doc(path); + return parent.doc.call(parent, path, MODULAR_DEPRECATION_ARG); } /** @@ -87,7 +87,7 @@ export function collectionGroup(firestore, collectionId) { * @returns {Promise} */ export function setDoc(reference, data, options) { - return reference.set(data, options); + return reference.set.call(reference, data, options, MODULAR_DEPRECATION_ARG); } /** @@ -100,18 +100,24 @@ export function setDoc(reference, data, options) { export function updateDoc(reference, fieldOrUpdateData, value, ...moreFieldsAndValues) { if (!fieldOrUpdateData) { // @ts-ignore - return reference.update(); + return reference.update.call(reference, MODULAR_DEPRECATION_ARG); } if (!value) { - return reference.update(fieldOrUpdateData); + return reference.update.call(reference, fieldOrUpdateData, MODULAR_DEPRECATION_ARG); } if (!moreFieldsAndValues || !Array.isArray(moreFieldsAndValues)) { - return reference.update(fieldOrUpdateData, value); + return reference.update.call(reference, fieldOrUpdateData, value, MODULAR_DEPRECATION_ARG); } - return reference.update(fieldOrUpdateData, value, ...moreFieldsAndValues); + return reference.update.call( + reference, + fieldOrUpdateData, + value, + ...moreFieldsAndValues, + MODULAR_DEPRECATION_ARG, + ); } /** @@ -120,7 +126,7 @@ export function updateDoc(reference, fieldOrUpdateData, value, ...moreFieldsAndV * @returns {Promise} */ export function addDoc(reference, data) { - return reference.add(data); + return reference.add.call(reference, data, MODULAR_DEPRECATION_ARG); } /** @@ -198,7 +204,7 @@ export function runTransaction(firestore, updateFunction) { * @returns {Promise} */ export function getCountFromServer(query) { - return query.count().get.call(query, MODULAR_DEPRECATION_ARG); + return query.count.call(query, MODULAR_DEPRECATION_ARG).get(); } export function getAggregateFromServer(query, aggregateSpec) { @@ -309,7 +315,7 @@ export function namedQuery(firestore, name) { * @returns {FirebaseFirestoreTypes.WriteBatch} */ export function writeBatch(firestore) { - return firestore.batch(); + return firestore.batch.call(firestore, MODULAR_DEPRECATION_ARG); } /** @@ -332,7 +338,7 @@ export function getPersistentCacheIndexManager(firestore) { * @returns {Promise} */ export function onSnapshot(reference, ...args) { - return reference.onSnapshot(...args); + return reference.onSnapshot.call(reference, ...args, MODULAR_DEPRECATION_ARG); } From b7180da83bc64b5341acc35223a309dd2205252b Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Thu, 12 Dec 2024 13:36:47 +0000 Subject: [PATCH 17/29] more firestore method deprecated --- packages/app/lib/common/index.js | 3 ++ .../firestore/__tests__/firestore.test.ts | 34 +++++++++++++++++++ packages/firestore/lib/modular/index.d.ts | 9 +++++ packages/firestore/lib/modular/index.js | 12 +++++-- 4 files changed, 56 insertions(+), 2 deletions(-) diff --git a/packages/app/lib/common/index.js b/packages/app/lib/common/index.js index aaddb1b952..dca7103abe 100644 --- a/packages/app/lib/common/index.js +++ b/packages/app/lib/common/index.js @@ -125,6 +125,9 @@ const mapOfDeprecationReplacements = { firestore: { default: { batch: 'writeBatch()', + loadBundle: 'loadBundle()', + namedQuery: 'namedQuery()', + clearPersistence: 'clearIndexedDbPersistence()', }, FirestoreCollectionReference: { count: 'getCountFromServer()', diff --git a/packages/firestore/__tests__/firestore.test.ts b/packages/firestore/__tests__/firestore.test.ts index 8c7b6bc41a..f50b8294a0 100644 --- a/packages/firestore/__tests__/firestore.test.ts +++ b/packages/firestore/__tests__/firestore.test.ts @@ -30,6 +30,7 @@ import firestore, { enableNetwork, disableNetwork, clearPersistence, + clearIndexedDbPersistence, terminate, waitForPendingWrites, initializeFirestore, @@ -768,6 +769,39 @@ describe('Firestore', function () { ); }); + it('firestore.loadBundle()', function () { + const firestore = getFirestore(); + firestoreRefV9Deprecation( + () => loadBundle(firestore, 'some bundle'), + () => firestore.loadBundle('some bundle'), + 'loadBundle', + ); + }); + + it('firestore.namedQuery()', function () { + const firestore = getFirestore(); + firestoreRefV9Deprecation( + () => namedQuery(firestore, 'some name'), + () => firestore.namedQuery('some name'), + 'namedQuery', + ); + }); + + it('firestore.clearPersistence()', function () { + const firestore = getFirestore(); + firestoreRefV9Deprecation( + () => clearIndexedDbPersistence(firestore), + () => firestore.clearPersistence(), + 'clearPersistence', + ); + // Deprecating the modular method clearPersistence() as it doesn't exist on firebase-js-sdk + firestoreRefV9Deprecation( + () => clearIndexedDbPersistence(firestore), + () => clearPersistence(firestore), + 'clearPersistence', + ); + }); + it('CollectionReference.count()', function () { const firestore = getFirestore(); diff --git a/packages/firestore/lib/modular/index.d.ts b/packages/firestore/lib/modular/index.d.ts index df51ad2ace..19f444eaab 100644 --- a/packages/firestore/lib/modular/index.d.ts +++ b/packages/firestore/lib/modular/index.d.ts @@ -373,10 +373,19 @@ export function disableNetwork(firestore: Firestore): Promise; * Aimed primarily at clearing up any data cached from running tests. Needs to be executed before any database calls * are made. * + * Deprecated, please use `clearIndexedDbPersistence` instead. * @param firestore - A reference to the root `Firestore` instance. */ export function clearPersistence(firestore: Firestore): Promise; +/** + * Aimed primarily at clearing up any data cached from running tests. Needs to be executed before any database calls + * are made. + * + * @param firestore - A reference to the root `Firestore` instance. + */ +export function clearIndexedDbPersistence(firestore: Firestore): Promise; + /** * Terminates the provided {@link Firestore} instance. * diff --git a/packages/firestore/lib/modular/index.js b/packages/firestore/lib/modular/index.js index db506dfef2..00e2d93512 100644 --- a/packages/firestore/lib/modular/index.js +++ b/packages/firestore/lib/modular/index.js @@ -150,8 +150,16 @@ export function disableNetwork(firestore) { * @returns {Promise} */ export function clearPersistence(firestore) { + // this will call deprecation warning as it isn't part of firebase-js-sdk API return firestore.clearPersistence(); } +/** + * @param {Firestore} firestore + * @returns {Promise} + */ +export function clearIndexedDbPersistence(firestore) { + return firestore.clearPersistence.call(firestore, MODULAR_DEPRECATION_ARG); +} /** * @param {Firestore} firestore @@ -298,7 +306,7 @@ export function count() { * @returns {import('.').LoadBundleTask} */ export function loadBundle(firestore, bundleData) { - return firestore.loadBundle(bundleData); + return firestore.loadBundle.call(firestore, bundleData, MODULAR_DEPRECATION_ARG); } /** @@ -307,7 +315,7 @@ export function loadBundle(firestore, bundleData) { * @returns {Query} */ export function namedQuery(firestore, name) { - return firestore.namedQuery(name); + return firestore.namedQuery.call(firestore, name, MODULAR_DEPRECATION_ARG); } /** From 65625572d841fa97e31a49e28179ce87f741c7d6 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Thu, 12 Dec 2024 14:12:34 +0000 Subject: [PATCH 18/29] add connectFirestoreEmulator + few more deprecations --- packages/app/lib/common/index.js | 3 + .../firestore/__tests__/firestore.test.ts | 28 +++++++ packages/firestore/lib/modular/index.d.ts | 77 +++++++++++++++++++ packages/firestore/lib/modular/index.js | 8 +- 4 files changed, 114 insertions(+), 2 deletions(-) diff --git a/packages/app/lib/common/index.js b/packages/app/lib/common/index.js index dca7103abe..a03b168498 100644 --- a/packages/app/lib/common/index.js +++ b/packages/app/lib/common/index.js @@ -128,6 +128,9 @@ const mapOfDeprecationReplacements = { loadBundle: 'loadBundle()', namedQuery: 'namedQuery()', clearPersistence: 'clearIndexedDbPersistence()', + waitForPendingWrites: 'waitForPendingWrites()', + terminate: 'terminate()', + useEmulator: 'connectFirestoreEmulator()', }, FirestoreCollectionReference: { count: 'getCountFromServer()', diff --git a/packages/firestore/__tests__/firestore.test.ts b/packages/firestore/__tests__/firestore.test.ts index f50b8294a0..a32f0042f4 100644 --- a/packages/firestore/__tests__/firestore.test.ts +++ b/packages/firestore/__tests__/firestore.test.ts @@ -15,6 +15,7 @@ import { import firestore, { firebase, + connectFirestoreEmulator, Filter, getFirestore, getAggregateFromServer, @@ -802,6 +803,33 @@ describe('Firestore', function () { ); }); + it('firestore.waitForPendingWrites()', function () { + const firestore = getFirestore(); + firestoreRefV9Deprecation( + () => waitForPendingWrites(firestore), + () => firestore.waitForPendingWrites(), + 'waitForPendingWrites', + ); + }); + + it('firestore.terminate()', function () { + const firestore = getFirestore(); + firestoreRefV9Deprecation( + () => terminate(firestore), + () => firestore.terminate(), + 'terminate', + ); + }); + + it('firestore.useEmulator()', function () { + const firestore = getFirestore(); + firestoreRefV9Deprecation( + () => connectFirestoreEmulator(firestore, 'localhost', 8080), + () => firestore.useEmulator('localhost', 8080), + 'useEmulator', + ); + }); + it('CollectionReference.count()', function () { const firestore = getFirestore(); diff --git a/packages/firestore/lib/modular/index.d.ts b/packages/firestore/lib/modular/index.d.ts index 19f444eaab..b91d31b5a4 100644 --- a/packages/firestore/lib/modular/index.d.ts +++ b/packages/firestore/lib/modular/index.d.ts @@ -96,6 +96,9 @@ export type WithFieldValue = ? { [K in keyof T]: WithFieldValue | FieldValue } : never); +export type EmulatorMockTokenOptions = ({ user_id: string } | { sub: string }) & + Partial; + /** * Returns the existing default {@link Firestore} instance that is associated with the * default {@link @firebase/app#FirebaseApp}. If no instance exists, initializes a new @@ -131,6 +134,24 @@ export function getFirestore(app?: FirebaseApp): Firestore; */ export declare function getFirestore(app?: FirebaseApp, databaseId?: string): Firestore; +/** + * Modify this instance to communicate with the Cloud Firestore emulator. + * + * @param firestore - A reference to the root `Firestore` instance. + * instance is associated with. + * @param host: emulator host (eg, 'localhost') + * @param port: emulator port (eg, 8080) + * @param options.mockUserToken - the mock auth token to use for unit testing + * @returns void. + */ +export declare function connectFirestoreEmulator( + firestore: Firestore, + host: string, + port: number, + options?: { + mockUserToken?: EmulatorMockTokenOptions | string; + }, +): void; /** * Gets a `DocumentReference` instance that refers to the document at the * specified absolute path. @@ -512,6 +533,62 @@ interface AggregateSpec { [field: string]: AggregateFieldType; } +interface FirebaseIdToken { + // Always set to https://securetoken.google.com/PROJECT_ID + iss: string; + + // Always set to PROJECT_ID + aud: string; + + // The user's unique ID + sub: string; + + // The token issue time, in seconds since epoch + iat: number; + + // The token expiry time, normally 'iat' + 3600 + exp: number; + + // The user's unique ID. Must be equal to 'sub' + user_id: string; + + // The time the user authenticated, normally 'iat' + auth_time: number; + + // The sign in provider, only set when the provider is 'anonymous' + provider_id?: 'anonymous'; + + // The user's primary email + email?: string; + + // The user's email verification status + email_verified?: boolean; + + // The user's primary phone number + phone_number?: string; + + // The user's display name + name?: string; + + // The user's profile photo URL + picture?: string; + + // Information on all identities linked to this user + firebase: { + // The primary sign-in provider + sign_in_provider: FirebaseSignInProvider; + + // A map of providers to the user's list of unique identifiers from + // each provider + identities?: { [provider in FirebaseSignInProvider]?: string[] }; + }; + + // Custom claims set by the developer + [claim: string]: unknown; + + uid?: never; // Try to catch a common mistake of "uid" (should be "sub" instead). +} + /** * The union of all `AggregateField` types that are supported by Firestore. */ diff --git a/packages/firestore/lib/modular/index.js b/packages/firestore/lib/modular/index.js index 00e2d93512..5bdd129ebf 100644 --- a/packages/firestore/lib/modular/index.js +++ b/packages/firestore/lib/modular/index.js @@ -166,7 +166,7 @@ export function clearIndexedDbPersistence(firestore) { * @returns {Promise} */ export function terminate(firestore) { - return firestore.terminate(); + return firestore.terminate.call(firestore, MODULAR_DEPRECATION_ARG); } /** @@ -174,7 +174,7 @@ export function terminate(firestore) { * @returns {Promise} */ export function waitForPendingWrites(firestore) { - return firestore.waitForPendingWrites(); + return firestore.waitForPendingWrites.call(firestore, MODULAR_DEPRECATION_ARG); } /** @@ -190,6 +190,10 @@ export async function initializeFirestore(app, settings /* databaseId */) { return firestore; } +export function connectFirestoreEmulator(firestore, host, port, options) { + return firestore.useEmulator.call(firestore, host, port, options, MODULAR_DEPRECATION_ARG); +} + /** * @param {import('./').LogLevel} logLevel * @returns {void} From c670dcf8067d4dd34ed2dde8aeef24d2fbe5a468 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Thu, 12 Dec 2024 14:32:26 +0000 Subject: [PATCH 19/29] more firestore.* api deprecations --- packages/app/lib/common/index.js | 5 +++ .../firestore/__tests__/firestore.test.ts | 45 +++++++++++++++++++ packages/firestore/lib/modular/index.js | 8 ++-- 3 files changed, 54 insertions(+), 4 deletions(-) diff --git a/packages/app/lib/common/index.js b/packages/app/lib/common/index.js index a03b168498..638510cb04 100644 --- a/packages/app/lib/common/index.js +++ b/packages/app/lib/common/index.js @@ -131,6 +131,11 @@ const mapOfDeprecationReplacements = { waitForPendingWrites: 'waitForPendingWrites()', terminate: 'terminate()', useEmulator: 'connectFirestoreEmulator()', + collection: 'collection()', + collectionGroup: 'collectionGroup()', + disableNetwork: 'disableNetwork()', + doc: 'doc()', + enableNetwork: 'enableNetwork()', }, FirestoreCollectionReference: { count: 'getCountFromServer()', diff --git a/packages/firestore/__tests__/firestore.test.ts b/packages/firestore/__tests__/firestore.test.ts index a32f0042f4..d8987ca49d 100644 --- a/packages/firestore/__tests__/firestore.test.ts +++ b/packages/firestore/__tests__/firestore.test.ts @@ -830,6 +830,51 @@ describe('Firestore', function () { ); }); + it('firestore.collection()', function () { + const firestore = getFirestore(); + firestoreRefV9Deprecation( + () => collection(firestore, 'collection'), + () => firestore.collection('collection'), + 'collection', + ); + }); + + it('firestore.collectionGroup()', function () { + const firestore = getFirestore(); + firestoreRefV9Deprecation( + () => collectionGroup(firestore, 'collection'), + () => firestore.collectionGroup('collection'), + 'collectionGroup', + ); + }); + + it('firestore.disableNetwork()', function () { + const firestore = getFirestore(); + firestoreRefV9Deprecation( + () => disableNetwork(firestore), + () => firestore.disableNetwork(), + 'disableNetwork', + ); + }); + + it('firestore.doc()', function () { + const firestore = getFirestore(); + firestoreRefV9Deprecation( + () => doc(firestore, 'collection/path'), + () => firestore.doc('collection/path'), + 'doc', + ); + }); + + it('firestore.enableNetwork()', function () { + const firestore = getFirestore(); + firestoreRefV9Deprecation( + () => enableNetwork(firestore), + () => firestore.enableNetwork(), + 'enableNetwork', + ); + }); + it('CollectionReference.count()', function () { const firestore = getFirestore(); diff --git a/packages/firestore/lib/modular/index.js b/packages/firestore/lib/modular/index.js index 5bdd129ebf..f231c06f94 100644 --- a/packages/firestore/lib/modular/index.js +++ b/packages/firestore/lib/modular/index.js @@ -68,7 +68,7 @@ export function collection(parent, path, ...pathSegments) { path = path + '/' + pathSegments.map(e => e.replace(/^\/|\/$/g, '')).join('/'); } - return parent.collection(path); + return parent.collection.call(parent, path, MODULAR_DEPRECATION_ARG); } /** @@ -77,7 +77,7 @@ export function collection(parent, path, ...pathSegments) { * @returns {Query} */ export function collectionGroup(firestore, collectionId) { - return firestore.collectionGroup(collectionId); + return firestore.collectionGroup.call(firestore, collectionId, MODULAR_DEPRECATION_ARG); } /** @@ -134,7 +134,7 @@ export function addDoc(reference, data) { * @returns {Promise} */ export function enableNetwork(firestore) { - return firestore.enableNetwork(); + return firestore.enableNetwork.call(firestore, MODULAR_DEPRECATION_ARG); } /** @@ -142,7 +142,7 @@ export function enableNetwork(firestore) { * @returns {Promise} */ export function disableNetwork(firestore) { - return firestore.disableNetwork(); + return firestore.disableNetwork.call(firestore, MODULAR_DEPRECATION_ARG); } /** From fbf42c0fa1a49ca29747754c7ff6a1190502f6e9 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Thu, 12 Dec 2024 14:45:00 +0000 Subject: [PATCH 20/29] further firestore.* deprecations --- packages/app/lib/common/index.js | 3 ++ .../firestore/__tests__/firestore.test.ts | 30 +++++++++++++++++++ packages/firestore/lib/modular/index.js | 6 ++-- 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/packages/app/lib/common/index.js b/packages/app/lib/common/index.js index 638510cb04..94301e75e2 100644 --- a/packages/app/lib/common/index.js +++ b/packages/app/lib/common/index.js @@ -136,6 +136,9 @@ const mapOfDeprecationReplacements = { disableNetwork: 'disableNetwork()', doc: 'doc()', enableNetwork: 'enableNetwork()', + runTransaction: 'runTransaction()', + settings: 'settings()', + persistentCacheIndexManager: 'getPersistentCacheIndexManager()', }, FirestoreCollectionReference: { count: 'getCountFromServer()', diff --git a/packages/firestore/__tests__/firestore.test.ts b/packages/firestore/__tests__/firestore.test.ts index d8987ca49d..a8d12eef84 100644 --- a/packages/firestore/__tests__/firestore.test.ts +++ b/packages/firestore/__tests__/firestore.test.ts @@ -875,6 +875,36 @@ describe('Firestore', function () { ); }); + it('firestore.runTransaction()', function () { + const firestore = getFirestore(); + firestoreRefV9Deprecation( + () => runTransaction(firestore, async () => {}), + () => firestore.runTransaction(async () => {}), + 'runTransaction', + ); + }); + + it('firestore.settings()', function () { + const firestore = getFirestore(); + const app = firebase.app(); + firestoreRefV9Deprecation( + // no equivalent settings method for firebase-js-sdk + () => initializeFirestore(app, {}), + () => firestore.settings({}), + 'settings', + ); + }); + + it('firestore.persistentCacheIndexManager()', function () { + const firestore = getFirestore(); + + firestoreRefV9Deprecation( + () => getPersistentCacheIndexManager(firestore), + () => firestore.persistentCacheIndexManager(), + 'persistentCacheIndexManager', + ); + }); + it('CollectionReference.count()', function () { const firestore = getFirestore(); diff --git a/packages/firestore/lib/modular/index.js b/packages/firestore/lib/modular/index.js index f231c06f94..8685bcf186 100644 --- a/packages/firestore/lib/modular/index.js +++ b/packages/firestore/lib/modular/index.js @@ -186,7 +186,7 @@ export function waitForPendingWrites(firestore) { export async function initializeFirestore(app, settings /* databaseId */) { // TODO(exaby73): implement 2nd database once it's supported const firestore = firebase.firestore(app); - await firestore.settings(settings); + await firestore.settings.call(firestore, settings, MODULAR_DEPRECATION_ARG); return firestore; } @@ -208,7 +208,7 @@ export function setLogLevel(logLevel) { * @returns {Promise} */ export function runTransaction(firestore, updateFunction) { - return firestore.runTransaction(updateFunction); + return firestore.runTransaction.call(firestore, updateFunction, MODULAR_DEPRECATION_ARG); } /** @@ -339,7 +339,7 @@ export function writeBatch(firestore) { * @returns {PersistentCacheIndexManager | null} */ export function getPersistentCacheIndexManager(firestore) { - return firestore.persistentCacheIndexManager(); + return firestore.persistentCacheIndexManager.call(firestore, MODULAR_DEPRECATION_ARG); } /** From 4467e54595c6a4f5c719e5d65bedc75a110e7a08 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Thu, 12 Dec 2024 14:54:25 +0000 Subject: [PATCH 21/29] FirestorePersistentCacheIndexManager.* deprecations --- packages/app/lib/common/index.js | 5 ++ .../firestore/__tests__/firestore.test.ts | 47 ++++++++++++++----- 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/packages/app/lib/common/index.js b/packages/app/lib/common/index.js index 94301e75e2..264172bb85 100644 --- a/packages/app/lib/common/index.js +++ b/packages/app/lib/common/index.js @@ -180,6 +180,11 @@ const mapOfDeprecationReplacements = { or: 'or()', and: 'and()', }, + FirestorePersistentCacheIndexManager: { + enableIndexAutoCreation: 'enablePersistentCacheIndexAutoCreation()', + disableIndexAutoCreation: 'disablePersistentCacheIndexAutoCreation()', + deleteAllIndexes: 'deleteAllPersistentCacheIndexes()', + }, }, }; diff --git a/packages/firestore/__tests__/firestore.test.ts b/packages/firestore/__tests__/firestore.test.ts index a8d12eef84..4757766397 100644 --- a/packages/firestore/__tests__/firestore.test.ts +++ b/packages/firestore/__tests__/firestore.test.ts @@ -717,7 +717,7 @@ describe('Firestore', function () { let docRefV9Deprecation: CheckV9DeprecationFunction; let fieldValueV9Deprecation: CheckV9DeprecationFunction; let filterV9Deprecation: CheckV9DeprecationFunction; - // let persistentCacheIndexManagerV9Deprecation: CheckV9DeprecationFunction; + let persistentCacheIndexManagerV9Deprecation: CheckV9DeprecationFunction; let firestoreRefV9Deprecation: CheckV9DeprecationFunction; beforeEach(function () { @@ -731,10 +731,10 @@ describe('Firestore', function () { fieldValueV9Deprecation = createCheckV9Deprecation(['firestore', 'FirestoreFieldValue']); filterV9Deprecation = createCheckV9Deprecation(['firestore', 'Filter']); - // persistentCacheIndexManagerV9Deprecation = createCheckV9Deprecation([ - // 'firestore', - // 'FirestorePersistentCacheIndexManager', - // ]); + persistentCacheIndexManagerV9Deprecation = createCheckV9Deprecation([ + 'firestore', + 'FirestorePersistentCacheIndexManager', + ]); // @ts-ignore test jest.spyOn(FirebaseModule.prototype, 'native', 'get').mockImplementation(() => { @@ -1257,13 +1257,34 @@ describe('Firestore', function () { ); }); - // it.only('FirestorePersistentCacheIndexManager.enableIndexAutoCreation()', function () { - // const firestore = getFirestore(); - // persistentCacheIndexManagerV9Deprecation( - // () => firestore.persistentCacheIndexManager(), - // () => firestore.GeoPoint, - // 'and', - // ); - // }); + it('FirestorePersistentCacheIndexManager.enableIndexAutoCreation()', function () { + const firestore = getFirestore(); + const indexManager = firestore.persistentCacheIndexManager(); + persistentCacheIndexManagerV9Deprecation( + () => enablePersistentCacheIndexAutoCreation(indexManager!), + () => indexManager!.enableIndexAutoCreation(), + 'enableIndexAutoCreation', + ); + }); + + it('FirestorePersistentCacheIndexManager.disableIndexAutoCreation()', function () { + const firestore = getFirestore(); + const indexManager = firestore.persistentCacheIndexManager(); + persistentCacheIndexManagerV9Deprecation( + () => disablePersistentCacheIndexAutoCreation(indexManager!), + () => indexManager!.disableIndexAutoCreation(), + 'disableIndexAutoCreation', + ); + }); + + it('FirestorePersistentCacheIndexManager.deleteAllIndexes()', function () { + const firestore = getFirestore(); + const indexManager = firestore.persistentCacheIndexManager(); + persistentCacheIndexManagerV9Deprecation( + () => deleteAllPersistentCacheIndexes(indexManager!), + () => indexManager!.deleteAllIndexes(), + 'deleteAllIndexes', + ); + }); }); }); From 9b451d4ae1ad010907906d3e54ac672b5c587495 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Fri, 13 Dec 2024 14:57:24 +0000 Subject: [PATCH 22/29] refactor deprecation warning. working --- packages/app/lib/common/index.js | 30 + packages/app/lib/common/unitTestUtils.ts | 2 +- .../app/lib/internal/registry/namespace.js | 5 +- .../firestore/__tests__/firestore.test.ts | 900 ++++++++++-------- packages/firestore/lib/modular/index.js | 2 +- 5 files changed, 528 insertions(+), 411 deletions(-) diff --git a/packages/app/lib/common/index.js b/packages/app/lib/common/index.js index 264172bb85..1e687b676a 100644 --- a/packages/app/lib/common/index.js +++ b/packages/app/lib/common/index.js @@ -123,6 +123,15 @@ const mapOfDeprecationReplacements = { }, }, firestore: { + statics: { + setLogLevel: 'setLogLevel()', + Filter: 'where()', + FieldValue: 'FieldValue', + Timestamp: 'Timestamp', + GeoPoint: 'GeoPoint', + Blob: 'Bytes', + FieldPath: 'FieldPath', + }, default: { batch: 'writeBatch()', loadBundle: 'loadBundle()', @@ -234,6 +243,10 @@ export function createMessage( } function getNamespace(target) { + if (target.GeoPoint) { + // target is statics. GeoPoint is a static class on Firestore + return 'firestore'; + } if (target._config && target._config.namespace) { return target._config.namespace; } @@ -246,6 +259,10 @@ function getNamespace(target) { } function getInstanceName(target) { + if (target.GeoPoint) { + // target is statics. GeoPoint is a static class on Firestore + return 'statics'; + } if (target._config) { // module class instance, we use default to store map of deprecated methods return 'default'; @@ -267,6 +284,19 @@ export function createDeprecationProxy(instance) { return target.constructor; } + if ( + prop === 'Filter' || + prop === 'FieldValue' || + prop === 'Timestamp' || + prop === 'GeoPoint' || + prop === 'Blob' || + prop === 'FieldPath' + ) { + // Firestore statics + deprecationConsoleWarning('firestore', prop, 'statics', false); + return target[prop]; + } + if (typeof originalMethod === 'function') { return function (...args) { const isModularMethod = args.includes(MODULAR_DEPRECATION_ARG); diff --git a/packages/app/lib/common/unitTestUtils.ts b/packages/app/lib/common/unitTestUtils.ts index 13532b6737..9372074ad4 100644 --- a/packages/app/lib/common/unitTestUtils.ts +++ b/packages/app/lib/common/unitTestUtils.ts @@ -40,7 +40,7 @@ export const createCheckV9Deprecation = (moduleNames: string[]): CheckV9Deprecat consoleWarnSpy.mockRestore(); const consoleWarnSpy2 = jest.spyOn(console, 'warn').mockImplementation(warnMessage => { const message = createMessage(moduleName, methodNameKey, instanceName, uniqueMessage); - expect(message).toMatch(warnMessage); + expect(warnMessage).toMatch(message); }); nonModularFunction(); diff --git a/packages/app/lib/internal/registry/namespace.js b/packages/app/lib/internal/registry/namespace.js index 6665be71c3..361852073c 100644 --- a/packages/app/lib/internal/registry/namespace.js +++ b/packages/app/lib/internal/registry/namespace.js @@ -180,8 +180,9 @@ function getOrCreateModuleForRoot(moduleNamespace) { } Object.assign(firebaseModuleWithApp, statics || {}); - Object.freeze(firebaseModuleWithApp); - MODULE_GETTER_FOR_ROOT[moduleNamespace] = firebaseModuleWithApp; + // Object.freeze(firebaseModuleWithApp); + // Wrap around statics, e.g. firebase.firestore.FieldValue, removed freeze as it stops proxy working. it is deprecated anyway + MODULE_GETTER_FOR_ROOT[moduleNamespace] = createDeprecationProxy(firebaseModuleWithApp); return MODULE_GETTER_FOR_ROOT[moduleNamespace]; } diff --git a/packages/firestore/__tests__/firestore.test.ts b/packages/firestore/__tests__/firestore.test.ts index 4757766397..5e0737ff80 100644 --- a/packages/firestore/__tests__/firestore.test.ts +++ b/packages/firestore/__tests__/firestore.test.ts @@ -7,6 +7,8 @@ import FirebaseModule from '../../app/lib/internal/FirebaseModule'; import FirestoreQuery from '../lib/FirestoreQuery'; // @ts-ignore test import FirestoreDocumentSnapshot from '../lib/FirestoreDocumentSnapshot'; +// @ts-ignore test +import * as nativeModule from '@react-native-firebase/app/lib/internal/nativeModuleAndroidIos'; import { createCheckV9Deprecation, @@ -719,6 +721,7 @@ describe('Firestore', function () { let filterV9Deprecation: CheckV9DeprecationFunction; let persistentCacheIndexManagerV9Deprecation: CheckV9DeprecationFunction; let firestoreRefV9Deprecation: CheckV9DeprecationFunction; + let staticsV9Deprecation: CheckV9DeprecationFunction; beforeEach(function () { firestoreRefV9Deprecation = createCheckV9Deprecation(['firestore']); @@ -736,6 +739,8 @@ describe('Firestore', function () { 'FirestorePersistentCacheIndexManager', ]); + staticsV9Deprecation = createCheckV9Deprecation(['firestore', 'statics']); + // @ts-ignore test jest.spyOn(FirebaseModule.prototype, 'native', 'get').mockImplementation(() => { return new Proxy( @@ -761,414 +766,410 @@ describe('Firestore', function () { }); }); - it('firestore.batch()', function () { - const firestore = getFirestore(); - firestoreRefV9Deprecation( - () => writeBatch(firestore), - () => firestore.batch(), - 'batch', - ); - }); - - it('firestore.loadBundle()', function () { - const firestore = getFirestore(); - firestoreRefV9Deprecation( - () => loadBundle(firestore, 'some bundle'), - () => firestore.loadBundle('some bundle'), - 'loadBundle', - ); - }); - - it('firestore.namedQuery()', function () { - const firestore = getFirestore(); - firestoreRefV9Deprecation( - () => namedQuery(firestore, 'some name'), - () => firestore.namedQuery('some name'), - 'namedQuery', - ); - }); + describe('Firestore', function () { + it('firestore.batch()', function () { + const firestore = getFirestore(); + firestoreRefV9Deprecation( + () => writeBatch(firestore), + () => firestore.batch(), + 'batch', + ); + }); - it('firestore.clearPersistence()', function () { - const firestore = getFirestore(); - firestoreRefV9Deprecation( - () => clearIndexedDbPersistence(firestore), - () => firestore.clearPersistence(), - 'clearPersistence', - ); - // Deprecating the modular method clearPersistence() as it doesn't exist on firebase-js-sdk - firestoreRefV9Deprecation( - () => clearIndexedDbPersistence(firestore), - () => clearPersistence(firestore), - 'clearPersistence', - ); - }); + it('firestore.loadBundle()', function () { + const firestore = getFirestore(); + firestoreRefV9Deprecation( + () => loadBundle(firestore, 'some bundle'), + () => firestore.loadBundle('some bundle'), + 'loadBundle', + ); + }); - it('firestore.waitForPendingWrites()', function () { - const firestore = getFirestore(); - firestoreRefV9Deprecation( - () => waitForPendingWrites(firestore), - () => firestore.waitForPendingWrites(), - 'waitForPendingWrites', - ); - }); + it('firestore.namedQuery()', function () { + const firestore = getFirestore(); + firestoreRefV9Deprecation( + () => namedQuery(firestore, 'some name'), + () => firestore.namedQuery('some name'), + 'namedQuery', + ); + }); - it('firestore.terminate()', function () { - const firestore = getFirestore(); - firestoreRefV9Deprecation( - () => terminate(firestore), - () => firestore.terminate(), - 'terminate', - ); - }); + it('firestore.clearPersistence()', function () { + const firestore = getFirestore(); + firestoreRefV9Deprecation( + () => clearIndexedDbPersistence(firestore), + () => firestore.clearPersistence(), + 'clearPersistence', + ); + // Deprecating the modular method clearPersistence() as it doesn't exist on firebase-js-sdk + firestoreRefV9Deprecation( + () => clearIndexedDbPersistence(firestore), + () => clearPersistence(firestore), + 'clearPersistence', + ); + }); - it('firestore.useEmulator()', function () { - const firestore = getFirestore(); - firestoreRefV9Deprecation( - () => connectFirestoreEmulator(firestore, 'localhost', 8080), - () => firestore.useEmulator('localhost', 8080), - 'useEmulator', - ); - }); + it('firestore.waitForPendingWrites()', function () { + const firestore = getFirestore(); + firestoreRefV9Deprecation( + () => waitForPendingWrites(firestore), + () => firestore.waitForPendingWrites(), + 'waitForPendingWrites', + ); + }); - it('firestore.collection()', function () { - const firestore = getFirestore(); - firestoreRefV9Deprecation( - () => collection(firestore, 'collection'), - () => firestore.collection('collection'), - 'collection', - ); - }); + it('firestore.terminate()', function () { + const firestore = getFirestore(); + firestoreRefV9Deprecation( + () => terminate(firestore), + () => firestore.terminate(), + 'terminate', + ); + }); - it('firestore.collectionGroup()', function () { - const firestore = getFirestore(); - firestoreRefV9Deprecation( - () => collectionGroup(firestore, 'collection'), - () => firestore.collectionGroup('collection'), - 'collectionGroup', - ); - }); + it('firestore.useEmulator()', function () { + const firestore = getFirestore(); + firestoreRefV9Deprecation( + () => connectFirestoreEmulator(firestore, 'localhost', 8080), + () => firestore.useEmulator('localhost', 8080), + 'useEmulator', + ); + }); - it('firestore.disableNetwork()', function () { - const firestore = getFirestore(); - firestoreRefV9Deprecation( - () => disableNetwork(firestore), - () => firestore.disableNetwork(), - 'disableNetwork', - ); - }); + it('firestore.collection()', function () { + const firestore = getFirestore(); + firestoreRefV9Deprecation( + () => collection(firestore, 'collection'), + () => firestore.collection('collection'), + 'collection', + ); + }); - it('firestore.doc()', function () { - const firestore = getFirestore(); - firestoreRefV9Deprecation( - () => doc(firestore, 'collection/path'), - () => firestore.doc('collection/path'), - 'doc', - ); - }); + it('firestore.collectionGroup()', function () { + const firestore = getFirestore(); + firestoreRefV9Deprecation( + () => collectionGroup(firestore, 'collection'), + () => firestore.collectionGroup('collection'), + 'collectionGroup', + ); + }); - it('firestore.enableNetwork()', function () { - const firestore = getFirestore(); - firestoreRefV9Deprecation( - () => enableNetwork(firestore), - () => firestore.enableNetwork(), - 'enableNetwork', - ); - }); + it('firestore.disableNetwork()', function () { + const firestore = getFirestore(); + firestoreRefV9Deprecation( + () => disableNetwork(firestore), + () => firestore.disableNetwork(), + 'disableNetwork', + ); + }); - it('firestore.runTransaction()', function () { - const firestore = getFirestore(); - firestoreRefV9Deprecation( - () => runTransaction(firestore, async () => {}), - () => firestore.runTransaction(async () => {}), - 'runTransaction', - ); - }); + it('firestore.doc()', function () { + const firestore = getFirestore(); + firestoreRefV9Deprecation( + () => doc(firestore, 'collection/path'), + () => firestore.doc('collection/path'), + 'doc', + ); + }); - it('firestore.settings()', function () { - const firestore = getFirestore(); - const app = firebase.app(); - firestoreRefV9Deprecation( - // no equivalent settings method for firebase-js-sdk - () => initializeFirestore(app, {}), - () => firestore.settings({}), - 'settings', - ); - }); + it('firestore.enableNetwork()', function () { + const firestore = getFirestore(); + firestoreRefV9Deprecation( + () => enableNetwork(firestore), + () => firestore.enableNetwork(), + 'enableNetwork', + ); + }); - it('firestore.persistentCacheIndexManager()', function () { - const firestore = getFirestore(); + it('firestore.runTransaction()', function () { + const firestore = getFirestore(); + firestoreRefV9Deprecation( + () => runTransaction(firestore, async () => {}), + () => firestore.runTransaction(async () => {}), + 'runTransaction', + ); + }); - firestoreRefV9Deprecation( - () => getPersistentCacheIndexManager(firestore), - () => firestore.persistentCacheIndexManager(), - 'persistentCacheIndexManager', - ); + it('firestore.settings()', function () { + const firestore = getFirestore(); + const app = firebase.app(); + firestoreRefV9Deprecation( + // no equivalent settings method for firebase-js-sdk + () => initializeFirestore(app, {}), + () => firestore.settings({}), + 'settings', + ); + }); }); - it('CollectionReference.count()', function () { - const firestore = getFirestore(); + describe('CollectionReference', function () { + it('CollectionReference.count()', function () { + const firestore = getFirestore(); - const query = collection(firestore, 'test'); + const query = collection(firestore, 'test'); - collectionRefV9Deprecation( - () => getCountFromServer(query), - () => query.count(), - 'count', - ); - }); + collectionRefV9Deprecation( + () => getCountFromServer(query), + () => query.count(), + 'count', + ); + }); - it('CollectionReference.countFromServer()', function () { - const firestore = getFirestore(); + it('CollectionReference.countFromServer()', function () { + const firestore = getFirestore(); - const query = collection(firestore, 'test'); + const query = collection(firestore, 'test'); - collectionRefV9Deprecation( - () => getCountFromServer(query), - () => query.countFromServer(), - 'count', - ); - }); + collectionRefV9Deprecation( + () => getCountFromServer(query), + () => query.countFromServer(), + 'count', + ); + }); - it('CollectionReference.endAt()', function () { - const firestore = getFirestore(); + it('CollectionReference.endAt()', function () { + const firestore = getFirestore(); - const query = collection(firestore, 'test'); + const query = collection(firestore, 'test'); - collectionRefV9Deprecation( - () => endAt('foo'), - () => query.endAt('foo'), - 'endAt', - ); - }); + collectionRefV9Deprecation( + () => endAt('foo'), + () => query.endAt('foo'), + 'endAt', + ); + }); - it('CollectionReference.endBefore()', function () { - const firestore = getFirestore(); + it('CollectionReference.endBefore()', function () { + const firestore = getFirestore(); - const query = collection(firestore, 'test'); + const query = collection(firestore, 'test'); - collectionRefV9Deprecation( - () => endBefore('foo'), - () => query.endBefore('foo'), - 'endBefore', - ); - }); + collectionRefV9Deprecation( + () => endBefore('foo'), + () => query.endBefore('foo'), + 'endBefore', + ); + }); - it('CollectionReference.get()', function () { - const firestore = getFirestore(); + it('CollectionReference.get()', function () { + const firestore = getFirestore(); - const query = collection(firestore, 'test'); + const query = collection(firestore, 'test'); - collectionRefV9Deprecation( - () => getDocs(query), - () => query.get(), - 'get', - ); - }); + collectionRefV9Deprecation( + () => getDocs(query), + () => query.get(), + 'get', + ); + }); - it('CollectionReference.isEqual()', function () { - const firestore = getFirestore(); + it('CollectionReference.isEqual()', function () { + const firestore = getFirestore(); - const query = collection(firestore, 'test'); + const query = collection(firestore, 'test'); - collectionRefV9Deprecation( - // no equivalent method - () => {}, - () => query.isEqual(query), - 'isEqual', - ); - }); + collectionRefV9Deprecation( + // no equivalent method + () => {}, + () => query.isEqual(query), + 'isEqual', + ); + }); - it('CollectionReference.limit()', function () { - const firestore = getFirestore(); + it('CollectionReference.limit()', function () { + const firestore = getFirestore(); - const query = collection(firestore, 'test'); + const query = collection(firestore, 'test'); - collectionRefV9Deprecation( - () => limit(9), - () => query.limit(9), - 'limit', - ); - }); + collectionRefV9Deprecation( + () => limit(9), + () => query.limit(9), + 'limit', + ); + }); - it('CollectionReference.limitToLast()', function () { - const firestore = getFirestore(); + it('CollectionReference.limitToLast()', function () { + const firestore = getFirestore(); - const query = collection(firestore, 'test'); + const query = collection(firestore, 'test'); - collectionRefV9Deprecation( - () => limitToLast(9), - () => query.limitToLast(9), - 'limitToLast', - ); - }); + collectionRefV9Deprecation( + () => limitToLast(9), + () => query.limitToLast(9), + 'limitToLast', + ); + }); - it('CollectionReference.onSnapshot()', function () { - const firestore = getFirestore(); + it('CollectionReference.onSnapshot()', function () { + const firestore = getFirestore(); - const query = collection(firestore, 'test'); + const query = collection(firestore, 'test'); - collectionRefV9Deprecation( - () => onSnapshot(query, () => {}), - () => query.onSnapshot(() => {}), - 'onSnapshot', - ); - }); + collectionRefV9Deprecation( + () => onSnapshot(query, () => {}), + () => query.onSnapshot(() => {}), + 'onSnapshot', + ); + }); - it('CollectionReference.orderBy()', function () { - const firestore = getFirestore(); + it('CollectionReference.orderBy()', function () { + const firestore = getFirestore(); - const query = collection(firestore, 'test'); + const query = collection(firestore, 'test'); - collectionRefV9Deprecation( - () => orderBy('foo', 'asc'), - () => query.orderBy('foo', 'asc'), - 'orderBy', - ); - }); + collectionRefV9Deprecation( + () => orderBy('foo', 'asc'), + () => query.orderBy('foo', 'asc'), + 'orderBy', + ); + }); - it('CollectionReference.startAfter()', function () { - const firestore = getFirestore(); + it('CollectionReference.startAfter()', function () { + const firestore = getFirestore(); - const query = collection(firestore, 'test'); + const query = collection(firestore, 'test'); - collectionRefV9Deprecation( - () => startAfter('foo'), - () => query.startAfter('foo'), - 'startAfter', - ); - }); + collectionRefV9Deprecation( + () => startAfter('foo'), + () => query.startAfter('foo'), + 'startAfter', + ); + }); - it('CollectionReference.startAt()', function () { - const firestore = getFirestore(); + it('CollectionReference.startAt()', function () { + const firestore = getFirestore(); - const query = collection(firestore, 'test'); + const query = collection(firestore, 'test'); - collectionRefV9Deprecation( - () => startAt('foo'), - () => query.startAt('foo'), - 'startAt', - ); - }); + collectionRefV9Deprecation( + () => startAt('foo'), + () => query.startAt('foo'), + 'startAt', + ); + }); - it('CollectionReference.where()', function () { - const firestore = getFirestore(); + it('CollectionReference.where()', function () { + const firestore = getFirestore(); - const query = collection(firestore, 'test'); + const query = collection(firestore, 'test'); - collectionRefV9Deprecation( - () => where('foo', '==', 'bar'), - () => query.where('foo', '==', 'bar'), - 'where', - ); - }); + collectionRefV9Deprecation( + () => where('foo', '==', 'bar'), + () => query.where('foo', '==', 'bar'), + 'where', + ); + }); - it('CollectionReference.add()', function () { - const firestore = getFirestore(); + it('CollectionReference.add()', function () { + const firestore = getFirestore(); - const query = collection(firestore, 'test'); + const query = collection(firestore, 'test'); - collectionRefV9Deprecation( - () => addDoc(query, { foo: 'bar' }), - () => query.add({ foo: 'bar' }), - 'add', - ); - }); + collectionRefV9Deprecation( + () => addDoc(query, { foo: 'bar' }), + () => query.add({ foo: 'bar' }), + 'add', + ); + }); - it('CollectionReference.doc()', function () { - const firestore = getFirestore(); + it('CollectionReference.doc()', function () { + const firestore = getFirestore(); - const query = collection(firestore, 'test'); + const query = collection(firestore, 'test'); - collectionRefV9Deprecation( - () => doc(query, 'bar'), - () => query.doc('foo'), - 'doc', - ); + collectionRefV9Deprecation( + () => doc(query, 'bar'), + () => query.doc('foo'), + 'doc', + ); + }); }); - it('DocumentReference.collection()', function () { - const firestore = getFirestore(); + describe('DocumentReference', function () { + it('DocumentReference.collection()', function () { + const firestore = getFirestore(); - const docRef = firestore.doc('some/foo'); + const docRef = firestore.doc('some/foo'); - docRefV9Deprecation( - () => collection(firestore, 'bar'), - () => docRef.collection('bar'), - 'collection', - ); - }); + docRefV9Deprecation( + () => collection(firestore, 'bar'), + () => docRef.collection('bar'), + 'collection', + ); + }); - it('DocumentReference.delete()', function () { - const firestore = getFirestore(); + it('DocumentReference.delete()', function () { + const firestore = getFirestore(); - const docRef = firestore.doc('some/foo'); + const docRef = firestore.doc('some/foo'); - docRefV9Deprecation( - () => deleteDoc(docRef), - () => docRef.delete(), - 'delete', - ); - }); + docRefV9Deprecation( + () => deleteDoc(docRef), + () => docRef.delete(), + 'delete', + ); + }); - it('DocumentReference.get()', function () { - const firestore = getFirestore(); + it('DocumentReference.get()', function () { + const firestore = getFirestore(); - const docRef = firestore.doc('some/foo'); + const docRef = firestore.doc('some/foo'); - docRefV9Deprecation( - () => getDoc(docRef), - () => docRef.get(), - 'get', - ); - }); + docRefV9Deprecation( + () => getDoc(docRef), + () => docRef.get(), + 'get', + ); + }); - it('DocumentReference.isEqual()', function () { - const firestore = getFirestore(); + it('DocumentReference.isEqual()', function () { + const firestore = getFirestore(); - const docRef = firestore.doc('some/foo'); + const docRef = firestore.doc('some/foo'); - docRefV9Deprecation( - // no equivalent method - () => {}, - () => docRef.isEqual(docRef), - 'isEqual', - ); - }); + docRefV9Deprecation( + // no equivalent method + () => {}, + () => docRef.isEqual(docRef), + 'isEqual', + ); + }); - it('DocumentReference.onSnapshot()', function () { - const firestore = getFirestore(); + it('DocumentReference.onSnapshot()', function () { + const firestore = getFirestore(); - const docRef = firestore.doc('some/foo'); + const docRef = firestore.doc('some/foo'); - docRefV9Deprecation( - () => onSnapshot(docRef, () => {}), - () => docRef.onSnapshot(() => {}), - 'onSnapshot', - ); - }); + docRefV9Deprecation( + () => onSnapshot(docRef, () => {}), + () => docRef.onSnapshot(() => {}), + 'onSnapshot', + ); + }); - it('DocumentReference.set()', function () { - const firestore = getFirestore(); + it('DocumentReference.set()', function () { + const firestore = getFirestore(); - const docRef = firestore.doc('some/foo'); + const docRef = firestore.doc('some/foo'); - docRefV9Deprecation( - () => setDoc(docRef, { foo: 'bar' }), - () => docRef.set({ foo: 'bar' }), - 'set', - ); - }); + docRefV9Deprecation( + () => setDoc(docRef, { foo: 'bar' }), + () => docRef.set({ foo: 'bar' }), + 'set', + ); + }); - it('DocumentReference.update()', function () { - const firestore = getFirestore(); + it('DocumentReference.update()', function () { + const firestore = getFirestore(); - const docRef = firestore.doc('some/foo'); + const docRef = firestore.doc('some/foo'); - docRefV9Deprecation( - () => updateDoc(docRef, { foo: 'bar' }), - () => docRef.update({ foo: 'bar' }), - 'update', - ); + docRefV9Deprecation( + () => updateDoc(docRef, { foo: 'bar' }), + () => docRef.update({ foo: 'bar' }), + 'update', + ); + }); }); it('FirestoreDocumentSnapshot.isEqual()', function () { @@ -1193,98 +1194,183 @@ describe('Firestore', function () { ); }); - it('FirestoreFieldValue.delete()', function () { - fieldValueV9Deprecation( - () => deleteField(), - () => firestore.FieldValue.delete(), - 'delete', - ); - }); + describe('FieldValue', function () { + it('FieldValue.delete()', function () { + const fieldValue = firestore.FieldValue; + fieldValueV9Deprecation( + () => deleteField(), + () => fieldValue.delete(), + 'delete', + ); + }); - it('FirestoreFieldValue.increment()', function () { - fieldValueV9Deprecation( - () => increment(3), - () => firestore.FieldValue.increment(4), - 'increment', - ); - }); + it('FieldValue.increment()', function () { + const fieldValue = firestore.FieldValue; + fieldValueV9Deprecation( + () => increment(3), + () => fieldValue.increment(4), + 'increment', + ); + }); - it('FirestoreFieldValue.serverTimestamp()', function () { - fieldValueV9Deprecation( - () => serverTimestamp(), - () => firestore.FieldValue.serverTimestamp(), - 'serverTimestamp', - ); - }); + it('FieldValue.serverTimestamp()', function () { + const fieldValue = firestore.FieldValue; + fieldValueV9Deprecation( + () => serverTimestamp(), + () => fieldValue.serverTimestamp(), + 'serverTimestamp', + ); + }); - it('FirestoreFieldValue.arrayUnion()', function () { - fieldValueV9Deprecation( - () => arrayUnion('foo'), - () => firestore.FieldValue.arrayUnion('bar'), - 'arrayUnion', - ); - }); + it('FieldValue.arrayUnion()', function () { + const fieldValue = firestore.FieldValue; + fieldValueV9Deprecation( + () => arrayUnion('foo'), + () => fieldValue.arrayUnion('bar'), + 'arrayUnion', + ); + }); - it('FirestoreFieldValue.arrayRemove()', function () { - fieldValueV9Deprecation( - () => arrayRemove('foo'), - () => firestore.FieldValue.arrayRemove('bar'), - 'arrayRemove', - ); + it('FieldValue.arrayRemove()', function () { + const fieldValue = firestore.FieldValue; + fieldValueV9Deprecation( + () => arrayRemove('foo'), + () => fieldValue.arrayRemove('bar'), + 'arrayRemove', + ); + }); }); - it('Filter.or()', function () { - filterV9Deprecation( - () => or(where('foo.bar', '==', null), where('foo.bar', '==', null)), - () => - firestore.Filter.or( - firestore.Filter('foo', '==', 'bar'), - firestore.Filter('baz', '==', 'qux'), - ), - 'or', - ); - }); + describe('statics', function () { + it('Firestore.setLogLevel()', function () { + // @ts-ignore test + jest + .spyOn(nativeModule, 'getReactNativeModule') + .mockReturnValue({ setLogLevel: jest.fn() }); + + staticsV9Deprecation( + () => setLogLevel('debug'), + () => firestore.setLogLevel('debug'), + 'setLogLevel', + ); + }); - it('Filter.and()', function () { - filterV9Deprecation( - () => and(where('foo.bar', '==', null), where('foo.bar', '==', null)), - () => - firestore.Filter.and( - firestore.Filter('foo', '==', 'bar'), - firestore.Filter('baz', '==', 'qux'), - ), - 'and', - ); - }); + it('Filter static', function () { + staticsV9Deprecation( + // no corresponding method + () => {}, + () => firestore.Filter, + 'Filter', + ); + }); - it('FirestorePersistentCacheIndexManager.enableIndexAutoCreation()', function () { - const firestore = getFirestore(); - const indexManager = firestore.persistentCacheIndexManager(); - persistentCacheIndexManagerV9Deprecation( - () => enablePersistentCacheIndexAutoCreation(indexManager!), - () => indexManager!.enableIndexAutoCreation(), - 'enableIndexAutoCreation', - ); + it('Timestamp static', function () { + staticsV9Deprecation( + () => Timestamp, + () => firestore.Timestamp, + 'Timestamp', + ); + }); + + it('FieldValue static', function () { + staticsV9Deprecation( + () => FieldValue, + () => firestore.FieldValue, + 'FieldValue', + ); + }); + + it('GeoPoint static', function () { + staticsV9Deprecation( + () => GeoPoint, + () => firestore.GeoPoint, + 'GeoPoint', + ); + }); + + it('Blob static', function () { + staticsV9Deprecation( + () => Blob, + () => firestore.Blob, + 'Blob', + ); + }); + + it('FieldPath static', function () { + staticsV9Deprecation( + () => FieldPath, + () => firestore.FieldPath, + 'FieldPath', + ); + }); }); - it('FirestorePersistentCacheIndexManager.disableIndexAutoCreation()', function () { - const firestore = getFirestore(); - const indexManager = firestore.persistentCacheIndexManager(); - persistentCacheIndexManagerV9Deprecation( - () => disablePersistentCacheIndexAutoCreation(indexManager!), - () => indexManager!.disableIndexAutoCreation(), - 'disableIndexAutoCreation', - ); + describe('Filter', function () { + it('Filter.or()', function () { + const filter = firestore.Filter; + filterV9Deprecation( + () => or(where('foo.bar', '==', null), where('foo.bar', '==', null)), + () => filter.or(filter('foo', '==', 'bar'), filter('baz', '==', 'qux')), + 'or', + ); + }); + + it('Filter.and()', function () { + const filter = firestore.Filter; + filterV9Deprecation( + () => and(where('foo.bar', '==', null), where('foo.bar', '==', null)), + () => filter.and(filter('foo', '==', 'bar'), filter('baz', '==', 'qux')), + 'and', + ); + }); }); - it('FirestorePersistentCacheIndexManager.deleteAllIndexes()', function () { - const firestore = getFirestore(); - const indexManager = firestore.persistentCacheIndexManager(); - persistentCacheIndexManagerV9Deprecation( - () => deleteAllPersistentCacheIndexes(indexManager!), - () => indexManager!.deleteAllIndexes(), - 'deleteAllIndexes', - ); + describe('FirestorePersistentCacheIndexManager', function () { + it('firestore.persistentCacheIndexManager()', function () { + const firestore = getFirestore(); + + firestoreRefV9Deprecation( + () => getPersistentCacheIndexManager(firestore), + () => firestore.persistentCacheIndexManager(), + 'persistentCacheIndexManager', + ); + }); + + it('FirestorePersistentCacheIndexManager.enableIndexAutoCreation()', function () { + const firestore = getFirestore(); + // @ts-ignore test + firestore._settings.persistence = true; + const indexManager = firestore.persistentCacheIndexManager(); + persistentCacheIndexManagerV9Deprecation( + () => enablePersistentCacheIndexAutoCreation(indexManager!), + () => indexManager!.enableIndexAutoCreation(), + 'enableIndexAutoCreation', + ); + }); + + it('FirestorePersistentCacheIndexManager.disableIndexAutoCreation()', function () { + const firestore = getFirestore(); + // @ts-ignore test + firestore._settings.persistence = true; + const indexManager = firestore.persistentCacheIndexManager(); + persistentCacheIndexManagerV9Deprecation( + () => disablePersistentCacheIndexAutoCreation(indexManager!), + () => indexManager!.disableIndexAutoCreation(), + 'disableIndexAutoCreation', + ); + }); + + it('FirestorePersistentCacheIndexManager.deleteAllIndexes()', function () { + const firestore = getFirestore(); + // @ts-ignore test + firestore._settings.persistence = true; + const indexManager = firestore.persistentCacheIndexManager(); + persistentCacheIndexManagerV9Deprecation( + () => deleteAllPersistentCacheIndexes(indexManager!), + () => indexManager!.deleteAllIndexes(), + 'deleteAllIndexes', + ); + }); }); }); }); diff --git a/packages/firestore/lib/modular/index.js b/packages/firestore/lib/modular/index.js index 8685bcf186..e291587e48 100644 --- a/packages/firestore/lib/modular/index.js +++ b/packages/firestore/lib/modular/index.js @@ -199,7 +199,7 @@ export function connectFirestoreEmulator(firestore, host, port, options) { * @returns {void} */ export function setLogLevel(logLevel) { - return firebase.firestore.setLogLevel(logLevel); + return firebase.firestore.setLogLevel.call(null, logLevel, MODULAR_DEPRECATION_ARG); } /** From 7dfec742cfd647e4d57f403ef657588fa8b5bc8c Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Tue, 17 Dec 2024 15:17:04 +0000 Subject: [PATCH 23/29] Timestamp proxy --- packages/app/lib/common/index.js | 26 ++++++++++++++----- .../firestore/__tests__/firestore.test.ts | 25 ++++++++++++++++++ packages/firestore/lib/FirestoreStatics.js | 2 +- 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/packages/app/lib/common/index.js b/packages/app/lib/common/index.js index 1e687b676a..c336bf3777 100644 --- a/packages/app/lib/common/index.js +++ b/packages/app/lib/common/index.js @@ -194,6 +194,10 @@ const mapOfDeprecationReplacements = { disableIndexAutoCreation: 'disablePersistentCacheIndexAutoCreation()', deleteAllIndexes: 'deleteAllPersistentCacheIndexes()', }, + FirestoreTimestamp: { + seconds: NO_REPLACEMENT, + nanoseconds: NO_REPLACEMENT, + }, }, }; @@ -277,6 +281,10 @@ function getInstanceName(target) { export function createDeprecationProxy(instance) { return new Proxy(instance, { + construct(target, args) { + // needed for Timestamp which we pass as static, when we construct new instance, we need to wrap it in proxy again. + return createDeprecationProxy(new target(...args)); + }, get(target, prop, receiver) { const originalMethod = target[prop]; @@ -284,13 +292,19 @@ export function createDeprecationProxy(instance) { return target.constructor; } + if (target && target.constructor && target.constructor.name === 'FirestoreTimestamp') { + deprecationConsoleWarning('firestore', prop, 'FirestoreTimestamp', false); + } + if ( - prop === 'Filter' || - prop === 'FieldValue' || - prop === 'Timestamp' || - prop === 'GeoPoint' || - prop === 'Blob' || - prop === 'FieldPath' + target && + target.name === 'firebaseModuleWithApp' && + (prop === 'Filter' || + prop === 'FieldValue' || + prop === 'Timestamp' || + prop === 'GeoPoint' || + prop === 'Blob' || + prop === 'FieldPath') ) { // Firestore statics deprecationConsoleWarning('firestore', prop, 'statics', false); diff --git a/packages/firestore/__tests__/firestore.test.ts b/packages/firestore/__tests__/firestore.test.ts index 5e0737ff80..145a7307e1 100644 --- a/packages/firestore/__tests__/firestore.test.ts +++ b/packages/firestore/__tests__/firestore.test.ts @@ -722,6 +722,7 @@ describe('Firestore', function () { let persistentCacheIndexManagerV9Deprecation: CheckV9DeprecationFunction; let firestoreRefV9Deprecation: CheckV9DeprecationFunction; let staticsV9Deprecation: CheckV9DeprecationFunction; + let timestampV9Deprecation: CheckV9DeprecationFunction; beforeEach(function () { firestoreRefV9Deprecation = createCheckV9Deprecation(['firestore']); @@ -741,6 +742,8 @@ describe('Firestore', function () { staticsV9Deprecation = createCheckV9Deprecation(['firestore', 'statics']); + timestampV9Deprecation = createCheckV9Deprecation(['firestore', 'FirestoreTimestamp']); + // @ts-ignore test jest.spyOn(FirebaseModule.prototype, 'native', 'get').mockImplementation(() => { return new Proxy( @@ -1372,5 +1375,27 @@ describe('Firestore', function () { ); }); }); + + describe('Timestamp', function () { + it('Timestamp.seconds', function () { + const timestamp = new firestore.Timestamp(2, 3); + timestampV9Deprecation( + // no corresponding method + () => {}, + () => timestamp.seconds, + 'seconds', + ); + }); + + it('Timestamp.nanoseconds', function () { + const timestamp = new firestore.Timestamp(2000, 3000000); + timestampV9Deprecation( + // no corresponding method + () => {}, + () => timestamp.nanoseconds, + 'nanoseconds', + ); + }); + }); }); }); diff --git a/packages/firestore/lib/FirestoreStatics.js b/packages/firestore/lib/FirestoreStatics.js index bac6b402d3..19f921160b 100644 --- a/packages/firestore/lib/FirestoreStatics.js +++ b/packages/firestore/lib/FirestoreStatics.js @@ -28,7 +28,7 @@ export default { FieldPath: FirestoreFieldPath, FieldValue: createDeprecationProxy(FirestoreFieldValue), GeoPoint: FirestoreGeoPoint, - Timestamp: FirestoreTimestamp, + Timestamp: createDeprecationProxy(FirestoreTimestamp), Filter: createDeprecationProxy(Filter), CACHE_SIZE_UNLIMITED: -1, From 071420c5ca055f4924a399529dfafe1ab83cb8f8 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Tue, 17 Dec 2024 16:25:59 +0000 Subject: [PATCH 24/29] chore: tidy up --- packages/app/lib/common/index.js | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/packages/app/lib/common/index.js b/packages/app/lib/common/index.js index c336bf3777..5b925ddb5d 100644 --- a/packages/app/lib/common/index.js +++ b/packages/app/lib/common/index.js @@ -123,15 +123,6 @@ const mapOfDeprecationReplacements = { }, }, firestore: { - statics: { - setLogLevel: 'setLogLevel()', - Filter: 'where()', - FieldValue: 'FieldValue', - Timestamp: 'Timestamp', - GeoPoint: 'GeoPoint', - Blob: 'Bytes', - FieldPath: 'FieldPath', - }, default: { batch: 'writeBatch()', loadBundle: 'loadBundle()', @@ -149,6 +140,15 @@ const mapOfDeprecationReplacements = { settings: 'settings()', persistentCacheIndexManager: 'getPersistentCacheIndexManager()', }, + statics: { + setLogLevel: 'setLogLevel()', + Filter: 'where()', + FieldValue: 'FieldValue', + Timestamp: 'Timestamp', + GeoPoint: 'GeoPoint', + Blob: 'Bytes', + FieldPath: 'FieldPath', + }, FirestoreCollectionReference: { count: 'getCountFromServer()', countFromServer: 'getCountFromServer()', @@ -248,7 +248,7 @@ export function createMessage( function getNamespace(target) { if (target.GeoPoint) { - // target is statics. GeoPoint is a static class on Firestore + // target is statics object. GeoPoint is a static class on Firestore return 'firestore'; } if (target._config && target._config.namespace) { @@ -264,7 +264,7 @@ function getNamespace(target) { function getInstanceName(target) { if (target.GeoPoint) { - // target is statics. GeoPoint is a static class on Firestore + // target is statics object. GeoPoint is a static class on Firestore return 'statics'; } if (target._config) { @@ -319,11 +319,7 @@ export function createDeprecationProxy(instance) { deprecationConsoleWarning(nameSpace, prop, instanceName, isModularMethod); - return originalMethod.apply( - target, - // Remove the modular deprecation argument from the arguments list - args.filter(arg => arg !== MODULAR_DEPRECATION_ARG), - ); + return originalMethod.apply(target, filterModularArgument(args)); }; } return Reflect.get(target, prop, receiver); From 02433f33ee5f1865c0516ee713c89c982df821a4 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Tue, 17 Dec 2024 16:42:39 +0000 Subject: [PATCH 25/29] chore: remove unused args --- packages/firestore/__tests__/firestore.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/__tests__/firestore.test.ts b/packages/firestore/__tests__/firestore.test.ts index 145a7307e1..b7005478f1 100644 --- a/packages/firestore/__tests__/firestore.test.ts +++ b/packages/firestore/__tests__/firestore.test.ts @@ -764,7 +764,7 @@ describe('Firestore', function () { jest .spyOn(FirestoreQuery.prototype, '_handleQueryCursor') // @ts-ignore test - .mockImplementation((cursor, docOrField, fields) => { + .mockImplementation(() => { return []; }); }); From f25329011031784d95924fd282dc7fded4e234de Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Wed, 18 Dec 2024 12:06:42 +0000 Subject: [PATCH 26/29] chore: update statics logic --- packages/app/lib/common/index.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/app/lib/common/index.js b/packages/app/lib/common/index.js index 5b925ddb5d..b1f4eebe73 100644 --- a/packages/app/lib/common/index.js +++ b/packages/app/lib/common/index.js @@ -296,18 +296,18 @@ export function createDeprecationProxy(instance) { deprecationConsoleWarning('firestore', prop, 'FirestoreTimestamp', false); } - if ( - target && - target.name === 'firebaseModuleWithApp' && - (prop === 'Filter' || + if (target && target.name === 'firebaseModuleWithApp') { + // statics + if ( + prop === 'Filter' || prop === 'FieldValue' || prop === 'Timestamp' || prop === 'GeoPoint' || prop === 'Blob' || - prop === 'FieldPath') - ) { - // Firestore statics - deprecationConsoleWarning('firestore', prop, 'statics', false); + prop === 'FieldPath' + ) { + deprecationConsoleWarning('firestore', prop, 'statics', false); + } return target[prop]; } From 61e782c6b7d665f88c93f7186affa8323ab910dc Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Wed, 18 Dec 2024 12:57:56 +0000 Subject: [PATCH 27/29] fix: proxy behavior --- packages/app/lib/common/index.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/app/lib/common/index.js b/packages/app/lib/common/index.js index b1f4eebe73..159ae0c40f 100644 --- a/packages/app/lib/common/index.js +++ b/packages/app/lib/common/index.js @@ -289,11 +289,12 @@ export function createDeprecationProxy(instance) { const originalMethod = target[prop]; if (prop === 'constructor') { - return target.constructor; + return Reflect.get(target, prop, receiver); } if (target && target.constructor && target.constructor.name === 'FirestoreTimestamp') { deprecationConsoleWarning('firestore', prop, 'FirestoreTimestamp', false); + return Reflect.get(target, prop, receiver); } if (target && target.name === 'firebaseModuleWithApp') { @@ -308,7 +309,10 @@ export function createDeprecationProxy(instance) { ) { deprecationConsoleWarning('firestore', prop, 'statics', false); } - return target[prop]; + if (prop !== 'setLogLevel') { + // we want to capture setLogLevel function call which we do below + return Reflect.get(target, prop, receiver); + } } if (typeof originalMethod === 'function') { From e3064a35cf19be8f43cecee4eefd5f98c1c443bc Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Wed, 18 Dec 2024 13:54:32 +0000 Subject: [PATCH 28/29] fix: broken e2e tests --- packages/firestore/e2e/WriteBatch/commit.e2e.js | 10 +++++----- packages/firestore/lib/FirestoreDocumentReference.js | 6 ++++-- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/firestore/e2e/WriteBatch/commit.e2e.js b/packages/firestore/e2e/WriteBatch/commit.e2e.js index 43cc9d3278..67566e3ed5 100644 --- a/packages/firestore/e2e/WriteBatch/commit.e2e.js +++ b/packages/firestore/e2e/WriteBatch/commit.e2e.js @@ -254,17 +254,17 @@ describe('firestore.WriteBatch.commit()', function () { }); it('should set & commit', async function () { - const { getFirestore, writeBatch, doc, setDoc, getDoc, deleteDoc } = firestoreModular; + const { getFirestore, writeBatch, doc, getDoc, deleteDoc } = firestoreModular; const db = getFirestore(); const lRef = doc(db, `${COLLECTION}/LON`); const nycRef = doc(db, `${COLLECTION}/NYC`); const sfRef = doc(db, `${COLLECTION}/SF`); const batch = writeBatch(db); - - setDoc(batch, lRef, { name: 'London' }); - setDoc(batch, nycRef, { name: 'New York' }); - setDoc(batch, sfRef, { name: 'San Francisco' }); + // This is the correct way of setting batch for modular API + batch.set(lRef, { name: 'London' }); + batch.set(nycRef, { name: 'New York' }); + batch.set(sfRef, { name: 'San Francisco' }); await batch.commit(); diff --git a/packages/firestore/lib/FirestoreDocumentReference.js b/packages/firestore/lib/FirestoreDocumentReference.js index 141eb63e70..094239acb9 100644 --- a/packages/firestore/lib/FirestoreDocumentReference.js +++ b/packages/firestore/lib/FirestoreDocumentReference.js @@ -20,6 +20,7 @@ import { isString, isUndefined, createDeprecationProxy, + filterModularArgument, } from '@react-native-firebase/app/lib/common'; import NativeError from '@react-native-firebase/app/lib/internal/NativeFirebaseError'; import { parseSetOptions, parseSnapshotArgs, parseUpdateArgs } from './utils'; @@ -197,7 +198,8 @@ export default class FirestoreDocumentReference { } update(...args) { - if (args.length === 0) { + const updatedArgs = filterModularArgument(args); + if (updatedArgs.length === 0) { throw new Error( 'firebase.firestore().doc().update(*) expected at least 1 argument but was called with 0 arguments.', ); @@ -205,7 +207,7 @@ export default class FirestoreDocumentReference { let data; try { - data = parseUpdateArgs(args); + data = parseUpdateArgs(updatedArgs); } catch (e) { throw new Error(`firebase.firestore().doc().update(*) ${e.message}`); } From c79ff515c9ae941ed5a65f28d58a354caa55718a Mon Sep 17 00:00:00 2001 From: Russell Wheatley Date: Fri, 20 Dec 2024 09:07:17 +0000 Subject: [PATCH 29/29] Apply suggestions from code review Co-authored-by: Mike Hardy --- packages/app/lib/FirebaseApp.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/app/lib/FirebaseApp.js b/packages/app/lib/FirebaseApp.js index 449829a863..7ff26f28c6 100644 --- a/packages/app/lib/FirebaseApp.js +++ b/packages/app/lib/FirebaseApp.js @@ -61,7 +61,6 @@ export default class FirebaseApp { } extendApp(extendedProps) { - // this method has no modular alternative, send true for param 'noAlternative' warnIfNotModularCall(arguments); this._checkDestroyed(); Object.assign(this, extendedProps); @@ -74,7 +73,6 @@ export default class FirebaseApp { } toString() { - // this method has no modular alternative, send true for param 'noAlternative' warnIfNotModularCall(arguments); return this.name; }