Ensure InitializeCore is run before app code #4814
Merged
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
NickGerleman
approved these changes
May 6, 2020
acoates-ms
added
Backport to 0.62
AutoMerge
|
Hello @acoates-ms! Because this pull request has the Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 10 hours, a condition that will be fulfilled in about 8 hours 52 minutes. No worries though, I will be back when the time is right! 😉 p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
merged commit
NickGerleman
pushed a commit
to NickGerleman/react-native-windows
that referenced
this pull request
May 7, 2020
* Ensure InitializeCore is run before app code * Change files
jonthysell
added
the
Breaking Change
4 tasks
This pull request was closed.
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.
Fixes #4779
The reason fast refresh wasn't working is that fast refresh requires a custom implementation of require to be used within the bundle. That implementation of require expects a custom property 'Refresh' to be set on it. That variable is set by 'setupReactRefresh', which is one of the things that InitializeCore does.
The solution to allow metro to work on all platforms at once caused the default config of metro to not run InitializeCore before other modules.
This happened because the default react-native config of metro uses a full path to react-native's InitializeCore to tell metro to require that module before the main entry file of the apps. Metro silently ignores paths in getModulesRunBeforeMainModule that are not part of the bundle. Since we use InitializeCore from react-native-windows, the module specified in getModulesRunBeforeMainModule was just ignored.
The change here adds the path to InitializeCore from react-native, react-native-windows and react-native-macos to the config. That way which ever of those modules ends up in the bundle will be required before the apps main entry file.
All of this custom metro config will be replaced by react-native-community/cli#1115 which will fix the defaults in the CLI. (But hopefully we can work through all these kinds of issues before we get that merged)
Microsoft Reviewers: Open in CodeFlow