Skip to content

Commit

Permalink
refactor: the way we deprecate methods
Browse files Browse the repository at this point in the history
  • Loading branch information
russellwheatley committed Nov 22, 2024
1 parent 2a77c9b commit 151a8ef
Show file tree
Hide file tree
Showing 6 changed files with 150 additions and 29 deletions.
60 changes: 60 additions & 0 deletions packages/app/lib/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
40 changes: 38 additions & 2 deletions packages/app/lib/common/unitTestUtils.ts
Original file line number Diff line number Diff line change
@@ -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();
};
};
27 changes: 23 additions & 4 deletions packages/app/lib/internal/registry/namespace.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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
Expand Down
28 changes: 23 additions & 5 deletions packages/crashlytics/__tests__/crashlytics.test.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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(
Expand All @@ -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',
);
});

Expand All @@ -118,6 +125,7 @@ describe('Crashlytics', function () {
checkV9Deprecation(
() => deleteUnsentReports(crashlytics),
() => crashlytics.deleteUnsentReports(),
'deleteUnsentReports',
);
});

Expand All @@ -126,6 +134,7 @@ describe('Crashlytics', function () {
checkV9Deprecation(
() => didCrashOnPreviousExecution(crashlytics),
() => crashlytics.didCrashOnPreviousExecution(),
'didCrashOnPreviousExecution',
);
});

Expand All @@ -134,6 +143,7 @@ describe('Crashlytics', function () {
checkV9Deprecation(
() => log(crashlytics, 'message'),
() => crashlytics.log('message'),
'log',
);
});

Expand All @@ -142,6 +152,7 @@ describe('Crashlytics', function () {
checkV9Deprecation(
() => setAttribute(crashlytics, 'name', 'value'),
() => crashlytics.setAttribute('name', 'value'),
'setAttribute',
);
});

Expand All @@ -150,6 +161,7 @@ describe('Crashlytics', function () {
checkV9Deprecation(
() => setAttributes(crashlytics, {}),
() => crashlytics.setAttributes({}),
'setAttributes',
);
});

Expand All @@ -158,6 +170,7 @@ describe('Crashlytics', function () {
checkV9Deprecation(
() => setUserId(crashlytics, 'id'),
() => crashlytics.setUserId('id'),
'setUserId',
);
});

Expand All @@ -166,6 +179,7 @@ describe('Crashlytics', function () {
checkV9Deprecation(
() => recordError(crashlytics, new Error(), 'name'),
() => crashlytics.recordError(new Error(), 'name'),
'recordError',
);
});

Expand All @@ -174,6 +188,7 @@ describe('Crashlytics', function () {
checkV9Deprecation(
() => sendUnsentReports(crashlytics),
() => crashlytics.sendUnsentReports(),
'sendUnsentReports',
);
});

Expand All @@ -182,6 +197,7 @@ describe('Crashlytics', function () {
checkV9Deprecation(
() => setCrashlyticsCollectionEnabled(crashlytics, true),
() => crashlytics.setCrashlyticsCollectionEnabled(true),
'setCrashlyticsCollectionEnabled',
);
});

Expand All @@ -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',
);
});
});
Expand Down
12 changes: 0 additions & 12 deletions packages/crashlytics/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import {
isObject,
isString,
isOther,
warnIfNotModularCall,
} from '@react-native-firebase/app/lib/common';
import {
createModuleNamespace,
Expand Down Expand Up @@ -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.",
Expand All @@ -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.',
Expand All @@ -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.',
Expand All @@ -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.',
Expand All @@ -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));
Expand All @@ -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.",
Expand Down
12 changes: 6 additions & 6 deletions packages/crashlytics/lib/modular/index.js
Original file line number Diff line number Diff line change
@@ -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 '..';

/**
Expand Down Expand Up @@ -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;
}

Expand Down

0 comments on commit 151a8ef

Please sign in to comment.