From 151a8ef0c04c3a84e6a79e326b6435d9aaacef71 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Fri, 22 Nov 2024 10:58:25 +0000 Subject: [PATCH] refactor: the way we deprecate methods --- packages/app/lib/common/index.js | 60 +++++++++++++++++++ packages/app/lib/common/unitTestUtils.ts | 40 ++++++++++++- .../app/lib/internal/registry/namespace.js | 27 +++++++-- .../crashlytics/__tests__/crashlytics.test.ts | 28 +++++++-- packages/crashlytics/lib/index.js | 12 ---- packages/crashlytics/lib/modular/index.js | 12 ++-- 6 files changed, 150 insertions(+), 29 deletions(-) diff --git a/packages/app/lib/common/index.js b/packages/app/lib/common/index.js index 68dcc755ed..f3d95fcbea 100644 --- a/packages/app/lib/common/index.js +++ b/packages/app/lib/common/index.js @@ -103,6 +103,66 @@ export function tryJSONStringify(data) { } } +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()', + }, +}; + +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) { + if (!isModularMethod) { + const moduleMap = mapOfDeprecationReplacements[moduleName]; + if (moduleMap) { + const replacementMethodName = moduleMap[methodName]; + // only warn if it is mapped and purposefully deprecated + if (replacementMethodName) { + let message; + if (replacementMethodName !== NO_REPLACEMENT) { + message = v8deprecationMessage + ` Please use \`${replacementMethodName}\` instead.`; + } + // eslint-disable-next-line no-console + console.warn(message); + } + } + } +} + +export function createConsoleWarningMessageTest(moduleName, methodName, uniqueMessage = '') { + if (uniqueMessage.length > 0) { + // Unique deprecation message + return uniqueMessage; + } + // use this to generate message for unit tests + const moduleMap = mapOfDeprecationReplacements[moduleName]; + if (moduleMap) { + const replacementMethodName = moduleMap[methodName]; + // only warn if it is mapped and purposefully deprecated + if (replacementMethodName) { + let message; + if (replacementMethodName !== NO_REPLACEMENT) { + message = v8deprecationMessage + ` Please use \`${replacementMethodName}\` instead.`; + } + return message; + } + } +} + export const MODULAR_DEPRECATION_ARG = 'react-native-firebase-modular-method-call'; export function warnIfNotModularCall(args, replacementMethodName, noAlternative) { diff --git a/packages/app/lib/common/unitTestUtils.ts b/packages/app/lib/common/unitTestUtils.ts index ce19cba3f6..f1aabecd9d 100644 --- a/packages/app/lib/common/unitTestUtils.ts +++ b/packages/app/lib/common/unitTestUtils.ts @@ -1,10 +1,46 @@ +// @ts-nocheck import { expect, jest } from '@jest/globals'; +import { createConsoleWarningMessageTest } from './index'; export const checkV9Deprecation = (modularFunction: () => void, nonModularFunction: () => void) => { const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + consoleWarnSpy.mockRestore(); modularFunction(); expect(consoleWarnSpy).not.toHaveBeenCalled(); + consoleWarnSpy.mockClear(); + const consoleWarnSpy2 = jest.spyOn(console, 'warn').mockImplementation(() => {}); nonModularFunction(); - expect(consoleWarnSpy).toHaveBeenCalledTimes(1); - consoleWarnSpy.mockRestore(); + + expect(consoleWarnSpy2).toHaveBeenCalledTimes(1); + consoleWarnSpy2.mockClear(); +}; + +export type CheckV9DeprecationFunction = ( + modularFunction: () => void, + nonModularFunction: () => void, + methodName: string, + uniqueMessage: string = '', +) => void; + +export const createCheckV9Deprecation = (moduleName: string): CheckV9DeprecationFunction => { + return ( + modularFunction: () => void, + nonModularFunction: () => void, + methodName: string, + uniqueMessage = '', + ) => { + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + consoleWarnSpy.mockRestore(); + modularFunction(); + expect(consoleWarnSpy).not.toHaveBeenCalled(); + consoleWarnSpy.mockReset(); + const consoleWarnSpy2 = jest.spyOn(console, 'warn').mockImplementation(warnMessage => { + const message = createConsoleWarningMessageTest(moduleName, methodName, uniqueMessage); + expect(message).toMatch(warnMessage); + }); + nonModularFunction(); + + expect(consoleWarnSpy2).toHaveBeenCalledTimes(1); + consoleWarnSpy2.mockReset(); + }; }; diff --git a/packages/app/lib/internal/registry/namespace.js b/packages/app/lib/internal/registry/namespace.js index d8a41903e6..55239e8361 100644 --- a/packages/app/lib/internal/registry/namespace.js +++ b/packages/app/lib/internal/registry/namespace.js @@ -15,7 +15,7 @@ * */ -import { isString } from '../../common'; +import { isString, MODULAR_DEPRECATION_ARG, deprecationConsoleWarning } from '../../common'; import FirebaseApp from '../../FirebaseApp'; import SDK_VERSION from '../../version'; import { DEFAULT_APP_NAME, KNOWN_NAMESPACES } from '../constants'; @@ -170,10 +170,10 @@ function getOrCreateModuleForRoot(moduleNamespace) { } if (!APP_MODULE_INSTANCE[_app.name][moduleNamespace]) { - APP_MODULE_INSTANCE[_app.name][moduleNamespace] = new ModuleClass( - _app, - NAMESPACE_REGISTRY[moduleNamespace], + const module = createDeprecationProxy( + new ModuleClass(_app, NAMESPACE_REGISTRY[moduleNamespace]), ); + APP_MODULE_INSTANCE[_app.name][moduleNamespace] = module; } return APP_MODULE_INSTANCE[_app.name][moduleNamespace]; @@ -277,6 +277,25 @@ export function getFirebaseRoot() { return createFirebaseRoot(); } +function createDeprecationProxy(instance) { + return new Proxy(instance, { + get(target, prop, receiver) { + const originalMethod = target[prop]; + 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 634b9228a4..8ba0825744 100644 --- a/packages/crashlytics/__tests__/crashlytics.test.ts +++ b/packages/crashlytics/__tests__/crashlytics.test.ts @@ -1,7 +1,10 @@ import { describe, expect, it, jest, beforeEach } from '@jest/globals'; // @ts-ignore test import FirebaseModule from '../../app/lib/internal/FirebaseModule'; -import { checkV9Deprecation } from '../../app/lib/common/unitTestUtils'; +import { + createCheckV9Deprecation, + CheckV9DeprecationFunction, +} from '../../app/lib/common/unitTestUtils'; import { firebase, getCrashlytics, @@ -83,7 +86,11 @@ describe('Crashlytics', function () { }); describe('test `console.warn` is called for RNFB v8 API & not called for v9 API', function () { - beforeEach(() => { + let checkV9Deprecation: CheckV9DeprecationFunction; + + beforeEach(function () { + checkV9Deprecation = createCheckV9Deprecation('crashlytics'); + // @ts-ignore test jest.spyOn(FirebaseModule.prototype, 'native', 'get').mockImplementation(() => { return new Proxy( @@ -97,19 +104,19 @@ describe('Crashlytics', function () { it('checkForUnsentReports', function () { const crashlytics = getCrashlytics(); - checkV9Deprecation( - () => checkForUnsentReports(crashlytics), + () => {}, () => crashlytics.checkForUnsentReports(), + 'checkForUnsentReports', ); }); it('crash', function () { const crashlytics = getCrashlytics(); - checkV9Deprecation( () => crash(crashlytics), () => crashlytics.crash(), + 'crash', ); }); @@ -118,6 +125,7 @@ describe('Crashlytics', function () { checkV9Deprecation( () => deleteUnsentReports(crashlytics), () => crashlytics.deleteUnsentReports(), + 'deleteUnsentReports', ); }); @@ -126,6 +134,7 @@ describe('Crashlytics', function () { checkV9Deprecation( () => didCrashOnPreviousExecution(crashlytics), () => crashlytics.didCrashOnPreviousExecution(), + 'didCrashOnPreviousExecution', ); }); @@ -134,6 +143,7 @@ describe('Crashlytics', function () { checkV9Deprecation( () => log(crashlytics, 'message'), () => crashlytics.log('message'), + 'log', ); }); @@ -142,6 +152,7 @@ describe('Crashlytics', function () { checkV9Deprecation( () => setAttribute(crashlytics, 'name', 'value'), () => crashlytics.setAttribute('name', 'value'), + 'setAttribute', ); }); @@ -150,6 +161,7 @@ describe('Crashlytics', function () { checkV9Deprecation( () => setAttributes(crashlytics, {}), () => crashlytics.setAttributes({}), + 'setAttributes', ); }); @@ -158,6 +170,7 @@ describe('Crashlytics', function () { checkV9Deprecation( () => setUserId(crashlytics, 'id'), () => crashlytics.setUserId('id'), + 'setUserId', ); }); @@ -166,6 +179,7 @@ describe('Crashlytics', function () { checkV9Deprecation( () => recordError(crashlytics, new Error(), 'name'), () => crashlytics.recordError(new Error(), 'name'), + 'recordError', ); }); @@ -174,6 +188,7 @@ describe('Crashlytics', function () { checkV9Deprecation( () => sendUnsentReports(crashlytics), () => crashlytics.sendUnsentReports(), + 'sendUnsentReports', ); }); @@ -182,6 +197,7 @@ describe('Crashlytics', function () { checkV9Deprecation( () => setCrashlyticsCollectionEnabled(crashlytics, true), () => crashlytics.setCrashlyticsCollectionEnabled(true), + 'setCrashlyticsCollectionEnabled', ); }); @@ -191,6 +207,8 @@ describe('Crashlytics', function () { // swapped order here because we're deprecating the modular method and keeping the property on Crashlytics instance () => crashlytics.isCrashlyticsCollectionEnabled, () => isCrashlyticsCollectionEnabled(crashlytics), + '', + '`isCrashlyticsCollectionEnabled()` is deprecated, please use `Crashlytics.isCrashlyticsCollectionEnabled` property instead', ); }); }); diff --git a/packages/crashlytics/lib/index.js b/packages/crashlytics/lib/index.js index 3b48e80d39..51b351dcdc 100644 --- a/packages/crashlytics/lib/index.js +++ b/packages/crashlytics/lib/index.js @@ -22,7 +22,6 @@ import { isObject, isString, isOther, - warnIfNotModularCall, } from '@react-native-firebase/app/lib/common'; import { createModuleNamespace, @@ -57,7 +56,6 @@ class FirebaseCrashlyticsModule extends FirebaseModule { } checkForUnsentReports() { - warnIfNotModularCall(arguments, 'checkForUnsentReports()'); if (this.isCrashlyticsCollectionEnabled) { throw new Error( "firebase.crashlytics().setCrashlyticsCollectionEnabled(*) has been set to 'true', all reports are automatically sent.", @@ -67,27 +65,22 @@ class FirebaseCrashlyticsModule extends FirebaseModule { } crash() { - warnIfNotModularCall(arguments, 'crash()'); this.native.crash(); } async deleteUnsentReports() { - warnIfNotModularCall(arguments, 'deleteUnsentReports()'); await this.native.deleteUnsentReports(); } didCrashOnPreviousExecution() { - warnIfNotModularCall(arguments, 'didCrashOnPreviousExecution()'); return this.native.didCrashOnPreviousExecution(); } log(message) { - warnIfNotModularCall(arguments, 'log()'); this.native.log(`${message}`); } setAttribute(name, value) { - warnIfNotModularCall(arguments, 'setAttribute()'); if (!isString(name)) { throw new Error( 'firebase.crashlytics().setAttribute(*, _): The supplied property name must be a string.', @@ -104,7 +97,6 @@ class FirebaseCrashlyticsModule extends FirebaseModule { } setAttributes(object) { - warnIfNotModularCall(arguments, 'setAttributes()'); if (!isObject(object)) { throw new Error( 'firebase.crashlytics().setAttributes(*): The supplied arg must be an object of key value strings.', @@ -115,7 +107,6 @@ class FirebaseCrashlyticsModule extends FirebaseModule { } setUserId(userId) { - warnIfNotModularCall(arguments, 'setUserId()'); if (!isString(userId)) { throw new Error( 'firebase.crashlytics().setUserId(*): The supplied userId must be a string value.', @@ -126,7 +117,6 @@ class FirebaseCrashlyticsModule extends FirebaseModule { } recordError(error, jsErrorName) { - warnIfNotModularCall(arguments, 'recordError()'); if (isError(error)) { StackTrace.fromError(error, { offline: true }).then(stackFrames => { this.native.recordError(createNativeErrorObj(error, stackFrames, false, jsErrorName)); @@ -139,14 +129,12 @@ class FirebaseCrashlyticsModule extends FirebaseModule { } sendUnsentReports() { - warnIfNotModularCall(arguments, 'sendUnsentReports()'); if (this.isCrashlyticsCollectionEnabled) { this.native.sendUnsentReports(); } } setCrashlyticsCollectionEnabled(enabled) { - warnIfNotModularCall(arguments, 'setCrashlyticsCollectionEnabled()'); if (!isBoolean(enabled)) { throw new Error( "firebase.crashlytics().setCrashlyticsCollectionEnabled(*) 'enabled' must be a boolean.", diff --git a/packages/crashlytics/lib/modular/index.js b/packages/crashlytics/lib/modular/index.js index 29e8a4a30e..1a4dcf8de6 100644 --- a/packages/crashlytics/lib/modular/index.js +++ b/packages/crashlytics/lib/modular/index.js @@ -1,7 +1,4 @@ -import { - MODULAR_DEPRECATION_ARG, - warnIfNotModularCall, -} from '@react-native-firebase/app/lib/common'; +import { MODULAR_DEPRECATION_ARG } from '@react-native-firebase/app/lib/common'; import { firebase } from '..'; /** @@ -35,8 +32,11 @@ export function getCrashlytics() { * @returns {boolean} */ export function isCrashlyticsCollectionEnabled(crashlytics) { - // Deprecating this method and allow it as a property on Crashlytics instance. - warnIfNotModularCall([], 'crashlytics.isCrashlyticsCollectionEnabled'); + // Unique. Deprecating modular method and allow it as a property on Crashlytics instance. + // eslint-disable-next-line no-console + console.warn( + '`isCrashlyticsCollectionEnabled()` is deprecated, please use `Crashlytics.isCrashlyticsCollectionEnabled` property instead', + ); return crashlytics.isCrashlyticsCollectionEnabled; }