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

[general][enhancement] Move JS-native version check to its own module + unit tests + prefix Obj-C macro w/RCT #16403

Closed
wants to merge 1 commit into from
Closed
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
22 changes: 3 additions & 19 deletions Libraries/Core/InitializeCore.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,25 +116,9 @@ if (!global.__fbDisableExceptionsManager) {
ErrorUtils.setGlobalHandler(handleError);
}

const {PlatformConstants} = require('NativeModules');
if (PlatformConstants) {
const formatVersion = version =>
`${version.major}.${version.minor}.${version.patch}` +
(version.prerelease !== null ? `-${version.prerelease}` : '');

const ReactNativeVersion = require('ReactNativeVersion');
const nativeVersion = PlatformConstants.reactNativeVersion;
if (ReactNativeVersion.version.major !== nativeVersion.major ||
ReactNativeVersion.version.minor !== nativeVersion.minor) {
throw new Error(
`React Native version mismatch.\n\nJavaScript version: ${formatVersion(ReactNativeVersion.version)}\n` +
`Native version: ${formatVersion(nativeVersion)}\n\n` +
'Make sure that you have rebuilt the native code. If the problem persists ' +
'try clearing the watchman and packager caches with `watchman watch-del-all ' +
'&& react-native start --reset-cache`.'
);
}
}
// Check for compatibility between the JS and native code
const ReactNativeVersionCheck = require('ReactNativeVersionCheck');
ReactNativeVersionCheck.checkVersions();

// Set up collections
const _shouldPolyfillCollection = require('_shouldPolyfillES6Collection');
Expand Down
54 changes: 54 additions & 0 deletions Libraries/Core/ReactNativeVersionCheck.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/**
* Copyright (c) 2017-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @providesModule ReactNativeVersionCheck
* @flow
Copy link
Contributor

@janicduplessis janicduplessis Oct 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can add @format pragma to tell tools this file uses prettier.

* @format
*/
'use strict';

const {PlatformConstants} = require('NativeModules');
const ReactNativeVersion = require('ReactNativeVersion');

/**
* Checks that the version of this React Native JS is compatible with the native
* code, throwing an error if it isn't.
*
* The existence of this module is part of the public interface of React Native
* even though it is used only internally within React Native. React Native
* implementations for other platforms (ex: Windows) may override this module
* and rely on its existence as a separate module.
*/
exports.checkVersions = function checkVersions(): void {
if (!PlatformConstants) {
return;
}

const nativeVersion = PlatformConstants.reactNativeVersion;
if (
ReactNativeVersion.version.major !== nativeVersion.major ||
ReactNativeVersion.version.minor !== nativeVersion.minor
) {
throw new Error(
`React Native version mismatch.\n\nJavaScript version: ${_formatVersion(
ReactNativeVersion.version,
)}\n` +
`Native version: ${_formatVersion(nativeVersion)}\n\n` +
'Make sure that you have rebuilt the native code. If the problem ' +
'persists try clearing the Watchman and packager caches with ' +
'`watchman watch-del-all && react-native start --reset-cache`.',
);
}
};

function _formatVersion(version): string {
return (
`${version.major}.${version.minor}.${version.patch}` +
(version.prerelease !== null ? `-${version.prerelease}` : '')
);
}
117 changes: 117 additions & 0 deletions Libraries/Core/__tests__/ReactNativeVersionCheck-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
/**
* Copyright (c) 2017-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same @format here

*
* @format
*/
'use strict';

describe('checkVersion', () => {
describe('in development', () => {
_setDevelopmentModeForTests(true);
_defineCheckVersionTests();
});

describe('in production', () => {
_setDevelopmentModeForTests(false);
_defineCheckVersionTests();
});
});

function _setDevelopmentModeForTests(dev) {
let originalDev;

beforeAll(() => {
originalDev = global.__DEV__;
global.__DEV__ = dev;
});

afterAll(() => {
global.__DEV__ = originalDev;
});
}

function _defineCheckVersionTests() {
afterEach(() => {
jest.resetModules();
});

it('passes when all the versions are zero', () => {
jest.dontMock('ReactNativeVersion');
_mockNativeVersion(0, 0, 0);

const ReactNativeVersion = require('ReactNativeVersion');
const ReactNativeVersionCheck = require('ReactNativeVersionCheck');
expect(ReactNativeVersion).toMatchObject({
version: {major: 0, minor: 0, patch: 0, prerelease: null},
});
expect(() => ReactNativeVersionCheck.checkVersions()).not.toThrow();
});

it('passes when the minor matches when the major is zero', () => {
_mockJsVersion(0, 1, 0);
_mockNativeVersion(0, 1, 0);

const ReactNativeVersionCheck = require('ReactNativeVersionCheck');
expect(() => ReactNativeVersionCheck.checkVersions()).not.toThrow();
});

it("throws when the minor doesn't match when the major is zero", () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test for major don't match too so we cover all cases

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, will do. We can't add a test for checking that the major matches since we don't have that logic right now, we'll need to add it later if/when we shift RN versions over by one point (RN 51, RN 52, etc...).

_mockJsVersion(0, 1, 0);
_mockNativeVersion(0, 2, 0);

const ReactNativeVersionCheck = require('ReactNativeVersionCheck');
expect(() => ReactNativeVersionCheck.checkVersions()).toThrowError(
/React Native version mismatch/,
);
});

it("throws when the major doesn't match", () => {
_mockJsVersion(1, 0, 0);
_mockNativeVersion(2, 0, 0);

const ReactNativeVersionCheck = require('ReactNativeVersionCheck');
expect(() => ReactNativeVersionCheck.checkVersions()).toThrowError(
/React Native version mismatch/,
);
});

it("doesn't throw if the patch doesn't match", () => {
_mockJsVersion(0, 1, 0);
_mockNativeVersion(0, 1, 2);

const ReactNativeVersionCheck = require('ReactNativeVersionCheck');
expect(() => ReactNativeVersionCheck.checkVersions()).not.toThrow();
});

it("doesn't throw if the prerelease doesn't match", () => {
_mockJsVersion(0, 1, 0, 'beta.0');
_mockNativeVersion(0, 1, 0, 'alpha.1');

const ReactNativeVersionCheck = require('ReactNativeVersionCheck');
expect(() => ReactNativeVersionCheck.checkVersions()).not.toThrow();
});
}

function _mockJsVersion(major = 0, minor = 0, patch = 0, prerelease = null) {
jest.doMock('ReactNativeVersion', () => ({
version: {major, minor, patch, prerelease},
}));
}

function _mockNativeVersion(
major = 0,
minor = 0,
patch = 0,
prerelease = null,
) {
jest.doMock('NativeModules', () => ({
PlatformConstants: {
reactNativeVersion: {major, minor, patch, prerelease},
},
}));
}
2 changes: 1 addition & 1 deletion React/Base/RCTPlatform.m
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ + (BOOL)requiresMainQueueSetup
@"systemName": [device systemName],
@"interfaceIdiom": interfaceIdiom([device userInterfaceIdiom]),
@"isTesting": @(RCTRunningInTestEnvironment()),
@"reactNativeVersion": REACT_NATIVE_VERSION,
@"reactNativeVersion": RCT_REACT_NATIVE_VERSION,
};
}

Expand Down
2 changes: 1 addition & 1 deletion React/Base/RCTVersion.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
* of patent rights can be found in the PATENTS file in the same directory.
*/

#define REACT_NATIVE_VERSION @{ \
#define RCT_REACT_NATIVE_VERSION @{ \
@"major": @(0), \
@"minor": @(0), \
@"patch": @(0), \
Expand Down
2 changes: 1 addition & 1 deletion scripts/versiontemplates/RCTVersion.h.template
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
* of patent rights can be found in the PATENTS file in the same directory.
*/

#define REACT_NATIVE_VERSION @{ \
#define RCT_REACT_NATIVE_VERSION @{ \
@"major": ${major}, \
@"minor": ${minor}, \
@"patch": ${patch}, \
Expand Down