Metro: Bundle command does not add InitializeCore to runBeforeMainModule

[Titozzz]

Bug report
I was tracking a react native bug, but since my fix is inside metro code, I figured I repost here:
https://github.com/facebook/react-native/issues/19827#issuecomment-404156181

What is the current behavior?
When using react-native bundle (the command that runs when you build a release), InitializeCore.js is not the first file that get required, but instead it is required later, sometime during the generation of the bridge method, randomly (which is why if you are lucky or not using Promise // setTimeout early you'll be fine)
iOS does not need all those polyfills which is why the issue is only detected on android in release mode.

My insight:
Metro does not add InitializeCore.js to the entry point when building the js bundle. I've logged here. The issue being that it takes options.runBeforeMainModule that defaults to [].
Maybe we could merge it with _this._opts.getModulesRunBeforeMainModule() which defaults to ['/node_modules/react-native/Libraries/Core/InitializeCore.js'].

I've tried to replace it with

runBeforeMainModule: [
            ...options.runBeforeMainModule,
            ..._this._opts.getModulesRunBeforeMainModule(),
          ],
and android was working in release 🎉

I'd like to get more insight from other people and feel free to ask any other question, thanks

To reproduce just create a new app with RN 0.56 and console.log(Promise) at the top of your index.js, you'll get Promise is undefined when android release.

RN version: 0.56.0 and related metro version.

[rodrigobdz]

The problem is also present in iOS. Your fix solves it.

- and android was working in release 🎉
+ and iOS and Android were working in release 🎉

[Titozzz]

Yeah I knew it was not bundled for either, but I did not thought iOS needed the polyfills and was crashing, so good catch @rodrigobdz 🎉

[rafeca]

Thanks for the report! @Titozzz your suggestion is correct, I'm going to prepare a fix for this

[danielgindi]

I think this might also cause this: https://github.com/facebook/react-native/issues/20171

[Titozzz]

@danielgindi Does my fix also addresses that issue?

[danielgindi]

@Titozzz Yes! I got to try it today and it works!

[rafeca]

Hey folks, https://github.com/facebook/metro/commit/64a5b1d9d40397899ca7eab69da62ee504ef4e54 has just landed with a fix for this. I'm going to cherry-pick this change to the 0.38.x branch and publish [email protected]

Sorry for the inconvenience

[rodrigobdz]

@rafeca It's great news that it is now fixed but the question here is: how can we prevent this from happening? No additional tests were added in 64a5b1d.

[rafeca]

@rodrigobdz we're currently working on a new configuration system for metro which will standardize the options object across the codebase and simplify the API boundary between react native and metro. As part of this initiative we will add some end to end tests to Metro that should be able to capture these issues.

To give some context, internally at FB we have quite custom logic for building the production bundles, so we don't use the logic that integrates react native with metro. We have our own end to end tests but they are testing the integration between Metro and our internal infra, so it's currently hard for us to detect these breakages (this'll change soon once we have the new API though).