Migrate cli-plugin-metro into repo

[huntie]

Summary:

Context

RFC: Decoupling Flipper from React Native core: react-native-community/discussions-and-proposals#641

Changes

Inits new package @react-native/community-cli-plugin. This migrates @react-native-community/cli-plugin-metro into the React Native repo, to enable faster iteration by the React Native core team. Specifically:

• This package contains several metro dependencies, which when removed from CLI will no longer require us to ship new CLI releases to get Metro patches and features to users.
• This package contains the start, bundle, and ram-bundle commands (central to the React Native development experience). The start command in particular encapsulates the React Native dev server, for which we have incoming changes to support improved debugging for 0.73.
• This package now only exports commands to be attached as an RN CLI plugin. With this move, we're aiming to internalise the default implementations of these dev commands within React Native — other RN CLI plugins can continue to override these, but must do so wholesale. (See also the recent fix for this: breaking: Fix ability to override commands via config react-native-community/cli#1999.)

In latest version:

• (Microsoft feedback) Re-export unstable_buildBundleWithConfig, marking as unstable. This gives us a time buffer to consider how we repackage this functionality in future.

The package source has been converted from TypeScript to Flow, with a number of new flow-typed/ defs added to meet type coverage requirements.

To dos

• For now, we aren't removing the existing @react-native-community/cli-plugin-metro source — until later PRs consolidate this move by changing dependencies in the react-native package.
• Exported API is reduced!: I'm working with @szymonrybczak to decouple references from RN CLI packages feat: add invoking CLI's scripts for launching Metro in run-ios command react-native-community/cli#2021.

cc @robhogan @thymikee @szymonrybczak @kelset @tido64

Changelog: [Internal]

Differential Revision: D46801501

[facebook-github-bot]

This pull request was exported from Phabricator. Differential Revision: D46801501

[facebook-github-bot]

This pull request was exported from Phabricator. Differential Revision: D46801501

[facebook-github-bot]

This pull request was exported from Phabricator. Differential Revision: D46801501

[tido64]

Can you also export buildBundleWithConfig? We're using it in rnx-kit.

[huntie]

Looking into this! 👍🏻

[tido64]

Before resolving this issue, can you repeat the reasoning behind why you're not going to export buildBundleWithConfig for future reference?

[huntie]

From offline discussion with @tido64:

API changes to this package

We're reducing the surface area of @react-native/cli-commands (formerly @react-native-community/cli-plugin-metro) to interface as a React Native CLI commands plugin (with no other JS exports). Longer term, it may become a completely internal part of React Native.

This means no longer exporting buildBundleWithConfig as a shareable component (previously depended on by rnx-kit).

We're doing this because, combined with Metro, the React Native team is already maintaining two stable public APIs for creating a bundle:

• The react-native bundle command (this package) — CLI entry point to creating a bundle, configured with a CLI config file + Metro config.
• Metro's Server API — JS interface which allows implementers (e.g. Expo, rnx-kit) to bootstrap their own bundle logic.

Maintaining buildBundleWithConfig as a third function is unnecessary surface area for us. We believe it's reasonable instead for alternative CLIs/tooling to reimplement this logic themselves (using the above Metro APIs), despite some foregone code reuse today.

[kelset]

am I wrong or you are hard-removing this code without even a deprecation phase?

[huntie]

We are, given this isn't a user facing API or feature (the command/config interface to the exposed bundle command for users is unchanged).

This code removal is inline with any inter-package API changes that can be made in the CLI codebase within a major React Native cycle. This change (removal of cli-plugin-metro) is one of many breaking changes to packages in CLI 12.x, and we've been working with the CLI team on this and the related dependency decoupling.

Given rnx-kit is an API consumer of this outgoing package (which we didn't see initially), you've rightly pointed out that we should consider compatibility. After discussing with @tido64 we've compared our points of view and agreed this API removal. In particular, since rnx-kit wants to enable further dynamic configuration of config, outside of the supported CLI config entry point (which is user facing), it makes sense for rnx-kit to now own its implementation of the bundle command.

[tido64]

After discussing with @tido64 we've compared our points of view and agreed this API removal.

Just to clarify, I did not agree to this removal. I am pretty sure I made this crystal clear.

cli-plugin-metro was created by us so that we can share the bundle logic between @react-native-community/cli and ourselves, and contribute to a greater good. My personal opinion is that us copying all the logic almost verbatim in our own @rnx-kit/metro-service is just an overhead for us while providing no gains whatsoever. I also argued that it splits the community because any improvements we make no longer automatically accrues value to anyone not using our CLI and vice versa. If the Metro team insists on this direction, we have no other choice but to go our separate paths.

[motiz88]

Hey @tido64 - I'm sure we all can come to an understanding here. The structure of dependencies between React Native, Metro and @react-native-community/cli is a longstanding pain point that's reduced our ability (as the React Native team at Meta) to improve the developer experience for the benefit of the entire ecosystem. Our aim in rearranging the packages is to facilitate greater sharing and more consistency across the major RN integration toolkits (Meta, RN CLI, rnx-kit, Expo). We have no intention to "split the community" or force you to do work you don't think is useful. Let's follow up in our upcoming Metro contributors meeting and make sure we have an agreed way forward.

[motiz88]

Following up from our meeting yesterday, I have a proposal for how to proceed.

RN 0.73

Since we weren't aware of the rnx-kit call site when planning this change, I'd like to first ensure we're not forcing a sudden breaking change on rnx-kit without an agreed migration path. To that end, we'll export unstable_buildBundleWithConfig from the new @react-native/community-cli-plugin package in React Native 0.73.0, with the understanding that it will likely move or otherwise change in future versions ( = some time in the next few months). We'll discuss changes with @tido64 ahead of time (but will explicitly not support any new call sites / packages beyond existing usage).

Migration path (RN 0.74+)

For ease of maintenance and deduplicating package responsibilities, @react-native/community-cli-plugin will eventually be purely a plugin for @react-native-community/cli. That is, it will not have secondary exports. Any other useful functionality it exposes on top of being a plugin will need to move elsewhere, where it can be consumed without being tied specifically to the RN CLI. As such, unstable_buildBundleWithConfig will need to be removed, and rnx-kit will need an alternative.

My understanding of what buildBundleWithConfig does (and how we might split it up) is:

1. It validates the platform argument against the Metro config, which can be moved directly into Metro IMO.
2. It mutates the current environment according to the dev parameter, which tbh is a strange side effect I wouldn't want to include in a reusable function. rnx-kit should feel free to copy this line of code if it makes sense for its users, or we can just delete it.
3. It strips path information from sourceMapUrl unless --sourcemap-use-absolute-path is specified. Not sure about this one, but we might be able to bake this into Metro and remove support for --sourcemap-use-absolute-path (I doubt anyone's using this option).
4. It uses documented, typed Metro APIs to build a bundle and get the asset information for that bundle - this code is trivial and safe to replicate.
5. It writes out asset files in a platform-dependent way. This part is responsible for the bulk of additional logic that rnx-kit had to fork from the CLI before buildBundleWithConfig existed. I'll discuss this below.

Re: (5) specifically - the build logic for assets should have a single source of truth in React Native, close to the @react-native/assets-registry package which already implicitly depends on it. It is clearly useful to all apps that use @react-native/assets-registry, not just ones that happen to be built with the RN CLI. It might belong in a new package, or directly in @react-native/assets-registry for that matter (but not in @react-native/community-cli-plugin).

@tido64, please let me know whether this seems workable to you. (I can imagine a few alternative designs, but let's start here.) I would love your help in getting the right abstractions in place so that rnx-kit continues to be a valuable tool for RN users as we iterate on the core (cross-framework) developer experience.

[acoates-ms]

For (5) specifically, the current logic there is currently too platform specific, and only for the built in platforms. This logic needs to be customizable for additional platforms. @react-native/assets-registry is one part of this logic, but it does not handle things like writing iOS/macOS assetCatalogs. And similar to some of the custom file location logic for android, windows will also need some custom file location logic.

I created react-native-community/cli#2002 (which after this PR will need porting to core instead), which would allow platforms to provide their own specific logic for how to save assets collected during the bundle process.

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,891,571	-155
android	hermes	armeabi-v7a	n/a	--
android	hermes	x86	n/a	--
android	hermes	x86_64	n/a	--
android	jsc	arm64-v8a	9,483,545	+44
android	jsc	armeabi-v7a	n/a	--
android	jsc	x86	n/a	--
android	jsc	x86_64	n/a	--

Base commit: 0c483d3
Branch: main

[acoates-ms]

How will we integrate additional platform specific logic into this?
Currently this code has platform specific logic to deal with assets (generate assetcatalogs on ios and use android specific paths for different scale factors).

We need to add specific logic for other platforms too. react-native-community/cli#2002 was going to modify the rn-cli to allow out of tree platforms to plugin platform specific functionality for asset handing, which needs to be part of the bundle step.

The rn-cli knows about the rn-config object which has all the infra to support out of tree platforms and therefore platform specific extension points. Moving the code here means we lose out on all the infra to handle out of tree platforms. - thus breaking rn-windows and rn-macos.

[huntie]

@acoates-ms This is a a relocation of the cli-plugin-metro package, and theoretically your PR can be reopened against this repo after these changes!

Moving the code here means we lose out on all the infra to handle out of tree platforms

AFAIK, we will retain support for out-of-tree platforms with this move.

@react-native/cli-commands remains a React Native CLI plugin, and continues to:

• Interface with the plugin command interface.
• Receive the Config context object from CLI (the integration point which you've extended in your PR).
• Depend on other CLI packages, e.g. @react-native-community/cli-tools.

[acoates-ms]

Ah, I thought the config stuff was all outside of the repo. I guess it just means there is a repo cycle of dependencies between the packages, which is annoying to deal with. But good to hear that this wouldn't block that.

[huntie]

@acoates-ms Yeah the cross-repo iteration friction is shifted, per the summary.

• Changes to this package can now be made inside the react-native repo and will run from source 🙂 (see Add shared monorepo build setup #38718), which should provide faster iteration when working with this code.
• Crucially, there is no circular package dependency, as we will apply these commands via the CLI's plugin mechanism (packages/react-native/react-native.config.js), and are detaching references in CLI in feat: add invoking CLI's scripts for launching Metro in run-ios command react-native-community/cli#2021.

[facebook-github-bot]

This pull request was exported from Phabricator. Differential Revision: D46801501

[facebook-github-bot]

This pull request was exported from Phabricator. Differential Revision: D46801501

[facebook-github-bot]

This pull request was exported from Phabricator. Differential Revision: D46801501

[facebook-github-bot]

This pull request was exported from Phabricator. Differential Revision: D46801501

[facebook-github-bot]

This pull request was exported from Phabricator. Differential Revision: D46801501

[facebook-github-bot]

This pull request was exported from Phabricator. Differential Revision: D46801501

[facebook-github-bot]

This pull request was exported from Phabricator. Differential Revision: D46801501

[github-actions]

This pull request was successfully merged by @huntie in e199880.

^{When will my fix make it into a release? | Upcoming Releases}