Checking for a version match between JS and native

[ide]

Many of the issues people run into - with quite a wide variety of symptoms - are caused by a mismatch between the JS and native code. There isn't a single reason for this mismatch, ex: it could be caused by stale Xcode/Android build data, forgetting to rebuild the native app after upgrading RN in node_modules, Watchman/Metro caching bugs, bundle caching on the client, and so on. Some kind of simple version check might help people discover these bugs more quickly with a more accurate error message.

Proposal

We add three new files: ReactNativeVersion.java, RCTVersion.h, and ReactNativeVersion.js. These files are marked as @generated and are written only by the release scripts, which already modify package.json, the Podspec, and a Gradle file.

// ReactNativeVersion.java
// @generated by scripts/bump-oss-version.js
public class ReactNativeVersion {
  public static final String VERSION = "0.47.0-rc.1";
  public static final int MAJOR = 0;
  public static final int MINOR = 47;
  public static final int PATCH = 0;
  public static final int PRERELEASE = "rc.1";
}

// RCTVersion.h
// @generated by scripts/bump-oss-version.js
#define REACT_NATIVE_VERSION @"0.47.0-rc.1"
#define REACT_NATIVE_VERSION_MAJOR 0
#define REACT_NATIVE_VERSION_MINOR 47
#define REACT_NATIVE_VERSION_PATCH 0
#define REACT_NATIVE_VERSION_PRERELEASE @"rc.1"

// ReactNativeVersion.js
// @generated by scripts/bump-oss-version.js
exports.version = '0.47.0-rc.1';
exports.major = 0;
exports.minor = 47;
exports.patch = 0;
exports.prerelease = 'rc.1'

These files are checked into the repo for simplicity. In addition to these autogenerated files, there would be small native modules that export these versions from native to JS (using constantsToExport, it should be fast since there's not much real work being done). JS would then compare ReactNativeVersion.version to NativeModules.Version.version and call console.error if they don't match (for some definition of match -- maybe it's OK if 0.46.1 JS runs on 0.46.0 native code, don't want this warning to be annoying).

[douglowder]

I like this idea a lot! Maybe version should be an object instead of a string... what about

// ReactNativeVersion.js
// @generated by scripts/bump-oss-version.js
exports.version = {
  'major' : '0.47',
  'minor' : '0',
  'build' : ''
};

Maybe console.error should only happen if there is a mismatch in the major string. Build could be something used by app developers who might be making customizations in RN code.

[jpshelley]

fwiw I've seen issues pop up when Native/JS libs are mismatched across minor versions as well @dlowder-salesforce (I think loading images had broke for me between 0.43.0 and 0.43.2 or something If I remember correctly). I like the object idea but would want minor version checked in the diff as well though.

[SimplGy]

This would be a huge help. What a great way to remove a whole class of confusing errors and replace with a simple, accurate message.

[rozele]

Do you guys have any proposals for how platform extensions play along with this? Unfortunately this proposal only covers iOS and Android (or any platform in this repo). For react-native-windows, we may be using JavaScript from this repo but native code from a separate repo, and this is yet another thing we have to keep in sync. It would be nice if this had hooks that accommodated for this (cc @axemclion).

I like that the basic implementation in #15518 only checks against the major/minor, because we tend to keep these in sync with react-native, but it would become very problematic if this check were to also include the build version (these do not have to be in sync). Also, is there a plan for updating this when we eventually start shipping major versions of react-native or is it "we'll cross that bridge when we get there"?

[ide]

If I understand correctly, the version check requires the JS to line up with the native code the same way the rest of React Native does. For react-native-windows, you'd do two things: make the native PlatformConstants module export a reactNativeVersion field, and override the ReactNativeVersion module to export the same value.

I think the approach above wouldn't be affected if RN were to check the build version since react-native-windows would have control over the version number it defines in both native and JS -- the only thing react-native-windows doesn't control is the checking mechanism in InitializeCore.js. We could also add a new module called VersionCheck.js that InitializeCore.js uses, and react-native-windows could override the former.

I haven't heard of a plan for major versions of RN but I don't think it materially changes things. There is a question of whether the native code from 1.1.0 would support JS code from 1.0.0 but we already have that same question today, just shifted over by one point (does native 0.1.1 support JS 0.1.0?). The current answer is "probably yes" hence the heuristic used in the current version check but no guarantees -- best to line up the numbers precisely.

[ide]

This version check is also out with 0.49 so I'm going to close this issue (for task-tracking, not to close off future conversation).

[rozele]

@ide I didn't word it very well. The UWP platform extension is composed of a native implementation, the core React Native JavaScript, and the platform specific JavaScript overrides. The current approach assumes a 1-to-1 mapping between native and core JS libraries. If I override the ReactNativeVersion module, I have no way of asserting that the core React Native JavaScript has the correct version, only that my platform specific JavaScript matches.

[dantman]

I presume it's possible for platform extensions to extend ReactNativeVersion.js rather than straight override it (ie: define a ReactNativeVersion.yourPlatform.js that requires the non-suffixed ReactNativeVersion.js provided by react-native and exports a modified version).

How about adding some semi-official semi-support for platform extensions. Not fully supporting them, but acknowledging they are part of the react-native ecosystem and providing a reasonable integration for them into the version handling.

For that in the version context, I think:

ReactNativeVersion.js should export in this format { major: number, minor: number, patch: number, prerelease: null, platform?: { major: number, minor: number, patch: number, prerelease: null } }.

On normal react-native both require('ReactNativeVersion').version.platform and NativeModules.PlatformConstants.reactNativeVersion.platform should be null. InitializeCore.js
should have a second section comparing the platform version if it's not null and making sure that one is null if the other is null.

Platform extensions will extend ReactNativeVersion.js and return the same major/minor/patch but export their own platform version.

In NativeModules.PlatformConstants.reactNativeVersion, the platform extension will define the major/minor/patch of the react-native version that their native code is coded against (ie: written to behave like) and under platform define their own version.

Then platform extensions have versions properly handled. The react-native version validates that the platform extension native code is coded to behave like the version the react-native js code is expecting. And the platform version validates that the platform js matches the version of platform native code running.

If tacking on a nested structure doesn't feel right, we could go straight into making it slightly more official and have a separate ReactNativeVersion.js and ReactNativePlatformVersion.js (react would return null from this) or ReactNativeVersion.js could export version and platform as separate exports.

[ide]

@rozele oh ok, I answered your first comment from the angle of "how can we not break react-native-windows?" rather than "how can react-native-windows do version checking?".

A simple and flexible approach that I suspect wouldn't need many upstream changes over time would be to add a new module called ReactNativeVersionCheck.js whose default implementation is:

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

exports.checkVersions = function checkVersions(): void {
  if (ReactNativeVersion ≠ PlatformConstants.reactNativeVersion) {
    throw new Error(...);
  }
}

InitializeCore.js would run:

ReactNativeVersionCheck.checkVersions();

This provides the same behavior as what's in 0.49 today. react-native-windows would provide a platform-specific override of ReactNativeVersionCheck.js where it could run whatever it wanted. The reason I like this is it potentially lets us do all three version checks: (RNW C#, RN Core JS), (RNW C#, RNW JS), and (RN Core JS, RNW JS) and RN Core would only have to try to keep ReactNativeVersionCheck.checkVersions() stable, for which we could add some unit tests.

[rozele]

Thanks @ide, I think that's a sound solution. Is this something you think we should patch into 0.49 or wait for 0.50?

One other question: I'm sure it's rare, but sometimes a version of react-native JS is compatible with a previous version of react-native binaries. Or even if it's not fully compatible, maybe the only part that is no longer compatible is no longer being used in my app. The scenario I'm watching out for is a CodePush user who acknowledges the risks of updating react-native JS only, but wants to get a new feature out that's available in the latest react-native JS, and has fully tested compatibility with the native modules they are using. It seems that this version check "feature" is preventing users who may want to do this.

[ide]

I think we'd make the change in 0.50 since react-native-windows can still implement PlatformConstants.reactNativeVersion in 0.49 and there's no regression compared to 0.48 and before.

For the CodePush scenario -- yes, this blocks that though I suspect it is uncommon and it's better to think of React Native as a single unit that's separate from app code, rather than drawing the boundaries of that unit around the RN JS + app code separately from the RN native code.

Regardless it'd be nice if we could provide an escape hatch somehow (e.g. by registering some custom hooks with RN before it fully initializes). For now we're erring on the side of soundness since mismatched JS and native has caused several non-obvious issues for people that could have been fixed quickly.

[axemclion]

Adding CodePush folks to this thread - @max-mironov and @sergey-akhalkov. Max/Sergey, could you also add Patrick and others who should be aware of this ?

[max-mironov]

Thanks guys! Adding @pniko , @buptkang, @ruslan-bikkinin for awareness. We'll also discuss this internally how to align it with CodePush well.

[ide]

Moved the version check into its own module (react-native-windows can override it and do whatever it wants in there) + added unit tests to make the interface more reliable in this PR: #16403

[allenhsu]

I noticed RCTVersion.h is not a public header, so I cannot import it in my project and check versions on my own. Why not making it public (adding to Copy Headers phase of React project and podspec accordingly)?