From f241a27360cee5da7608c57cb49624404349e289 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Thu, 31 Oct 2024 17:52:42 +0000 Subject: [PATCH 1/8] chore(crashlytics): deprecation warning for v8 API ahead of future major release --- packages/app/lib/common/index.js | 11 ++++ packages/crashlytics/lib/index.js | 68 +++++++++++++++++++++++ packages/crashlytics/lib/modular/index.js | 33 +++++++---- 3 files changed, 101 insertions(+), 11 deletions(-) diff --git a/packages/app/lib/common/index.js b/packages/app/lib/common/index.js index 8eba4403c2..2b01c9a25f 100644 --- a/packages/app/lib/common/index.js +++ b/packages/app/lib/common/index.js @@ -102,3 +102,14 @@ export function tryJSONStringify(data) { return null; } } + +export const MODULAR_DEPRECATION_ARG = 'react-native-firebase-modular-method-call'; + +export function isNotModularCall(args) { + for (let i = 0; i < args.length; i++) { + if (args[i] === MODULAR_DEPRECATION_ARG) { + return false; + } + } + return true; +} diff --git a/packages/crashlytics/lib/index.js b/packages/crashlytics/lib/index.js index 2f37ed8dd8..e58ecb9eff 100644 --- a/packages/crashlytics/lib/index.js +++ b/packages/crashlytics/lib/index.js @@ -22,6 +22,7 @@ import { isObject, isString, isOther, + isNotModularCall, } from '@react-native-firebase/app/lib/common'; import { createModuleNamespace, @@ -51,10 +52,17 @@ class FirebaseCrashlyticsModule extends FirebaseModule { } get isCrashlyticsCollectionEnabled() { + // Purposefully did not deprecate this as I think it should remain a property rather than a method. return this._isCrashlyticsCollectionEnabled; } checkForUnsentReports() { + if (isNotModularCall(arguments)) { + // eslint-disable-next-line no-console + console.warn( + '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. Please use `checkForUnsentReports()` instead.', + ); + } if (this.isCrashlyticsCollectionEnabled) { throw new Error( "firebase.crashlytics().setCrashlyticsCollectionEnabled(*) has been set to 'true', all reports are automatically sent.", @@ -64,22 +72,52 @@ class FirebaseCrashlyticsModule extends FirebaseModule { } crash() { + if (isNotModularCall(arguments)) { + // eslint-disable-next-line no-console + console.warn( + '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. Please use `crash()` instead.', + ); + } this.native.crash(); } async deleteUnsentReports() { + if (isNotModularCall(arguments)) { + // eslint-disable-next-line no-console + console.warn( + '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. Please use `deleteUnsentReports()` instead.', + ); + } await this.native.deleteUnsentReports(); } didCrashOnPreviousExecution() { + if (isNotModularCall(arguments)) { + // eslint-disable-next-line no-console + console.warn( + '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. Please use `didCrashOnPreviousExecution()` instead.', + ); + } return this.native.didCrashOnPreviousExecution(); } log(message) { + if (isNotModularCall(arguments)) { + // eslint-disable-next-line no-console + console.warn( + '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. Please use `log()` instead.', + ); + } this.native.log(`${message}`); } setAttribute(name, value) { + if (isNotModularCall(arguments)) { + // eslint-disable-next-line no-console + console.warn( + '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. Please use `log()` instead.', + ); + } if (!isString(name)) { throw new Error( 'firebase.crashlytics().setAttribute(*, _): The supplied property name must be a string.', @@ -96,6 +134,12 @@ class FirebaseCrashlyticsModule extends FirebaseModule { } setAttributes(object) { + if (isNotModularCall(arguments)) { + // eslint-disable-next-line no-console + console.warn( + '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. Please use `setAttributes()` instead.', + ); + } if (!isObject(object)) { throw new Error( 'firebase.crashlytics().setAttributes(*): The supplied arg must be an object of key value strings.', @@ -106,6 +150,12 @@ class FirebaseCrashlyticsModule extends FirebaseModule { } setUserId(userId) { + if (isNotModularCall(arguments)) { + // eslint-disable-next-line no-console + console.warn( + '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. Please use `setUserId()` instead.', + ); + } if (!isString(userId)) { throw new Error( 'firebase.crashlytics().setUserId(*): The supplied userId must be a string value.', @@ -116,6 +166,12 @@ class FirebaseCrashlyticsModule extends FirebaseModule { } recordError(error, jsErrorName) { + if (isNotModularCall(arguments)) { + // eslint-disable-next-line no-console + console.warn( + '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. Please use `recordError()` instead.', + ); + } if (isError(error)) { StackTrace.fromError(error, { offline: true }).then(stackFrames => { this.native.recordError(createNativeErrorObj(error, stackFrames, false, jsErrorName)); @@ -128,12 +184,24 @@ class FirebaseCrashlyticsModule extends FirebaseModule { } sendUnsentReports() { + if (isNotModularCall(arguments)) { + // eslint-disable-next-line no-console + console.warn( + '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. Please use `sendUnsentReports()` instead.', + ); + } if (this.isCrashlyticsCollectionEnabled) { this.native.sendUnsentReports(); } } setCrashlyticsCollectionEnabled(enabled) { + if (isNotModularCall(arguments)) { + // eslint-disable-next-line no-console + console.warn( + '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. Please use `setCrashlyticsCollectionEnabled()` instead.', + ); + } 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 fc581bce29..2de1eb76d6 100644 --- a/packages/crashlytics/lib/modular/index.js +++ b/packages/crashlytics/lib/modular/index.js @@ -1,3 +1,4 @@ +import { MODULAR_DEPRECATION_ARG } from '@react-native-firebase/app/lib/common'; import { firebase } from '..'; /** @@ -31,6 +32,12 @@ export function getCrashlytics() { * @returns {boolean} */ export function isCrashlyticsCollectionEnabled(crashlytics) { + // Deprecating this method and allow it as a property on Crashlytics instance + // auth instance has plenty of properties so I think this should also be one: https://firebase.google.com/docs/reference/js/auth.auth + // eslint-disable-next-line no-console + console.warn( + 'This method is deprecated and will be removed in the next major release. Please just use `crashlytics.isCrashlyticsCollectionEnabled` property instead.', + ); return crashlytics.isCrashlyticsCollectionEnabled; } @@ -53,7 +60,7 @@ export function isCrashlyticsCollectionEnabled(crashlytics) { * @returns {Promise} */ export function checkForUnsentReports(crashlytics) { - return crashlytics.checkForUnsentReports(); + return crashlytics.checkForUnsentReports.call(crashlytics, MODULAR_DEPRECATION_ARG); } /** @@ -70,7 +77,7 @@ export function checkForUnsentReports(crashlytics) { * @returns {Promise} */ export function deleteUnsentReports(crashlytics) { - return crashlytics.deleteUnsentReports(); + return crashlytics.deleteUnsentReports.call(crashlytics, MODULAR_DEPRECATION_ARG); } /** @@ -91,7 +98,7 @@ export function deleteUnsentReports(crashlytics) { * @returns {Promise} */ export function didCrashOnPreviousExecution(crashlytics) { - return crashlytics.didCrashOnPreviousExecution(); + return crashlytics.didCrashOnPreviousExecution.call(crashlytics, MODULAR_DEPRECATION_ARG); } /** @@ -109,7 +116,7 @@ export function didCrashOnPreviousExecution(crashlytics) { * @returns {void} */ export function crash(crashlytics) { - return crashlytics.crash(); + return crashlytics.crash.call(crashlytics, MODULAR_DEPRECATION_ARG); } /** @@ -127,7 +134,7 @@ export function crash(crashlytics) { * @returns {void} */ export function log(crashlytics, message) { - return crashlytics.log(message); + return crashlytics.log.call(crashlytics, message, MODULAR_DEPRECATION_ARG); } /** @@ -152,7 +159,7 @@ export function log(crashlytics, message) { * @returns {void} */ export function recordError(crashlytics, error, jsErrorName) { - return crashlytics.recordError(error, jsErrorName); + return crashlytics.recordError.call(crashlytics, error, jsErrorName, MODULAR_DEPRECATION_ARG); } /** @@ -169,7 +176,7 @@ export function recordError(crashlytics, error, jsErrorName) { * @returns {void} */ export function sendUnsentReports(crashlytics) { - return crashlytics.sendUnsentReports(); + return crashlytics.sendUnsentReports.call(crashlytics, MODULAR_DEPRECATION_ARG); } /** @@ -196,7 +203,7 @@ export function sendUnsentReports(crashlytics) { * @returns {Promise} */ export function setUserId(crashlytics, userId) { - return crashlytics.setUserId(userId); + return crashlytics.setUserId.call(crashlytics, userId, MODULAR_DEPRECATION_ARG); } /** @@ -214,7 +221,7 @@ export function setUserId(crashlytics, userId) { * @returns {Promise} */ export function setAttribute(crashlytics, name, value) { - return crashlytics.setAttribute(name, value); + return crashlytics.setAttribute.call(crashlytics, name, value, MODULAR_DEPRECATION_ARG); } /** @@ -234,7 +241,7 @@ export function setAttribute(crashlytics, name, value) { * @returns {Promise} */ export function setAttributes(crashlytics, attributes) { - return crashlytics.setAttributes(attributes); + return crashlytics.setAttributes.call(crashlytics, attributes, MODULAR_DEPRECATION_ARG); } /** @@ -254,5 +261,9 @@ export function setAttributes(crashlytics, attributes) { * @returns {Promise} */ export function setCrashlyticsCollectionEnabled(crashlytics, enabled) { - return crashlytics.setCrashlyticsCollectionEnabled(enabled); + return crashlytics.setCrashlyticsCollectionEnabled.call( + crashlytics, + enabled, + MODULAR_DEPRECATION_ARG, + ); } From aa803af0d012debb5bbd8836dde5d4623e651595 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Thu, 31 Oct 2024 17:54:09 +0000 Subject: [PATCH 2/8] test(crashlytics): ensure console.warns() are called correctly --- .../crashlytics/__tests__/crashlytics.test.ts | 213 +++++++++++++++++- 1 file changed, 212 insertions(+), 1 deletion(-) diff --git a/packages/crashlytics/__tests__/crashlytics.test.ts b/packages/crashlytics/__tests__/crashlytics.test.ts index 2867aecc2a..2f63042f8c 100644 --- a/packages/crashlytics/__tests__/crashlytics.test.ts +++ b/packages/crashlytics/__tests__/crashlytics.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from '@jest/globals'; +import { describe, expect, it, jest } from '@jest/globals'; import { firebase, @@ -79,4 +79,215 @@ describe('Crashlytics', function () { expect(setCrashlyticsCollectionEnabled).toBeDefined(); }); }); + + describe('test `console.warn` is called for RNFB v8 API & not called for v9 API', function () { + it('checkForUnsentReports', function () { + const crashlytics = getCrashlytics(); + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + // @ts-ignore test + const nativeMock = { checkForUnsentReports: jest.fn() }; + // @ts-ignore test + jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); + + checkForUnsentReports(crashlytics); + // Check that console.warn was not called for v9 method call + expect(consoleWarnSpy).not.toHaveBeenCalled(); + + crashlytics.checkForUnsentReports(); + // Check that console.warn was called for v8 method call + expect(consoleWarnSpy).toHaveBeenCalled(); + // Restore the original console.warn + consoleWarnSpy.mockRestore(); + }); + + it('crash', function () { + const crashlytics = getCrashlytics(); + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + // @ts-ignore test + const nativeMock = { crash: jest.fn() }; + // @ts-ignore test + jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); + + crash(crashlytics); + // Check that console.warn was not called for v9 method call + expect(consoleWarnSpy).not.toHaveBeenCalled(); + + crashlytics.crash(); + // Check that console.warn was called for v8 method call + expect(consoleWarnSpy).toHaveBeenCalled(); + // Restore the original console.warn + consoleWarnSpy.mockRestore(); + }); + + it('deleteUnsentReports', function () { + const crashlytics = getCrashlytics(); + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + // @ts-ignore test + const nativeMock = { deleteUnsentReports: jest.fn() }; + // @ts-ignore test + jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); + + deleteUnsentReports(crashlytics); + // Check that console.warn was not called for v9 method call + expect(consoleWarnSpy).not.toHaveBeenCalled(); + + crashlytics.deleteUnsentReports(); + // Check that console.warn was called for v8 method call + expect(consoleWarnSpy).toHaveBeenCalled(); + // Restore the original console.warn + consoleWarnSpy.mockRestore(); + }); + + it('didCrashOnPreviousExecution', function () { + const crashlytics = getCrashlytics(); + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + // @ts-ignore test + const nativeMock = { didCrashOnPreviousExecution: jest.fn() }; + // @ts-ignore test + jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); + + didCrashOnPreviousExecution(crashlytics); + // Check that console.warn was not called for v9 method call + expect(consoleWarnSpy).not.toHaveBeenCalled(); + + crashlytics.didCrashOnPreviousExecution(); + // Check that console.warn was called for v8 method call + expect(consoleWarnSpy).toHaveBeenCalled(); + // Restore the original console.warn + consoleWarnSpy.mockRestore(); + }); + + it('log', function () { + const crashlytics = getCrashlytics(); + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + // @ts-ignore test + const nativeMock = { log: jest.fn() }; + // @ts-ignore test + jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); + + log(crashlytics, 'message'); + // Check that console.warn was not called for v9 method call + expect(consoleWarnSpy).not.toHaveBeenCalled(); + + crashlytics.log('message'); + // Check that console.warn was called for v8 method call + expect(consoleWarnSpy).toHaveBeenCalled(); + // Restore the original console.warn + consoleWarnSpy.mockRestore(); + }); + + it('setAttribute', function () { + const crashlytics = getCrashlytics(); + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + // @ts-ignore test + const nativeMock = { setAttribute: jest.fn() }; + // @ts-ignore test + jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); + + setAttribute(crashlytics, 'name', 'value'); + // Check that console.warn was not called for v9 method call + expect(consoleWarnSpy).not.toHaveBeenCalled(); + + crashlytics.setAttribute('name', 'value'); + // Check that console.warn was called for v8 method call + expect(consoleWarnSpy).toHaveBeenCalled(); + // Restore the original console.warn + consoleWarnSpy.mockRestore(); + }); + + it('setAttributes', function () { + const crashlytics = getCrashlytics(); + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + // @ts-ignore test + const nativeMock = { setAttributes: jest.fn() }; + // @ts-ignore test + jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); + + setAttributes(crashlytics, {}); + // Check that console.warn was not called for v9 method call + expect(consoleWarnSpy).not.toHaveBeenCalled(); + + crashlytics.setAttributes({}); + // Check that console.warn was called for v8 method call + expect(consoleWarnSpy).toHaveBeenCalled(); + // Restore the original console.warn + consoleWarnSpy.mockRestore(); + }); + + it('setUserId', function () { + const crashlytics = getCrashlytics(); + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + // @ts-ignore test + const nativeMock = { setUserId: jest.fn() }; + // @ts-ignore test + jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); + + setUserId(crashlytics, 'id'); + // Check that console.warn was not called for v9 method call + expect(consoleWarnSpy).not.toHaveBeenCalled(); + + crashlytics.setUserId('id'); + // Check that console.warn was called for v8 method call + expect(consoleWarnSpy).toHaveBeenCalled(); + // Restore the original console.warn + consoleWarnSpy.mockRestore(); + }); + + it('recordError', function () { + const crashlytics = getCrashlytics(); + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + // @ts-ignore test + const nativeMock = { recordError: jest.fn() }; + // @ts-ignore test + jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); + + recordError(crashlytics, new Error(), 'name'); + // Check that console.warn was not called for v9 method call + expect(consoleWarnSpy).not.toHaveBeenCalled(); + + crashlytics.recordError(new Error(), 'name'); + // Check that console.warn was called for v8 method call + expect(consoleWarnSpy).toHaveBeenCalled(); + // Restore the original console.warn + consoleWarnSpy.mockRestore(); + }); + + it('sendUnsentReports', function () { + const crashlytics = getCrashlytics(); + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + // @ts-ignore test + const nativeMock = { sendUnsentReports: jest.fn() }; + // @ts-ignore test + jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); + + sendUnsentReports(crashlytics); + // Check that console.warn was not called for v9 method call + expect(consoleWarnSpy).not.toHaveBeenCalled(); + + crashlytics.sendUnsentReports(); + // Check that console.warn was called for v8 method call + expect(consoleWarnSpy).toHaveBeenCalled(); + // Restore the original console.warn + consoleWarnSpy.mockRestore(); + }); + + it('setCrashlyticsCollectionEnabled', function () { + const crashlytics = getCrashlytics(); + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + // @ts-ignore test + const nativeMock = { setCrashlyticsCollectionEnabled: jest.fn() }; + // @ts-ignore test + jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); + + setCrashlyticsCollectionEnabled(crashlytics, true); + // Check that console.warn was not called for v9 method call + expect(consoleWarnSpy).not.toHaveBeenCalled(); + + crashlytics.setCrashlyticsCollectionEnabled(true); + // Check that console.warn was called for v8 method call + expect(consoleWarnSpy).toHaveBeenCalled(); + // Restore the original console.warn + consoleWarnSpy.mockRestore(); + }); + }); }); From 87a64736475430bad5bf50ce84bbe080b8fc5e8a Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Thu, 31 Oct 2024 18:08:08 +0000 Subject: [PATCH 3/8] chore: update console.warn --- packages/crashlytics/lib/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/crashlytics/lib/index.js b/packages/crashlytics/lib/index.js index e58ecb9eff..ffd8e2bccc 100644 --- a/packages/crashlytics/lib/index.js +++ b/packages/crashlytics/lib/index.js @@ -115,7 +115,7 @@ class FirebaseCrashlyticsModule extends FirebaseModule { if (isNotModularCall(arguments)) { // eslint-disable-next-line no-console console.warn( - '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. Please use `log()` instead.', + '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. Please use `setAttribute()` instead.', ); } if (!isString(name)) { From 9ac873cec05ba62ec06f5130940db539af870e24 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Wed, 13 Nov 2024 14:59:30 +0000 Subject: [PATCH 4/8] test(crashlytics): fix test to ignore deprecation warning --- packages/crashlytics/e2e/crashlytics.e2e.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/crashlytics/e2e/crashlytics.e2e.js b/packages/crashlytics/e2e/crashlytics.e2e.js index 9ec30f2aa5..0086cfbd6e 100644 --- a/packages/crashlytics/e2e/crashlytics.e2e.js +++ b/packages/crashlytics/e2e/crashlytics.e2e.js @@ -90,10 +90,13 @@ describe('crashlytics()', function () { let logged = false; // eslint-disable-next-line no-console console.warn = msg => { - msg.should.containEql('expects an instance of Error'); - logged = true; - // eslint-disable-next-line no-console - console.warn = orig; + // we console.warn for deprecated API, can be removed when we move to v9 + if (!msg.includes('v8 method is deprecated')) { + msg.should.containEql('expects an instance of Error'); + logged = true; + // eslint-disable-next-line no-console + console.warn = orig; + } }; firebase.crashlytics().recordError(1337); From b66ce06b03b86c9db8663e527e7025eaaede40e4 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Tue, 19 Nov 2024 18:12:11 +0000 Subject: [PATCH 5/8] chore(crashlytics): move to util functions --- .../crashlytics/__tests__/crashlytics.test.ts | 156 +++++------------- packages/crashlytics/lib/index.js | 79 ++------- packages/crashlytics/lib/modular/index.js | 13 +- 3 files changed, 63 insertions(+), 185 deletions(-) diff --git a/packages/crashlytics/__tests__/crashlytics.test.ts b/packages/crashlytics/__tests__/crashlytics.test.ts index 2f63042f8c..d976cfaf74 100644 --- a/packages/crashlytics/__tests__/crashlytics.test.ts +++ b/packages/crashlytics/__tests__/crashlytics.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, jest } from '@jest/globals'; - +import { checkV9Deprecation } from '../../app/lib/common/unitTestUtils'; import { firebase, getCrashlytics, @@ -83,211 +83,145 @@ describe('Crashlytics', function () { describe('test `console.warn` is called for RNFB v8 API & not called for v9 API', function () { it('checkForUnsentReports', function () { const crashlytics = getCrashlytics(); - const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); // @ts-ignore test const nativeMock = { checkForUnsentReports: jest.fn() }; // @ts-ignore test jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); - checkForUnsentReports(crashlytics); - // Check that console.warn was not called for v9 method call - expect(consoleWarnSpy).not.toHaveBeenCalled(); - - crashlytics.checkForUnsentReports(); - // Check that console.warn was called for v8 method call - expect(consoleWarnSpy).toHaveBeenCalled(); - // Restore the original console.warn - consoleWarnSpy.mockRestore(); + checkV9Deprecation( + () => checkForUnsentReports(crashlytics), + () => crashlytics.checkForUnsentReports(), + ); }); it('crash', function () { const crashlytics = getCrashlytics(); - const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); // @ts-ignore test const nativeMock = { crash: jest.fn() }; // @ts-ignore test jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); - crash(crashlytics); - // Check that console.warn was not called for v9 method call - expect(consoleWarnSpy).not.toHaveBeenCalled(); - - crashlytics.crash(); - // Check that console.warn was called for v8 method call - expect(consoleWarnSpy).toHaveBeenCalled(); - // Restore the original console.warn - consoleWarnSpy.mockRestore(); + checkV9Deprecation( + () => crash(crashlytics), + () => crashlytics.crash(), + ); }); it('deleteUnsentReports', function () { const crashlytics = getCrashlytics(); - const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); // @ts-ignore test const nativeMock = { deleteUnsentReports: jest.fn() }; // @ts-ignore test jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); - deleteUnsentReports(crashlytics); - // Check that console.warn was not called for v9 method call - expect(consoleWarnSpy).not.toHaveBeenCalled(); - - crashlytics.deleteUnsentReports(); - // Check that console.warn was called for v8 method call - expect(consoleWarnSpy).toHaveBeenCalled(); - // Restore the original console.warn - consoleWarnSpy.mockRestore(); + checkV9Deprecation( + () => deleteUnsentReports(crashlytics), + () => crashlytics.deleteUnsentReports(), + ); }); it('didCrashOnPreviousExecution', function () { const crashlytics = getCrashlytics(); - const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); // @ts-ignore test const nativeMock = { didCrashOnPreviousExecution: jest.fn() }; // @ts-ignore test jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); - didCrashOnPreviousExecution(crashlytics); - // Check that console.warn was not called for v9 method call - expect(consoleWarnSpy).not.toHaveBeenCalled(); - - crashlytics.didCrashOnPreviousExecution(); - // Check that console.warn was called for v8 method call - expect(consoleWarnSpy).toHaveBeenCalled(); - // Restore the original console.warn - consoleWarnSpy.mockRestore(); + checkV9Deprecation( + () => didCrashOnPreviousExecution(crashlytics), + () => crashlytics.didCrashOnPreviousExecution(), + ); }); it('log', function () { const crashlytics = getCrashlytics(); - const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); // @ts-ignore test const nativeMock = { log: jest.fn() }; // @ts-ignore test jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); - log(crashlytics, 'message'); - // Check that console.warn was not called for v9 method call - expect(consoleWarnSpy).not.toHaveBeenCalled(); - - crashlytics.log('message'); - // Check that console.warn was called for v8 method call - expect(consoleWarnSpy).toHaveBeenCalled(); - // Restore the original console.warn - consoleWarnSpy.mockRestore(); + checkV9Deprecation( + () => log(crashlytics, 'message'), + () => crashlytics.log('message'), + ); }); it('setAttribute', function () { const crashlytics = getCrashlytics(); - const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); // @ts-ignore test const nativeMock = { setAttribute: jest.fn() }; // @ts-ignore test jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); - setAttribute(crashlytics, 'name', 'value'); - // Check that console.warn was not called for v9 method call - expect(consoleWarnSpy).not.toHaveBeenCalled(); - - crashlytics.setAttribute('name', 'value'); - // Check that console.warn was called for v8 method call - expect(consoleWarnSpy).toHaveBeenCalled(); - // Restore the original console.warn - consoleWarnSpy.mockRestore(); + checkV9Deprecation( + () => setAttribute(crashlytics, 'name', 'value'), + () => crashlytics.setAttribute('name', 'value'), + ); }); it('setAttributes', function () { const crashlytics = getCrashlytics(); - const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); // @ts-ignore test const nativeMock = { setAttributes: jest.fn() }; // @ts-ignore test jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); - setAttributes(crashlytics, {}); - // Check that console.warn was not called for v9 method call - expect(consoleWarnSpy).not.toHaveBeenCalled(); - - crashlytics.setAttributes({}); - // Check that console.warn was called for v8 method call - expect(consoleWarnSpy).toHaveBeenCalled(); - // Restore the original console.warn - consoleWarnSpy.mockRestore(); + checkV9Deprecation( + () => setAttributes(crashlytics, {}), + () => crashlytics.setAttributes({}), + ); }); it('setUserId', function () { const crashlytics = getCrashlytics(); - const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); // @ts-ignore test const nativeMock = { setUserId: jest.fn() }; // @ts-ignore test jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); - setUserId(crashlytics, 'id'); - // Check that console.warn was not called for v9 method call - expect(consoleWarnSpy).not.toHaveBeenCalled(); - - crashlytics.setUserId('id'); - // Check that console.warn was called for v8 method call - expect(consoleWarnSpy).toHaveBeenCalled(); - // Restore the original console.warn - consoleWarnSpy.mockRestore(); + checkV9Deprecation( + () => setUserId(crashlytics, 'id'), + () => crashlytics.setUserId('id'), + ); }); it('recordError', function () { const crashlytics = getCrashlytics(); - const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); // @ts-ignore test const nativeMock = { recordError: jest.fn() }; // @ts-ignore test jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); - recordError(crashlytics, new Error(), 'name'); - // Check that console.warn was not called for v9 method call - expect(consoleWarnSpy).not.toHaveBeenCalled(); - - crashlytics.recordError(new Error(), 'name'); - // Check that console.warn was called for v8 method call - expect(consoleWarnSpy).toHaveBeenCalled(); - // Restore the original console.warn - consoleWarnSpy.mockRestore(); + checkV9Deprecation( + () => recordError(crashlytics, new Error(), 'name'), + () => crashlytics.recordError(new Error(), 'name'), + ); }); it('sendUnsentReports', function () { const crashlytics = getCrashlytics(); - const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); // @ts-ignore test const nativeMock = { sendUnsentReports: jest.fn() }; // @ts-ignore test jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); - sendUnsentReports(crashlytics); - // Check that console.warn was not called for v9 method call - expect(consoleWarnSpy).not.toHaveBeenCalled(); - - crashlytics.sendUnsentReports(); - // Check that console.warn was called for v8 method call - expect(consoleWarnSpy).toHaveBeenCalled(); - // Restore the original console.warn - consoleWarnSpy.mockRestore(); + checkV9Deprecation( + () => sendUnsentReports(crashlytics), + () => crashlytics.sendUnsentReports(), + ); }); it('setCrashlyticsCollectionEnabled', function () { const crashlytics = getCrashlytics(); - const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); // @ts-ignore test const nativeMock = { setCrashlyticsCollectionEnabled: jest.fn() }; // @ts-ignore test jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); - setCrashlyticsCollectionEnabled(crashlytics, true); - // Check that console.warn was not called for v9 method call - expect(consoleWarnSpy).not.toHaveBeenCalled(); - - crashlytics.setCrashlyticsCollectionEnabled(true); - // Check that console.warn was called for v8 method call - expect(consoleWarnSpy).toHaveBeenCalled(); - // Restore the original console.warn - consoleWarnSpy.mockRestore(); + checkV9Deprecation( + () => setCrashlyticsCollectionEnabled(crashlytics, true), + () => crashlytics.setCrashlyticsCollectionEnabled(true), + ); }); }); }); diff --git a/packages/crashlytics/lib/index.js b/packages/crashlytics/lib/index.js index ffd8e2bccc..3b48e80d39 100644 --- a/packages/crashlytics/lib/index.js +++ b/packages/crashlytics/lib/index.js @@ -22,7 +22,7 @@ import { isObject, isString, isOther, - isNotModularCall, + warnIfNotModularCall, } from '@react-native-firebase/app/lib/common'; import { createModuleNamespace, @@ -57,12 +57,7 @@ class FirebaseCrashlyticsModule extends FirebaseModule { } checkForUnsentReports() { - if (isNotModularCall(arguments)) { - // eslint-disable-next-line no-console - console.warn( - '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. Please use `checkForUnsentReports()` instead.', - ); - } + warnIfNotModularCall(arguments, 'checkForUnsentReports()'); if (this.isCrashlyticsCollectionEnabled) { throw new Error( "firebase.crashlytics().setCrashlyticsCollectionEnabled(*) has been set to 'true', all reports are automatically sent.", @@ -72,52 +67,27 @@ class FirebaseCrashlyticsModule extends FirebaseModule { } crash() { - if (isNotModularCall(arguments)) { - // eslint-disable-next-line no-console - console.warn( - '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. Please use `crash()` instead.', - ); - } + warnIfNotModularCall(arguments, 'crash()'); this.native.crash(); } async deleteUnsentReports() { - if (isNotModularCall(arguments)) { - // eslint-disable-next-line no-console - console.warn( - '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. Please use `deleteUnsentReports()` instead.', - ); - } + warnIfNotModularCall(arguments, 'deleteUnsentReports()'); await this.native.deleteUnsentReports(); } didCrashOnPreviousExecution() { - if (isNotModularCall(arguments)) { - // eslint-disable-next-line no-console - console.warn( - '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. Please use `didCrashOnPreviousExecution()` instead.', - ); - } + warnIfNotModularCall(arguments, 'didCrashOnPreviousExecution()'); return this.native.didCrashOnPreviousExecution(); } log(message) { - if (isNotModularCall(arguments)) { - // eslint-disable-next-line no-console - console.warn( - '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. Please use `log()` instead.', - ); - } + warnIfNotModularCall(arguments, 'log()'); this.native.log(`${message}`); } setAttribute(name, value) { - if (isNotModularCall(arguments)) { - // eslint-disable-next-line no-console - console.warn( - '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. Please use `setAttribute()` instead.', - ); - } + warnIfNotModularCall(arguments, 'setAttribute()'); if (!isString(name)) { throw new Error( 'firebase.crashlytics().setAttribute(*, _): The supplied property name must be a string.', @@ -134,12 +104,7 @@ class FirebaseCrashlyticsModule extends FirebaseModule { } setAttributes(object) { - if (isNotModularCall(arguments)) { - // eslint-disable-next-line no-console - console.warn( - '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. Please use `setAttributes()` instead.', - ); - } + warnIfNotModularCall(arguments, 'setAttributes()'); if (!isObject(object)) { throw new Error( 'firebase.crashlytics().setAttributes(*): The supplied arg must be an object of key value strings.', @@ -150,12 +115,7 @@ class FirebaseCrashlyticsModule extends FirebaseModule { } setUserId(userId) { - if (isNotModularCall(arguments)) { - // eslint-disable-next-line no-console - console.warn( - '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. Please use `setUserId()` instead.', - ); - } + warnIfNotModularCall(arguments, 'setUserId()'); if (!isString(userId)) { throw new Error( 'firebase.crashlytics().setUserId(*): The supplied userId must be a string value.', @@ -166,12 +126,7 @@ class FirebaseCrashlyticsModule extends FirebaseModule { } recordError(error, jsErrorName) { - if (isNotModularCall(arguments)) { - // eslint-disable-next-line no-console - console.warn( - '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. Please use `recordError()` instead.', - ); - } + warnIfNotModularCall(arguments, 'recordError()'); if (isError(error)) { StackTrace.fromError(error, { offline: true }).then(stackFrames => { this.native.recordError(createNativeErrorObj(error, stackFrames, false, jsErrorName)); @@ -184,24 +139,14 @@ class FirebaseCrashlyticsModule extends FirebaseModule { } sendUnsentReports() { - if (isNotModularCall(arguments)) { - // eslint-disable-next-line no-console - console.warn( - '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. Please use `sendUnsentReports()` instead.', - ); - } + warnIfNotModularCall(arguments, 'sendUnsentReports()'); if (this.isCrashlyticsCollectionEnabled) { this.native.sendUnsentReports(); } } setCrashlyticsCollectionEnabled(enabled) { - if (isNotModularCall(arguments)) { - // eslint-disable-next-line no-console - console.warn( - '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. Please use `setCrashlyticsCollectionEnabled()` instead.', - ); - } + 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 2de1eb76d6..29e8a4a30e 100644 --- a/packages/crashlytics/lib/modular/index.js +++ b/packages/crashlytics/lib/modular/index.js @@ -1,4 +1,7 @@ -import { MODULAR_DEPRECATION_ARG } from '@react-native-firebase/app/lib/common'; +import { + MODULAR_DEPRECATION_ARG, + warnIfNotModularCall, +} from '@react-native-firebase/app/lib/common'; import { firebase } from '..'; /** @@ -32,12 +35,8 @@ export function getCrashlytics() { * @returns {boolean} */ export function isCrashlyticsCollectionEnabled(crashlytics) { - // Deprecating this method and allow it as a property on Crashlytics instance - // auth instance has plenty of properties so I think this should also be one: https://firebase.google.com/docs/reference/js/auth.auth - // eslint-disable-next-line no-console - console.warn( - 'This method is deprecated and will be removed in the next major release. Please just use `crashlytics.isCrashlyticsCollectionEnabled` property instead.', - ); + // Deprecating this method and allow it as a property on Crashlytics instance. + warnIfNotModularCall([], 'crashlytics.isCrashlyticsCollectionEnabled'); return crashlytics.isCrashlyticsCollectionEnabled; } From cd09fea60d00e3073cdd5b21d9400444efb44d42 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Wed, 20 Nov 2024 10:31:29 +0000 Subject: [PATCH 6/8] test: isCrashlyticsCollectionEnabled console warning --- packages/crashlytics/__tests__/crashlytics.test.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/crashlytics/__tests__/crashlytics.test.ts b/packages/crashlytics/__tests__/crashlytics.test.ts index d976cfaf74..dab5dc1728 100644 --- a/packages/crashlytics/__tests__/crashlytics.test.ts +++ b/packages/crashlytics/__tests__/crashlytics.test.ts @@ -223,5 +223,14 @@ describe('Crashlytics', function () { () => crashlytics.setCrashlyticsCollectionEnabled(true), ); }); + + it('isCrashlyticsCollectionEnabled', function () { + const crashlytics = getCrashlytics(); + checkV9Deprecation( + // swapped order here because we're deprecating the modular method and keeping the property on Crashlytics instance + () => crashlytics.isCrashlyticsCollectionEnabled, + () => isCrashlyticsCollectionEnabled(crashlytics), + ); + }); }); }); From f5d21524f65d4f9e237a8dc5c6d7c68a26e4e2fb Mon Sep 17 00:00:00 2001 From: Russell Wheatley Date: Fri, 22 Nov 2024 13:48:14 +0000 Subject: [PATCH 7/8] chore(crashlytics): refactor deprecation implementation (#8155) --- packages/app/lib/common/index.js | 58 ++++++++++++ packages/app/lib/common/unitTestUtils.ts | 40 +++++++- .../app/lib/internal/registry/namespace.js | 30 +++++- .../crashlytics/__tests__/crashlytics.test.ts | 93 +++++++------------ packages/crashlytics/lib/index.js | 12 --- packages/crashlytics/lib/modular/index.js | 12 +-- 6 files changed, 164 insertions(+), 81 deletions(-) diff --git a/packages/app/lib/common/index.js b/packages/app/lib/common/index.js index 68dcc755ed..832cad9377 100644 --- a/packages/app/lib/common/index.js +++ b/packages/app/lib/common/index.js @@ -103,6 +103,64 @@ export function tryJSONStringify(data) { } } +// Used to indicate if there is no corresponding modular function +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) { + const message = createMessage(moduleName, methodName); + + // eslint-disable-next-line no-console + console.warn(message); + } + } + } +} + +export function createMessage(moduleName, methodName, uniqueMessage = '') { + if (uniqueMessage.length > 0) { + // Unique deprecation message used for testing + return uniqueMessage; + } + + const moduleMap = mapOfDeprecationReplacements[moduleName]; + if (moduleMap) { + const replacementMethodName = moduleMap[methodName]; + 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..7196fd99b4 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 { createMessage } 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.mockReset(); + modularFunction(); + expect(consoleWarnSpy).not.toHaveBeenCalled(); + consoleWarnSpy.mockReset(); + const consoleWarnSpy2 = jest.spyOn(console, 'warn').mockImplementation(warnMessage => { + const message = createMessage(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..6ee73bf20b 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,28 @@ 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 dab5dc1728..7e9775445e 100644 --- a/packages/crashlytics/__tests__/crashlytics.test.ts +++ b/packages/crashlytics/__tests__/crashlytics.test.ts @@ -1,5 +1,10 @@ -import { describe, expect, it, jest } from '@jest/globals'; -import { checkV9Deprecation } from '../../app/lib/common/unitTestUtils'; +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 { firebase, getCrashlytics, @@ -81,146 +86,118 @@ describe('Crashlytics', function () { }); describe('test `console.warn` is called for RNFB v8 API & not called for v9 API', function () { - it('checkForUnsentReports', function () { - const crashlytics = getCrashlytics(); - // @ts-ignore test - const nativeMock = { checkForUnsentReports: jest.fn() }; + let checkV9Deprecation: CheckV9DeprecationFunction; + + beforeEach(function () { + checkV9Deprecation = createCheckV9Deprecation('crashlytics'); + // @ts-ignore test - jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); + jest.spyOn(FirebaseModule.prototype, 'native', 'get').mockImplementation(() => { + return new Proxy( + {}, + { + get: () => jest.fn(), + }, + ); + }); + }); + it('checkForUnsentReports', function () { + const crashlytics = getCrashlytics(); checkV9Deprecation( () => checkForUnsentReports(crashlytics), () => crashlytics.checkForUnsentReports(), + 'checkForUnsentReports', ); }); it('crash', function () { const crashlytics = getCrashlytics(); - // @ts-ignore test - const nativeMock = { crash: jest.fn() }; - // @ts-ignore test - jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); - checkV9Deprecation( () => crash(crashlytics), () => crashlytics.crash(), + 'crash', ); }); it('deleteUnsentReports', function () { const crashlytics = getCrashlytics(); - // @ts-ignore test - const nativeMock = { deleteUnsentReports: jest.fn() }; - // @ts-ignore test - jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); - checkV9Deprecation( () => deleteUnsentReports(crashlytics), () => crashlytics.deleteUnsentReports(), + 'deleteUnsentReports', ); }); it('didCrashOnPreviousExecution', function () { const crashlytics = getCrashlytics(); - // @ts-ignore test - const nativeMock = { didCrashOnPreviousExecution: jest.fn() }; - // @ts-ignore test - jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); - checkV9Deprecation( () => didCrashOnPreviousExecution(crashlytics), () => crashlytics.didCrashOnPreviousExecution(), + 'didCrashOnPreviousExecution', ); }); it('log', function () { const crashlytics = getCrashlytics(); - // @ts-ignore test - const nativeMock = { log: jest.fn() }; - // @ts-ignore test - jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); - checkV9Deprecation( () => log(crashlytics, 'message'), () => crashlytics.log('message'), + 'log', ); }); it('setAttribute', function () { const crashlytics = getCrashlytics(); - // @ts-ignore test - const nativeMock = { setAttribute: jest.fn() }; - // @ts-ignore test - jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); - checkV9Deprecation( () => setAttribute(crashlytics, 'name', 'value'), () => crashlytics.setAttribute('name', 'value'), + 'setAttribute', ); }); it('setAttributes', function () { const crashlytics = getCrashlytics(); - // @ts-ignore test - const nativeMock = { setAttributes: jest.fn() }; - // @ts-ignore test - jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); - checkV9Deprecation( () => setAttributes(crashlytics, {}), () => crashlytics.setAttributes({}), + 'setAttributes', ); }); it('setUserId', function () { const crashlytics = getCrashlytics(); - // @ts-ignore test - const nativeMock = { setUserId: jest.fn() }; - // @ts-ignore test - jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); - checkV9Deprecation( () => setUserId(crashlytics, 'id'), () => crashlytics.setUserId('id'), + 'setUserId', ); }); it('recordError', function () { const crashlytics = getCrashlytics(); - // @ts-ignore test - const nativeMock = { recordError: jest.fn() }; - // @ts-ignore test - jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); - checkV9Deprecation( () => recordError(crashlytics, new Error(), 'name'), () => crashlytics.recordError(new Error(), 'name'), + 'recordError', ); }); it('sendUnsentReports', function () { const crashlytics = getCrashlytics(); - // @ts-ignore test - const nativeMock = { sendUnsentReports: jest.fn() }; - // @ts-ignore test - jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); - checkV9Deprecation( () => sendUnsentReports(crashlytics), () => crashlytics.sendUnsentReports(), + 'sendUnsentReports', ); }); it('setCrashlyticsCollectionEnabled', function () { const crashlytics = getCrashlytics(); - // @ts-ignore test - const nativeMock = { setCrashlyticsCollectionEnabled: jest.fn() }; - // @ts-ignore test - jest.spyOn(crashlytics, 'native', 'get').mockReturnValue(nativeMock); - checkV9Deprecation( () => setCrashlyticsCollectionEnabled(crashlytics, true), () => crashlytics.setCrashlyticsCollectionEnabled(true), + 'setCrashlyticsCollectionEnabled', ); }); @@ -230,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; } From afc13efc67eb08d1a22f814822c1c74eadc889e4 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Fri, 22 Nov 2024 13:53:23 +0000 Subject: [PATCH 8/8] chore: improve message logic --- packages/app/lib/common/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/app/lib/common/index.js b/packages/app/lib/common/index.js index 832cad9377..9f7cb63bb6 100644 --- a/packages/app/lib/common/index.js +++ b/packages/app/lib/common/index.js @@ -152,11 +152,11 @@ export function createMessage(moduleName, methodName, uniqueMessage = '') { if (moduleMap) { const replacementMethodName = moduleMap[methodName]; if (replacementMethodName) { - let message; if (replacementMethodName !== NO_REPLACEMENT) { - message = v8deprecationMessage + ` Please use \`${replacementMethodName}\` instead.`; + return v8deprecationMessage + ` Please use \`${replacementMethodName}\` instead.`; + } else { + return v8deprecationMessage; } - return message; } } }