[RN] Remove __EXPERIMENTAL__ from enableOwnerStacks, enableUseDeferredValueInitialArg #30322
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
This was referenced Jul 11, 2024
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This was referenced Jul 12, 2024
poteto
added a commit
that referenced
this pull request
Jul 12, 2024
In www, the experimental versions get a .modern.js or .classic.js prefix and get copied into the same folder. In RN, they don't seem to have .modern.js and .classic.js versions so they end up getting the same name. sebmarkbage's theory is that what happens is that they then override the file that was already there. So depending on if experimental or stable build finishes first you get a different version at the end. It doesn't make sense to use `__EXPERIMENTAL__` for flags in native-fb since there's no modern/classic split there. So that flag should just be hardcoded to true or false and then it doesn't matter which one finishes first. We don't support experimental builds in OSS RN neither so the same thing could happen with [`enableOwnerStacks`](https://github.com/facebook/react/blob/5dcf3ca8d45a276a8b4cee0cedd234967661ca35/packages/shared/forks/ReactFeatureFlags.native-oss.js#L60). You can see that the build errors in the previous PR but passes after these flag changes. ghstack-source-id: da04295a8f6a7c080d8ed3ebbb4ca5527eabe0e6 Pull Request resolved: #30322
facebook-github-bot
added
the
React Core Team
jackpope
reviewed
Jul 12, 2024
| @@ -69,7 +69,7 @@ export const enableTaint = true; | |||
| export const enableTransitionTracing = false; | |||
| export const enableTrustedTypesIntegration = false; | |||
| export const enableUpdaterTracking = false; | |||
| export const enableUseDeferredValueInitialArg = __EXPERIMENTAL__; | |||
| export const enableUseDeferredValueInitialArg = false; | |||
There was a problem hiding this comment.
I'm not sure why this got set to experimental in #29903 but seems like it would be pretty safe to switch to true seeing as that is the value for native-fb and native-oss now. cc @yungsters
poteto
added a commit
that referenced
this pull request
Jul 12, 2024
In www, the experimental versions get a .modern.js or .classic.js prefix and get copied into the same folder. In RN, they don't seem to have .modern.js and .classic.js versions so they end up getting the same name. sebmarkbage's theory is that what happens is that they then override the file that was already there. So depending on if experimental or stable build finishes first you get a different version at the end. It doesn't make sense to use `__EXPERIMENTAL__` for flags in native-fb since there's no modern/classic split there. So that flag should just be hardcoded to true or false and then it doesn't matter which one finishes first. We don't support experimental builds in OSS RN neither so the same thing could happen with [`enableOwnerStacks`](https://github.com/facebook/react/blob/5dcf3ca8d45a276a8b4cee0cedd234967661ca35/packages/shared/forks/ReactFeatureFlags.native-oss.js#L60). You can see that the build errors in the previous PR but passes after these flag changes. ghstack-source-id: d10f37bcea0e485fdb4f136370c179999badd560 Pull Request resolved: #30322
poteto
changed the title
github-actions bot
pushed a commit
that referenced
this pull request
Jul 12, 2024
In www, the experimental versions get a .modern.js or .classic.js prefix and get copied into the same folder. In RN, they don't seem to have .modern.js and .classic.js versions so they end up getting the same name. sebmarkbage's theory is that what happens is that they then override the file that was already there. So depending on if experimental or stable build finishes first you get a different version at the end. It doesn't make sense to use `__EXPERIMENTAL__` for flags in native-fb since there's no modern/classic split there. So that flag should just be hardcoded to true or false and then it doesn't matter which one finishes first. We don't support experimental builds in OSS RN neither so the same thing could happen with [`enableOwnerStacks`](https://github.com/facebook/react/blob/5dcf3ca8d45a276a8b4cee0cedd234967661ca35/packages/shared/forks/ReactFeatureFlags.native-oss.js#L60). You can see that the build errors in the previous PR but passes after these flag changes. ghstack-source-id: d10f37bcea0e485fdb4f136370c179999badd560 Pull Request resolved: #30322 DiffTrain build for commit ff3f1fa.
felixshiftellecon
added a commit
to felixshiftellecon/react
that referenced
this pull request
Jul 24, 2024
In www, the experimental versions get a .modern.js or .classic.js prefix and get copied into the same folder. In RN, they don't seem to have .modern.js and .classic.js versions so they end up getting the same name. sebmarkbage's theory is that what happens is that they then override the file that was already there. So depending on if experimental or stable build finishes first you get a different version at the end. It doesn't make sense to use `__EXPERIMENTAL__` for flags in native-fb since there's no modern/classic split there. So that flag should just be hardcoded to true or false and then it doesn't matter which one finishes first. We don't support experimental builds in OSS RN neither so the same thing could happen with [`enableOwnerStacks`](https://github.com/facebook/react/blob/5dcf3ca8d45a276a8b4cee0cedd234967661ca35/packages/shared/forks/ReactFeatureFlags.native-oss.js#L60). You can see that the build errors in the previous PR but passes after these flag changes. ghstack-source-id: d10f37bcea0e485fdb4f136370c179999badd560 Pull Request resolved: facebook#30322
This was referenced Jul 25, 2024
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.
Stack from ghstack (oldest at bottom):
In www, the experimental versions get a .modern.js or .classic.js prefix
and get copied into the same folder. In RN, they don't seem to have
.modern.js and .classic.js versions so they end up getting the same
name.
@sebmarkbage's theory is that what happens is that they then override
the file that was already there. So depending on if experimental or
stable build finishes first you get a different version at the end.
It doesn't make sense to use
__EXPERIMENTAL__for flags in native-fbsince there's no modern/classic split there. So that flag should just be
hardcoded to true or false and then it doesn't matter which one finishes
first.
We don't support experimental builds in OSS RN neither so the same thing
could happen with
enableOwnerStacks.You can see that the build errors in the previous PR but passes after
these flag changes.
Test plan