Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: de-duplicate deprecation warning calls and tests #8135

Merged
merged 1 commit into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 43 additions & 55 deletions packages/app/__tests__/app.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,16 @@ import firebase, {
setLogLevel,
} from '../lib';

// this could be extracted to some test utils location
const checkV9Deprecation = (modularFunction: () => void, nonModularFunction: () => void) => {
const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
modularFunction();
expect(consoleWarnSpy).not.toHaveBeenCalled();
nonModularFunction();
expect(consoleWarnSpy).toHaveBeenCalledTimes(1);
consoleWarnSpy.mockRestore();
};

describe('App', function () {
describe('modular', function () {
it('`deleteApp` function is properly exposed to end user', function () {
Expand Down Expand Up @@ -41,78 +51,56 @@ describe('App', function () {
});
});

describe('test `console.warn` is called for RNFB v8 API & not called for v9 API', function () {
describe('`console.warn` only called for non-modular API', function () {
it('deleteApp', function () {
const firebaseApp = firebase.app();
const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
// this test has a slightly special setup
// @ts-ignore test
jest.spyOn(firebaseApp, '_deleteApp').mockImplementation(() => Promise.resolve(null));
// we don't need to test this because we call underlying deleteApp directly
// deleteApp(firebaseApp);

firebaseApp.delete();
// Check that console.warn was called for v8 method call
expect(consoleWarnSpy).toHaveBeenCalled();
// Restore the original console.warn
consoleWarnSpy.mockRestore();
jest.spyOn(getApp(), '_deleteApp').mockImplementation(() => Promise.resolve(null));
checkV9Deprecation(
() => {}, // no modular replacement
() => getApp().delete(), // modular getApp(), then non-modular to check
);
});

it('getApps', function () {
const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
// Check that console.warn was not called for v9 method call
getApps();
expect(consoleWarnSpy).not.toHaveBeenCalled();
// Check that console.warn was called for v8 method call
firebase.apps;
expect(consoleWarnSpy).toHaveBeenCalled();
// Restore the original console.warn
consoleWarnSpy.mockRestore();
checkV9Deprecation(
() => getApps(),
() => firebase.apps,
);
});

it('getApp', function () {
const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
// Check that console.warn was not called for v9 method call
getApp();
expect(consoleWarnSpy).not.toHaveBeenCalled();
// Check that console.warn was called for v8 method call
firebase.app();
expect(consoleWarnSpy).toHaveBeenCalled();
// Restore the original console.warn
consoleWarnSpy.mockRestore();
checkV9Deprecation(
() => getApp(),
() => firebase.app(),
);
});

it('setLogLevel', function () {
const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
// Check that console.warn was not called for v9 method call
setLogLevel('debug');
expect(consoleWarnSpy).not.toHaveBeenCalled();
// Check that console.warn was called for v8 method call
firebase.setLogLevel('debug');
expect(consoleWarnSpy).toHaveBeenCalled();
// Restore the original console.warn
consoleWarnSpy.mockRestore();
checkV9Deprecation(
() => setLogLevel('debug'),
() => firebase.setLogLevel('debug'),
);
});

it('FirebaseApp.toString()', function () {
const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
const app = firebase.app();
app.toString();
// Check that console.warn was called for deprecated method call
firebase.setLogLevel('debug');
expect(consoleWarnSpy).toHaveBeenCalled();
// Restore the original console.warn
consoleWarnSpy.mockRestore();
checkV9Deprecation(
() => {}, // no modular replacement
() => getApp().toString(), // modular getApp(), then non-modular to check
);
});

it('FirebaseApp.extendApp()', function () {
const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
const app = firebase.app();
// To overcome type assertion
(app as any).extendApp({ some: 'property' });
// Check that console.warn was called for deprecated method call
expect(consoleWarnSpy).toHaveBeenCalled();
// Restore the original console.warn
consoleWarnSpy.mockRestore();
checkV9Deprecation(
// no modular replacement for this one so no modular func to send in
() => {},
// modular getApp(), then non-modular to check
() => {
const app = getApp();
(app as any).extendApp({ some: 'property' });
return;
},
);
});
});
});
24 changes: 6 additions & 18 deletions packages/app/lib/FirebaseApp.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*
*/
import { isNotModularCall } from '@react-native-firebase/app/lib/common';
import { warnIfNotModularCall } from '@react-native-firebase/app/lib/common';
import { getAppModule } from './internal/registry/nativeModule';

export default class FirebaseApp {
Expand Down Expand Up @@ -61,33 +61,21 @@ export default class FirebaseApp {
}

extendApp(extendedProps) {
// 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.',
);

// this method has no modular alternative, send true for param 'noAlternative'
warnIfNotModularCall(arguments, '', true);
this._checkDestroyed();
Object.assign(this, extendedProps);
}

delete() {
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 `deleteApp()` instead.',
);
}
warnIfNotModularCall(arguments, 'deleteApp()');
this._checkDestroyed();
return this._deleteApp();
}

toString() {
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.',
);
}
// this method has no modular alternative, send true for param 'noAlternative'
warnIfNotModularCall(arguments, '', true);
return this.name;
}
}
15 changes: 12 additions & 3 deletions packages/app/lib/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,20 @@ export function tryJSONStringify(data) {

export const MODULAR_DEPRECATION_ARG = 'react-native-firebase-modular-method-call';

export function isNotModularCall(args) {
export function warnIfNotModularCall(args, replacementMethodName, noAlternative) {
for (let i = 0; i < args.length; i++) {
if (args[i] === MODULAR_DEPRECATION_ARG) {
return false;
return;
}
}
return true;
let message =
'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) {
message += ` Please use \`${replacementMethodName}\` instead.`;
}

// eslint-disable-next-line no-console
console.warn(message);
}
30 changes: 5 additions & 25 deletions packages/app/lib/internal/registry/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
isIOS,
isOther,
isNull,
isNotModularCall,
warnIfNotModularCall,
isObject,
isFunction,
isString,
Expand Down Expand Up @@ -85,12 +85,7 @@
* @param name
*/
export function getApp(name = DEFAULT_APP_NAME) {
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 `getApp()` instead.',
);
}
warnIfNotModularCall(arguments, 'getApp()');
if (!initializedNativeApps) {
initializeNativeApps();
}
Expand All @@ -107,12 +102,7 @@
* Gets all app instances, used for `firebase.apps`
*/
export function getApps() {
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 `getApps()` instead.',
);
}
warnIfNotModularCall(arguments, 'getApps()');
if (!initializedNativeApps) {
initializeNativeApps();
}
Expand All @@ -125,12 +115,7 @@
* @param configOrName
*/
export function initializeApp(options = {}, configOrName) {
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 `initializeApp()` instead.',
);
}
warnIfNotModularCall(arguments, 'initializeApp()');

Check warning on line 118 in packages/app/lib/internal/registry/app.js

View check run for this annotation

Codecov / codecov/patch

packages/app/lib/internal/registry/app.js#L118

Added line #L118 was not covered by tests
let appConfig = configOrName;

if (!isObject(configOrName) || isNull(configOrName)) {
Expand Down Expand Up @@ -219,12 +204,7 @@
}

export function setLogLevel(logLevel) {
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 `setLogLevel()` instead.',
);
}
warnIfNotModularCall(arguments, 'setLogLevel()');
if (!['error', 'warn', 'info', 'debug', 'verbose'].includes(logLevel)) {
throw new Error('LogLevel must be one of "error", "warn", "info", "debug", "verbose"');
}
Expand Down
Loading