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

Stamp React Native Version Into C++ Code #28036

Closed
wants to merge 2 commits into from

Conversation

NickGerleman
Copy link
Contributor

@NickGerleman NickGerleman commented Feb 12, 2020

Summary

The PlatformConstants native module exposes the ability to query the React
Native version used to build native code. This is managed on iOS and Android by
a version bumping script, which replaces module code based on a template.

It is currently difficult to accurately determine this version for out-of-tree C++
platforms (I.e. React Native Windows). The version of upstream react-native we resolve to is ultimately
dependent on the version of react-native chosen a peer dependency, which is not
neccesarily constant given a build of react-native-windows.

We could try to hack around this, and make our native build try to reason about
the resolved pacakge for react-native using a lockfile, but a much cleaner
solution is to embed version into C++ code, similar to what is done for Android
and iOS. This change does that, adding a header with React Native version and
updating the build stamping script to write to it.

Usage sample:

  constants["reactNativeVersion"] = folly::dynamic::object();
  constants["reactNativeVersion"]["major"] = ReactNativeVersion.Major;
  constants["reactNativeVersion"]["minor"] = ReactNativeVersion.Minor;
  constants["reactNativeVersion"]["patch"] = ReactNativeVersion.Patch;

Changelog

[General] [Added] - Stamp React Native Version Into C++ Code

Test Plan

Validated that the bumping script will accurately update the header, can compile under both MSVC and Clang.

The PlatformConstants native module exposes the ability to query the React
Native version used to build native code. This is managed on iOS and Android by
a version bumping script, which replaces module code based on a template.

It is currently difficult to accurately determine this version for out-of-tree
platforms. The version of upstream react-native we resolve to is ultimately
dependent on the version of react-native chosen a peer dependency, which is not
neccesarily constant given a build of react-native-windows.

We could try to hack around this, and make our native build try to reason about
the resolved pacakge for react-native using a lockfile, but a much cleaner
solution is to embed version into C++ code, similar to what is done for Android
and iOS. This change does that, adding a header with React Native version and
updating the build stamping script to write to it.

Validated that the bumping script will accurately update the header, can
compile under both MSVC and Clang.

Usage sample:

```c++
  constants["reactNativeVersion"] = folly::dynamic::object();
  constants["reactNativeVersion"]["major"] = ReactNativeVersion.Major;
  constants["reactNativeVersion"]["minor"] = ReactNativeVersion.Minor;
  constants["reactNativeVersion"]["patch"] = ReactNativeVersion.Patch;
```
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Microsoft Partner: Microsoft Partner labels Feb 12, 2020
@react-native-bot react-native-bot added the Type: Enhancement A new feature or enhancement of an existing feature. label Feb 12, 2020
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@hramos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@NickGerleman
Copy link
Contributor Author

@hramos do you know what failed for Facebook Internal CI?

@hramos
Copy link
Contributor

hramos commented Feb 27, 2020

@NickGerleman this patch no longer applies on top of master. Looks like ReactCommon/cxxreact/ReactNativeVersion.h no longer exists.

@hramos
Copy link
Contributor

hramos commented Feb 27, 2020

Oh, my bad, that file is added in this PR. In any case, we failed to apply the patch internally, usually it's either due to a file that moved or because the PR is based off an older revision and the process timed out (can happen if the base commit is over 3 months old). I'm applying the changes manually on top of HEAD and will push them out. Hold tight.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @NickGerleman in 427ba35.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Feb 27, 2020
osdnk pushed a commit to osdnk/react-native that referenced this pull request Mar 9, 2020
Summary:
The PlatformConstants native module exposes the ability to query the React
Native version used to build native code. This is managed on iOS and Android by
a version bumping script, which replaces module code based on a template.

It is currently difficult to accurately determine this version for out-of-tree C++
platforms (I.e. React Native Windows). The version of upstream react-native we resolve to is ultimately
dependent on the version of react-native chosen a peer dependency, which is not
neccesarily constant given a build of react-native-windows.

We could try to hack around this, and make our native build try to reason about
the resolved pacakge for react-native using a lockfile, but a much cleaner
solution is to embed version into C++ code, similar to what is done for Android
and iOS. This change does that, adding a header with React Native version and
updating the build stamping script to write to it.

Usage sample:

```c++
  constants["reactNativeVersion"] = folly::dynamic::object();
  constants["reactNativeVersion"]["major"] = ReactNativeVersion.Major;
  constants["reactNativeVersion"]["minor"] = ReactNativeVersion.Minor;
  constants["reactNativeVersion"]["patch"] = ReactNativeVersion.Patch;
```
## Changelog

[General] [Added] - Stamp React Native Version Into C++ Code
Pull Request resolved: facebook#28036

Test Plan: Validated that the bumping script will accurately update the header, can compile under both MSVC and Clang.

Differential Revision: D19865992

Pulled By: hramos

fbshipit-source-id: 9e0b8e9519015bb62c60b9935a234cd367a1926a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. p: Microsoft Partner: Microsoft Partner Type: Enhancement A new feature or enhancement of an existing feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants