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

chore(core): deprecation warning for v8 API ahead of future major release #8132

Merged
merged 6 commits into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from 5 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
59 changes: 56 additions & 3 deletions packages/app/__tests__/app.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { describe, expect, it } from '@jest/globals';

import {
import { describe, expect, it, jest } from '@jest/globals';
import { checkV9Deprecation } from '../lib/common/unitTestUtils';
import firebase, {
mikehardy marked this conversation as resolved.
Show resolved Hide resolved
deleteApp,
registerVersion,
onLog,
Expand Down Expand Up @@ -40,4 +40,57 @@ describe('App', function () {
expect(setLogLevel).toBeDefined();
});
});

describe('`console.warn` only called for non-modular API', function () {
it('deleteApp', function () {
// this test has a slightly special setup
// @ts-ignore test
jest.spyOn(getApp(), '_deleteApp').mockImplementation(() => Promise.resolve(null));
checkV9Deprecation(
() => {}, // no modular replacement
() => getApp().delete(), // modular getApp(), then non-modular to check
);
});

it('getApps', function () {
checkV9Deprecation(
() => getApps(),
() => firebase.apps,
);
});

it('getApp', function () {
checkV9Deprecation(
() => getApp(),
() => firebase.app(),
);
});

it('setLogLevel', function () {
checkV9Deprecation(
() => setLogLevel('debug'),
() => firebase.setLogLevel('debug'),
);
});

it('FirebaseApp.toString()', function () {
checkV9Deprecation(
() => {}, // no modular replacement
() => getApp().toString(), // modular getApp(), then non-modular to check
);
});

it('FirebaseApp.extendApp()', function () {
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;
},
);
});
});
});
7 changes: 6 additions & 1 deletion 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 { warnIfNotModularCall } from '@react-native-firebase/app/lib/common';
import { getAppModule } from './internal/registry/nativeModule';

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

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

delete() {
warnIfNotModularCall(arguments, 'deleteApp()');
this._checkDestroyed();
return this._deleteApp();
}

toString() {
// this method has no modular alternative, send true for param 'noAlternative'
warnIfNotModularCall(arguments, '', true);
return this.name;
}
}
20 changes: 20 additions & 0 deletions packages/app/lib/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,23 @@ export function tryJSONStringify(data) {
return null;
}
}

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

export function warnIfNotModularCall(args, replacementMethodName, noAlternative) {
for (let i = 0; i < args.length; i++) {
if (args[i] === MODULAR_DEPRECATION_ARG) {
return;
}
}
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);
}
10 changes: 10 additions & 0 deletions packages/app/lib/common/unitTestUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { expect, jest } from '@jest/globals';

export 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();
};
5 changes: 5 additions & 0 deletions packages/app/lib/internal/registry/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
isIOS,
isOther,
isNull,
warnIfNotModularCall,
isObject,
isFunction,
isString,
Expand Down Expand Up @@ -84,6 +85,7 @@
* @param name
*/
export function getApp(name = DEFAULT_APP_NAME) {
warnIfNotModularCall(arguments, 'getApp()');
if (!initializedNativeApps) {
initializeNativeApps();
}
Expand All @@ -100,6 +102,7 @@
* Gets all app instances, used for `firebase.apps`
*/
export function getApps() {
warnIfNotModularCall(arguments, 'getApps()');
if (!initializedNativeApps) {
initializeNativeApps();
}
Expand All @@ -112,6 +115,7 @@
* @param configOrName
*/
export function initializeApp(options = {}, configOrName) {
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 @@ -200,6 +204,7 @@
}

export function setLogLevel(logLevel) {
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
14 changes: 9 additions & 5 deletions packages/app/lib/modular/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { MODULAR_DEPRECATION_ARG } from '@react-native-firebase/app/lib/common';
/* eslint-disable @typescript-eslint/no-unused-vars */
import {
deleteApp as deleteAppCompat,
Expand All @@ -6,6 +7,7 @@
initializeApp as initializeAppCompat,
setLogLevel as setLogLevelCompat,
} from '../internal';
import sdkVersion from '../version';

/**
* @typedef {import('..').ReactNativeFirebase.FirebaseApp} FirebaseApp
Expand All @@ -19,7 +21,7 @@
* @returns {Promise<void>}
*/
export function deleteApp(app) {
return deleteAppCompat(app.name, app._nativeInitialized);
return deleteAppCompat.call(null, app.name, app._nativeInitialized, MODULAR_DEPRECATION_ARG);

Check warning on line 24 in packages/app/lib/modular/index.js

View check run for this annotation

Codecov / codecov/patch

packages/app/lib/modular/index.js#L24

Added line #L24 was not covered by tests
}

/**
Expand Down Expand Up @@ -48,7 +50,7 @@
* @returns {FirebaseApp[]} - An array of all initialized Firebase apps.
*/
export function getApps() {
return getAppsCompat();
return getAppsCompat.call(null, MODULAR_DEPRECATION_ARG);
}

/**
Expand All @@ -58,7 +60,7 @@
* @returns {FirebaseApp} - The initialized Firebase app.
*/
export function initializeApp(options, name) {
return initializeAppCompat(options, name);
return initializeAppCompat.call(null, options, name, MODULAR_DEPRECATION_ARG);

Check warning on line 63 in packages/app/lib/modular/index.js

View check run for this annotation

Codecov / codecov/patch

packages/app/lib/modular/index.js#L63

Added line #L63 was not covered by tests
}

/**
Expand All @@ -67,7 +69,7 @@
* @returns {FirebaseApp} - The requested Firebase app instance.
*/
export function getApp(name) {
return getAppCompat(name);
return getAppCompat.call(null, name, MODULAR_DEPRECATION_ARG);
}

/**
Expand All @@ -76,5 +78,7 @@
* @returns {void}
*/
export function setLogLevel(logLevel) {
return setLogLevelCompat(logLevel);
return setLogLevelCompat.call(null, logLevel, MODULAR_DEPRECATION_ARG);
}

export const SDK_VERSION = sdkVersion;
11 changes: 7 additions & 4 deletions packages/crashlytics/e2e/crashlytics.e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading