Unify usage of USE_HERMES flag #41625
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D51549284 |
Base commit: ef9c164 |
cipolleschi
force-pushed
the
export-D51549284
branch
from
cd86f02 to
1afe7c2
Compare
cipolleschi
pushed a commit
to cipolleschi/react-native
that referenced
this pull request
Nov 23, 2023
Summary: To tell React Native whether we are building with hermes or not on iOS, we were using 2 different build time flags: - USE_HERMES - RCT_USE_HERMES The first was widely used by the OSS use case, while the latter was set internally. Worse than that, their default values were the opposite and we were never setting the RCT_USE_HERMES explicitly with Cocoapods, while there was some piece of code that was trying to "smartly" detect whether Hermes was included or not. This change unifies the behavior, removing the "smartness" in favor od a declarative approach. ## Changelog: [Internal] - Unify the USE_HERMES flags Differential Revision: D51549284
|
This pull request was exported from Phabricator. Differential Revision: D51549284 |
cipolleschi
pushed a commit
to cipolleschi/react-native
that referenced
this pull request
Nov 24, 2023
Summary: To tell React Native whether we are building with hermes or not on iOS, we were using 2 different build time flags: - USE_HERMES - RCT_USE_HERMES The first was widely used by the OSS use case, while the latter was set internally. Worse than that, their default values were the opposite and we were never setting the RCT_USE_HERMES explicitly with Cocoapods, while there was some piece of code that was trying to "smartly" detect whether Hermes was included or not. This change unifies the behavior, removing the "smartness" in favor od a declarative approach. ## Changelog: [Internal] - Unify the USE_HERMES flags Differential Revision: D51549284
cipolleschi
force-pushed
the
export-D51549284
branch
from
1afe7c2 to
911aa72
Compare
|
This pull request was exported from Phabricator. Differential Revision: D51549284 |
cipolleschi
pushed a commit
to cipolleschi/react-native
that referenced
this pull request
Nov 24, 2023
Summary: To tell React Native whether we are building with hermes or not on iOS, we were using 2 different build time flags: - USE_HERMES - RCT_USE_HERMES The first was widely used by the OSS use case, while the latter was set internally. Worse than that, their default values were the opposite and we were never setting the RCT_USE_HERMES explicitly with Cocoapods, while there was some piece of code that was trying to "smartly" detect whether Hermes was included or not. This change unifies the behavior, removing the "smartness" in favor od a declarative approach. ## Changelog: [Internal] - Unify the USE_HERMES flags Differential Revision: D51549284
cipolleschi
force-pushed
the
export-D51549284
branch
from
911aa72 to
6f0ecf3
Compare
|
This pull request was exported from Phabricator. Differential Revision: D51549284 |
1 similar comment
|
This pull request was exported from Phabricator. Differential Revision: D51549284 |
cipolleschi
pushed a commit
to cipolleschi/react-native
that referenced
this pull request
Nov 24, 2023
Summary: To tell React Native whether we are building with hermes or not on iOS, we were using 2 different build time flags: - USE_HERMES - RCT_USE_HERMES The first was widely used by the OSS use case, while the latter was set internally. Worse than that, their default values were the opposite and we were never setting the RCT_USE_HERMES explicitly with Cocoapods, while there was some piece of code that was trying to "smartly" detect whether Hermes was included or not. This change unifies the behavior, removing the "smartness" in favor od a declarative approach. ## Changelog: [Internal] - Unify the USE_HERMES flags Differential Revision: D51549284
cipolleschi
force-pushed
the
export-D51549284
branch
from
6f0ecf3 to
08bc362
Compare
cipolleschi
pushed a commit
to cipolleschi/react-native
that referenced
this pull request
Nov 27, 2023
Summary: To tell React Native whether we are building with hermes or not on iOS, we were using 2 different build time flags: - USE_HERMES - RCT_USE_HERMES The first was widely used by the OSS use case, while the latter was set internally. Worse than that, their default values were the opposite and we were never setting the RCT_USE_HERMES explicitly with Cocoapods, while there was some piece of code that was trying to "smartly" detect whether Hermes was included or not. This change unifies the behavior, removing the "smartness" in favor od a declarative approach. ## Changelog: [Internal] - Unify the USE_HERMES flags Reviewed By: christophpurrer Differential Revision: D51549284
cipolleschi
force-pushed
the
export-D51549284
branch
from
08bc362 to
310e55d
Compare
|
This pull request was exported from Phabricator. Differential Revision: D51549284 |
cipolleschi
pushed a commit
to cipolleschi/react-native
that referenced
this pull request
Nov 27, 2023
Summary: To tell React Native whether we are building with hermes or not on iOS, we were using 2 different build time flags: - USE_HERMES - RCT_USE_HERMES The first was widely used by the OSS use case, while the latter was set internally. Worse than that, their default values were the opposite and we were never setting the RCT_USE_HERMES explicitly with Cocoapods, while there was some piece of code that was trying to "smartly" detect whether Hermes was included or not. This change unifies the behavior, removing the "smartness" in favor od a declarative approach. ## Changelog: [Internal] - Unify the USE_HERMES flags Reviewed By: christophpurrer Differential Revision: D51549284
cipolleschi
force-pushed
the
export-D51549284
branch
from
310e55d to
20d08bf
Compare
|
This pull request was exported from Phabricator. Differential Revision: D51549284 |
cipolleschi
pushed a commit
to cipolleschi/react-native
that referenced
this pull request
Nov 27, 2023
Summary: To tell React Native whether we are building with hermes or not on iOS, we were using 2 different build time flags: - USE_HERMES - RCT_USE_HERMES The first was widely used by the OSS use case, while the latter was set internally. Worse than that, their default values were the opposite and we were never setting the RCT_USE_HERMES explicitly with Cocoapods, while there was some piece of code that was trying to "smartly" detect whether Hermes was included or not. This change unifies the behavior, removing the "smartness" in favor od a declarative approach. ## Changelog: [Internal] - Unify the USE_HERMES flags Reviewed By: christophpurrer Differential Revision: D51549284
cipolleschi
force-pushed
the
export-D51549284
branch
from
20d08bf to
7f1ed53
Compare
|
This pull request was exported from Phabricator. Differential Revision: D51549284 |
cipolleschi
pushed a commit
to cipolleschi/react-native
that referenced
this pull request
Nov 27, 2023
Summary: To tell React Native whether we are building with hermes or not on iOS, we were using 2 different build time flags: - USE_HERMES - RCT_USE_HERMES The first was widely used by the OSS use case, while the latter was set internally. Worse than that, their default values were the opposite and we were never setting the RCT_USE_HERMES explicitly with Cocoapods, while there was some piece of code that was trying to "smartly" detect whether Hermes was included or not. This change unifies the behavior, removing the "smartness" in favor od a declarative approach. ## Changelog: [Internal] - Unify the USE_HERMES flags Reviewed By: christophpurrer Differential Revision: D51549284
cipolleschi
force-pushed
the
export-D51549284
branch
from
7f1ed53 to
faaccfb
Compare
|
This pull request was exported from Phabricator. Differential Revision: D51549284 |
cipolleschi
pushed a commit
to cipolleschi/react-native
that referenced
this pull request
Nov 27, 2023
Summary: To tell React Native whether we are building with hermes or not on iOS, we were using 2 different build time flags: - USE_HERMES - RCT_USE_HERMES The first was widely used by the OSS use case, while the latter was set internally. Worse than that, their default values were the opposite and we were never setting the RCT_USE_HERMES explicitly with Cocoapods, while there was some piece of code that was trying to "smartly" detect whether Hermes was included or not. This change unifies the behavior, removing the "smartness" in favor od a declarative approach. ## Changelog: [Internal] - Unify the USE_HERMES flags Reviewed By: christophpurrer Differential Revision: D51549284
cipolleschi
force-pushed
the
export-D51549284
branch
from
faaccfb to
8557c76
Compare
|
This pull request was exported from Phabricator. Differential Revision: D51549284 |
cipolleschi
pushed a commit
to cipolleschi/react-native
that referenced
this pull request
Nov 27, 2023
Summary: To tell React Native whether we are building with hermes or not on iOS, we were using 2 different build time flags: - USE_HERMES - RCT_USE_HERMES The first was widely used by the OSS use case, while the latter was set internally. Worse than that, their default values were the opposite and we were never setting the RCT_USE_HERMES explicitly with Cocoapods, while there was some piece of code that was trying to "smartly" detect whether Hermes was included or not. This change unifies the behavior, removing the "smartness" in favor od a declarative approach. ## Changelog: [Internal] - Unify the USE_HERMES flags Reviewed By: christophpurrer Differential Revision: D51549284
cipolleschi
force-pushed
the
export-D51549284
branch
from
8557c76 to
9380f56
Compare
|
This pull request was exported from Phabricator. Differential Revision: D51549284 |
added 2 commits
November 27, 2023 06:23
…MES Flag" (facebook#41626) Summary: In the codebase, we never set the RCT_USE_HERMES flag. When we install the pods, we use the USE_HERMES flag and we set USE_HERMES as a build setting. So, the RCT_USE_HERMES flag will always be not set for the OSS. https://pxl.cl/3RRxr ## Changelog: [iOS][Fixed] - use the right USE_HERMES flag ## Facebook: This change was incorrect as in OSS we never set the RCT_USE_HERMES flag, while we actually set the USE_HERMES one. I will align the BUCK RNTester setup in the next diff of the stackog: Reviewed By: dmytrorykun Differential Revision: D51547810
Summary: To tell React Native whether we are building with hermes or not on iOS, we were using 2 different build time flags: - USE_HERMES - RCT_USE_HERMES The first was widely used by the OSS use case, while the latter was set internally. Worse than that, their default values were the opposite and we were never setting the RCT_USE_HERMES explicitly with Cocoapods, while there was some piece of code that was trying to "smartly" detect whether Hermes was included or not. This change unifies the behavior, removing the "smartness" in favor od a declarative approach. ## Changelog: [Internal] - Unify the USE_HERMES flags Reviewed By: christophpurrer Differential Revision: D51549284
cipolleschi
force-pushed
the
export-D51549284
branch
from
9380f56 to
0a66ea7
Compare
|
This pull request was exported from Phabricator. Differential Revision: D51549284 |
|
This pull request has been merged in 3a045b6. |
fkgozali
added a commit
to fkgozali/react-native
that referenced
this pull request
Jan 5, 2024
Summary: facebook#41625 cleaned up some usages, but there are some remaining ones. Changelog: [Fixed][iOS] Further cleaned up RCT_USE_HERMES Differential Revision: D52555309
Othinn
pushed a commit
to Othinn/react-native
that referenced
this pull request
Jan 9, 2024
Summary: Pull Request resolved: facebook#41625 To tell React Native whether we are building with hermes or not on iOS, we were using 2 different build time flags: - USE_HERMES - RCT_USE_HERMES The first was widely used by the OSS use case, while the latter was set internally. Worse than that, their default values were the opposite and we were never setting the RCT_USE_HERMES explicitly with Cocoapods, while there was some piece of code that was trying to "smartly" detect whether Hermes was included or not. This change unifies the behavior, removing the "smartness" in favor od a declarative approach. ## Changelog: [Internal] - Unify the USE_HERMES flags Reviewed By: christophpurrer Differential Revision: D51549284 fbshipit-source-id: 829ad361e185d5b4fa227605523af3a8e590e95c
Othinn
pushed a commit
to Othinn/react-native
that referenced
this pull request
Jan 9, 2024
…#42148) Summary: Pull Request resolved: facebook#42148 facebook#41625 cleaned up some usages, but there are some remaining ones. Changelog: [iOS][Fixed] Further cleaned up RCT_USE_HERMES Reviewed By: cipolleschi Differential Revision: D52555309 fbshipit-source-id: e19d10de339636eca2f4762a78cebb282d0427d9
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
To tell React Native whether we are building with hermes or not on iOS, we were using 2 different build time flags:
The first was widely used by the OSS use case, while the latter was set internally.
Worse than that, their default values were the opposite and we were never setting the RCT_USE_HERMES explicitly with Cocoapods, while there was some piece of code that was trying to "smartly" detect whether Hermes was included or not.
This change unifies the behavior, removing the "smartness" in favor od a declarative approach.
Changelog:
[Internal] - Unify the USE_HERMES flags
Differential Revision: D51549284