React-native: Use metro-bundler from master

[cpojer]

At Facebook we use both react-native and metro-bundler from master. The open source version of react-native depends on a concrete version of metro from npm. We often break react-native when we change the module boundaries in a single diff for both repos internally at Facebook, and this is not ideal as it breaks the react-native repo and requires us to bump a Metro version.

Instead, I propose to:

• Change react-native to depend on github's version of Metro from master
• Compile Metro using babel-register and link it into node_modules/metro-bundler
• When making an RN release, publish a new version of Metro and lock that version into package.json

It's possible that we still break react-native master if the two repos diverge or the sync script for metro-bundler is delayed, but that shouldn't happen often and is the cause of another issue that can be fixed (or will fix itself with time). I think this will lead to fewer breakages from us and allow everyone to move faster :)

@brentvatne @grabbou know anyone who could work on this?

[skevy]

@cpojer what about working towards a world where metro-bundler, RN, and local-cli live in one Lerna/Yarn monorepo (like Jest?)? Then the coversioning can be done by a tool and not rely on shipit staying in sync + GitHub dependencies (which can be flaky). cc @ide

[cpojer]

I don't think that will work for us as we explicitly took Metro out of RN so that we can better monitor the repo. Metro has uses outside of RN and we'd prefer not to tie them together again. The Metro repo is also growing and imho too big for us to manage in one single GitHub repo across multiple teams at FB.

[skevy]

Ok, how about this: what if we brought local-cli out of the RN repo somehow and coversioned it with metro, as most metro changes that affect RN affect local cli, not RN JS or native code (of course that’s not always true, but bear with me)? I’ve tested this before — generally, there’s no reason why a newer version of the local cli + metro bundled can’t work with several versions of React native (just like webpack can work with all kinds of JS code, versions of Babel, etc). There may be certain api boundaries that exist today that make this somewhat difficult (such as unbundle + the RN bridge), but what if we focused on ironing those out and making those certain boundaries more stable?

Whatever the solution, generally I think we should move towards less coupling between the RN tooling and the core native/JS modules that make up RN and the RN release process. It would be great for us to move RN towards a world where there’s more of an “React Native core”, with less fluff and less unmaintained modules and it’s own release schedule and stability guarnatees. Separating out the cli and making clear boundaries there would help that effort, I think.

[cpojer]

Oh, this sounds great to me but right now we depend on the local-cli internally at FB which I'm aiming on fixing. I think my proposal would work well to get make the decoupling easier for us, and then we could remove the cli completely from the FB codebase and move it somewhere else where the community maintains it. Then it's up to you to keep the cli in sync with Metro. We are working on a Metro cli right now and I'll try to hook that into the local-cli but I expect the seams to be much clearer soon. Making this workflow better would help me in the meantime.

[grabbou]

I cannot speak much about internal FB process and how you guys handle dependencies internally. I agree with @skevy that we should work towards decoupling React Native from its dependencies (including Metro and, for example: local-cli).

Until that happens, I see no reason for not orchestrating Metro releases along React Native ones. According to what's been proposed, it would be a matter of few additional steps in the releasing guide. Since the process itself is async (you cut branch, wait for it to build, wait for tests), in the meantime, one could switch to metro-bundler and push it onto npm.

I believe that's something we have roughly discussed on Slack and would be happy to work towards making this happen as a part of my releases volunteering (that I really like, as someone being "infrastructure" guy).

[sophiebits]

I am mildly averse to this, if only because it will make bisecting in OSS very difficult since it will be hard to tell what metro version to be used. Can we change shipit to update a metro commit hash in this repo for each commit so they're actually tied together?

[cpojer]

@sophiebits: I think Metro and RN are sufficiently decoupled enough that no one version would break RN, unless we make changes to the module system in a way that RN doesn't expect, which normally doesn't happen. The biggest thing we don't use at FB that we conflict with is local-cli. In general though, the latest version of Metro should always work with the latest version of RN.

[sophiebits]

I'm talking about if I am trying to bisect a 6-months-old version of RN. It'll be frustrating if the version of metro is incompatible and I have to guess about which metro version would be correct.

[cpojer]

If you have a six month old version of RN, that should have a hardcoded version of Metro or a range of versions of Metro from the previous release up to that release, so I don't see how that would change. In general, the change I'm proposing doesn't change any of the versions in the final release and is simply focused on reducing breakage on master. I'm not sure how your concern is relevant to that, but maybe I'm not seeing it. However, I'm not opposed to make shipit hardcode the metro commit hash if that is possible, that actually sounds like a great idea.

[sophiebits]

Yes, it would have a range of Metro versions – some of which might not work. If it's just Metro, this is probably bearable; if it's 10 dependencies, it's definitely not. If we can copy over the commit hash then I'm supportive though. :)

[cpojer]

Got it, yeah I don't think that's ever an issue with Metro that we observed so far, but you are right it could apply to other things (like, React).

[cpojer]

Soo.. anybody who is excited about making this change? :D

[sophiebits]

cc @zpao how hard would this be?

[sophiebits]

I don't think that's ever an issue with Metro that we observed so far

I thought you said "We often break react-native when we change the module boundaries in a single diff for both repos internally at Facebook"? Maybe I'm misunderstanding.

[cpojer]

Ohh, sorry maybe I wasn't explaining well. Internally, react-native and metro-bundler are always on master and the two are always in sync. However, in open source, the react-native repo has "metro-bundler" locked to a specific (previous) version of Metro. When we change the type of a module in metro-bundler that react-native's local-cli depends on, react-native will have flow errors until we manually bump the version. We also don't notice this, because internally we make the change in one diff that syncs out to two repos, so everything is green until it hits the react-native repo. The master versions of react-native and metro-bundler should always be in sync, and they are at FB, but not here in open source.

[sophiebits]

OK, great. I think that is evidence in support of my claim that checking out an old version of RN wouldn't necessarily work with latest master of metro. (Unless you disproportionately make additive changes to metro?) It sounds like my suggestions solves both our problems/worries though.

[nihgwu]

sorry to break the topic, I'm using RN0.49RC with metro-bundler@^0.13.0, and I can't start my app because I'm using moment, and it always breaks here, but I don't get that error with previous RN versions, so I believe that's a issue from metro-bundler, but as RN has a lock version of it, unless we cherry-pick the right version with the patch, it will always break, I have to comment out that line every time I run yarn add

[cpojer]

@nihgwu this is not the right issue to discuss that. In the future there may be a way for you to pick any version of Metro together with React Native. In the meantime, you can use the selective versions resolution feature we built into Yarn and outlined in the 1.0 blogpost: https://code.facebook.com/posts/274518539716230/announcing-yarn-1-0/

[rigdern]

However, I'm not opposed to make shipit hardcode the metro commit hash if that is possible, that actually sounds like a great idea.

I think the relationship between Yoga and React Native is similar to what we want the relationship between Metro and React Native to be (anytime Yoga master is changed, React Native master begins consuming those changes in short order).

A difference is that Yoga's source code is copied directly into the RN repo whereas it sounds like you want RN to consume Metro as a reference.

It looks like each commit that happens in the Yoga repo has a corresponding commit in the RN repo (e.g. https://github.com/facebook/yoga/commit/d90914f3dcc353c113c3f40dafd973829269d66d and https://github.com/facebook/react-native/commit/67d0cc5c0858b465eec73fa4916b03522a2b9d0e). For Metro, I know you want to update hashes instead of copying code but maybe the Yoga team has some infrastructure you can reuse for syncing Metro and RN.

[ide]

This seems hard since you might have a diff that touches both RN and Metro and shipit is going to split the diff into two commits that need to be associated with each other (specifically RN needs to know about Metro). Fundamentally I see two options:

1. shipit has the notion of an inter-repo dependency graph and lands the Metro commit first and puts the Metro commit hash into the RN commit (by any mechanism, even a submodule) before landing the latter. This pushes the complexity into shipit.
2. shipit generates a token when it processes a diff and puts the same token in the Metro and RN commits. RN is then responsible for using that token to fetch the corresponding version of Metro. This pushes the complexity into RN.

[rigdern]

That's an interesting point about changes that affect RN and Metro. Yoga has the same problem. For example, 992e37c8bcdb13c89cb680e385a0fb344137d0fa is an import of a Yoga change but also affects RCTShadowView.m. I wonder if the Yoga team has a system for dealing with this or if they just import their Yoga commits into RN manually.

[sophiebits]

Internally at FB, Yoga and RN both live in a single repo that contains all our mobile apps. Changes can be made to both at once and then that commit gets synced out to both open-source repos.

[ide]

To add to sophiebits's comment, shipit can split up commits in fairly arbitrary ways. E.g. if an internal commit modifies Yoga, shipit can derive two external commits (one for facebook/react-native and one for facebook/yoga).

Maybe we should just do the same for Metro. If Metro and RN are going to be coupled in practice for the time being, then maybe we should group them together and do the same thing as Yoga. Making RN depend on a separate Metro repo doesn't actually decouple the projects if their code still needs to be tightly co-versioned.

[janicduplessis]

Maybe we could vendor in a copy of metro for now a bit like we do with the RN React renderer that lives in the react repo and is synced when needed. Ideally the process would be automated so if you commit something that affects both RN (local-cli) and metro it would build metro and copy the files in the RN repo. This would make sure there are no race condition possible. It would always work no matter if the metro diff or the RN diff lands first.

[janicduplessis]

The main issue I see with this approach however is how we deal with node_modules dependencies of metro. We'd probably have to bundle everything with webpack...

[TheSavior]

@cpojer, what’s the plan for this? Not going to do it anymore? Just do it anyways?

[cpojer]

Metro is not part of React Native any more, as it is now pulling in through the extracted CLI. If we wanna do this, it would be there.