-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Integrate RN Nightly 10/31 #14152
Integrate RN Nightly 10/31 #14152
Conversation
@@ -53,7 +52,7 @@ FabicUIManagerProperty() noexcept { | |||
} | |||
|
|||
FabricUIManager::FabricUIManager() { | |||
facebook::react::CoreFeatures::enablePropIteratorSetter = true; | |||
facebook::react::ReactNativeFeatureFlags::enableCppPropsIteratorSetter(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this line, it is no longer going anything.
But we do need to override the value of this flag in our feature flags - in ReactNativeWindowsFeatureFlags in ReactHost.cpp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed - will we need to call the new override ReactNativeWindowsFeatureFlags::enableCppPropsIteratorSetter()
in the constructor, in place of ReactNativeFeatureFlags::enableCppPropsIteratorSetter()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need some // [Windows
comments and an integration bugs about getting these changes upstreamed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Yajur-Grover Correct me if I'm wrong but I think these changes are upstream code!
The issue for the file is here: #13172, there's no specific windows changes, we just used an older version of this file with the hope we can remove these forks when Hermes gets caught up to upstream
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes these changes are all from upstream - the only forked change is line 152, which already has the mentioned issue created.
Description
Integration
Commit Range: facebook/react-native@e7a3f47...3a01a0c
Notable PRs
Changelog
Should this change be included in the release notes: No
Microsoft Reviewers: Open in CodeFlow