Skip to content

breaking: Update cli-plugin-metro to use defaults in app metro.config.js (RN 0.72) - #1875

Merged
thymikee merged 1 commit into
react-native-community:mainfrom
huntie:rn-metro-config-stack-1
Mar 23, 2023
Merged

breaking: Update cli-plugin-metro to use defaults in app metro.config.js (RN 0.72)#1875
thymikee merged 1 commit into
react-native-community:mainfrom
huntie:rn-metro-config-stack-1

Conversation

@huntie

@huntiehuntie commented Mar 15, 2023

Copy link
Copy Markdown
Collaborator

Context

React Native Metro config → React Native repo (react/react-native#36502)

We (the React Native team) are aiming to relocate the default Metro config for React Native out of @react-native-community/cli-plugin-metro and into the React Native repo + app template as a new @react-native/metro-config package.

This is the first (and minimum viable) phase we can ship to separate the release process of Metro from RN CLI in order to reduce coupling and iterate faster for our users.

See full motivation, design, and test plan here: react/react-native#36502

Changes

This PR updates React Native CLI so that the new config setup will work.

image

  • Removes all static config values and renames internal function to getOverrideConfig.
  • Removes loadMetroConfig from the public API of cli-plugin-metro.

Changelog: [Breaking] Remove static Metro defaults from cli-plugin-metro, remove getDefaultConfig

Test plan

image

Inside React Native repo using rn-tester:

  • ✅ Local metro.config.js matching new template is loaded
  • @react-native/metro-config config is extended
  • ✅ These CLI changes are loaded
  • ✅ App packages and runs

Rollout plan

This is a breaking change to RN CLI that will be incompatible with React Native < 0.72.

React Native 0.72 is being prepared, and will include a version bump to React Native CLI once the following react-native PRs are merged.

@huntiehuntie changed the title rn metro config stack 1[RFC] feat: Update cli-plugin-metro to use defaults in app metro.config.js from RN 0.72Mar 15, 2023
@huntie
huntieforce-pushed the rn-metro-config-stack-1 branch 6 times, most recently from 598f1a0 to c53e49bCompareMarch 16, 2023 13:56
@huntie

Copy link
Copy Markdown
CollaboratorAuthor

Opening PR for feedback and am preparing E2E test scenarios.

@huntie
huntie marked this pull request as ready for review March 16, 2023 16:04
@huntie

Copy link
Copy Markdown
CollaboratorAuthor

Hmm, it's dawned on me that this could be done as a breaking change to RN CLI, and we can drop the version-checking logic. Rest of plan unchanged. Will update tomorrow.

@huntie
huntieforce-pushed the rn-metro-config-stack-1 branch from c53e49b to eab2533CompareMarch 17, 2023 15:27
@huntiehuntie changed the title [RFC] feat: Update cli-plugin-metro to use defaults in app metro.config.js from RN 0.72breaking: Update cli-plugin-metro to use defaults in app metro.config.js (RN 0.72)Mar 17, 2023
@huntie

Copy link
Copy Markdown
CollaboratorAuthor

PR is simplified, retitled, and ready for review. Note that I'm still validating E2E behaviour in the test plan of react/react-native#36502.

@huntie

huntie commented Mar 20, 2023

Copy link
Copy Markdown
CollaboratorAuthor

Test plan updated — this change is tested against all incoming changes in the RN repo (update: upstream is ready).

@adamTrz@thymikee Ready for review.

Comment threadpackages/cli-plugin-metro/src/tools/loadMetroConfig.ts
* Default configuration
* Get the config options to override based on RN CLI inputs.
*/
export const getDefaultConfig = (ctx: ConfigLoadingContext): InputConfigT => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's check if this is not used by Windows/macOS teams. cc @tido64@Saadnajmi

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find any references to this function in react-native-windows/-macos. Nor do we use it in rnx-kit. I'd say it's safe to remove it, but I'll let @Saadnajmi verify as well.

@jonthyselljonthysellMar 30, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tido While this particular function isn't used, the answer is yes, this overall PR breaks RNW. :) All of our PRs are blocked because we test changes by creating a new app with the CLI and verifying it builds/bundles.

See: microsoft/react-native-windows#11437

We currently have a (hopefully temporary) workaround here: microsoft/react-native-windows#11438

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should've been fixed in: #1889. Is this still happening as in 11.0.1?

@thymikeethymikee left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's happening 🚀

@huntie
huntieforce-pushed the rn-metro-config-stack-1 branch from eab2533 to 47278fdCompareMarch 22, 2023 11:29
@huntie

huntie commented Mar 22, 2023

Copy link
Copy Markdown
CollaboratorAuthor

Feedback addressed. Ready to merge once fully confirmed by @tido64 / @Saadnajmi :).

@Saadnajmi

Copy link
Copy Markdown
Contributor

Feedback addressed. Ready to merge once fully confirmed by @tido64 / @Saadnajmi :).

Why are you tagging me? I'm not a maintainer.

@tido64tido64 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feedback addressed. Ready to merge once fully confirmed by @tido64 / @Saadnajmi :).

Why are you tagging me? I'm not a maintainer.

I think @huntie wants you to verify this: #1875 (comment)

Anyway, I don't think we should delay this any further.

@Saadnajmi

Copy link
Copy Markdown
Contributor

Feedback addressed. Ready to merge once fully confirmed by @tido64 / @Saadnajmi :).

Why are you tagging me? I'm not a maintainer.

I think @huntie wants you to verify this: #1875 (comment)

Anyway, I don't think we should delay this any further.

Yeah just go for it.

@thymikee
thymikee merged commit 7867612 into react-native-community:mainMar 23, 2023
@huntie
huntie deleted the rn-metro-config-stack-1 branch March 27, 2023 10:31
facebook-github-bot pushed a commit to react/react-native that referenced this pull request Mar 30, 2023
Summary:
Pull Request resolved: #36623
Changelog:
[General][Changed] - The default `metro.config.js` in apps now extends `react-native/metro-config`, and should be updated in existing apps.
~~`react-native/rn-get-polyfills.js` is removed and should be updated to `react-native/js-polyfills` in existing apps (this is part of the new default config).~~
#publish-packages-to-npm
## Context
### React Native Metro config → React Native repo (#36502)
We (the React Native team) are aiming to relocate the default Metro config for React Native out of `react-native-community/cli-plugin-metro` and **into the React Native repo + app template** as a new `react-native/metro-config` package.
This is the first (and minimum viable) phase we can ship to separate the release process of Metro from RN CLI in order to reduce coupling and iterate faster for our users.
**See full motivation, design, and test plan (which previewed the CLI bump) here: #36502
## Changes
NOTE: This PR is pending the inclusion of a bump to `react-native-community/cli`, and will be sequenced after react-native-community/cli#1875 is merged.
- Upgrade `react-native-community/cli` to `11.0.0`, upgrade all `metro` packages to `0.76.0` (version distributed in this CLI release).
- Update the `metro.config.js` file in `packages/react-native/template/`.
- Now merges defaults from `react-native/metro-config`, and can be used with CLI >= 11.0.0.
- Update the `metro.config.js` files for `packages/react-native/` and `packages/rn-tester/` (these are integration test locations).
- Now merges defaults from `react-native/metro-config`, and can be used with CLI >= 11.0.0.
Changes to `react-native/metro-config` — `0.72.1` (prepared but not depended on yet):
- Export `mergeConfig` util (removing direct `metro-config` dependency in consuming projects).
- Explicitly depend on `metro-react-native-babel-transformer` and `metro-runtime` (transitively included today).
Reviewed By: cortinico, blakef
Differential Revision: D44099691
fbshipit-source-id: 405635dd69fd50a1e9548279eaeda3c932b5b167
blakef pushed a commit to blakef/template that referenced this pull request Feb 28, 2024
Summary:
Pull Request resolved: react/react-native#36623
Changelog:
[General][Changed] - The default `metro.config.js` in apps now extends `react-native/metro-config`, and should be updated in existing apps.
~~`react-native/rn-get-polyfills.js` is removed and should be updated to `react-native/js-polyfills` in existing apps (this is part of the new default config).~~
#publish-packages-to-npm
## Context
### React Native Metro config → React Native repo (react/react-native#36502)
We (the React Native team) are aiming to relocate the default Metro config for React Native out of `react-native-community/cli-plugin-metro` and **into the React Native repo + app template** as a new `react-native/metro-config` package.
This is the first (and minimum viable) phase we can ship to separate the release process of Metro from RN CLI in order to reduce coupling and iterate faster for our users.
**See full motivation, design, and test plan (which previewed the CLI bump) here: react/react-native#36502
## Changes
NOTE: This PR is pending the inclusion of a bump to `react-native-community/cli`, and will be sequenced after react-native-community/cli#1875 is merged.
- Upgrade `react-native-community/cli` to `11.0.0`, upgrade all `metro` packages to `0.76.0` (version distributed in this CLI release).
- Update the `metro.config.js` file in `packages/react-native/template/`.
- Now merges defaults from `react-native/metro-config`, and can be used with CLI >= 11.0.0.
- Update the `metro.config.js` files for `packages/react-native/` and `packages/rn-tester/` (these are integration test locations).
- Now merges defaults from `react-native/metro-config`, and can be used with CLI >= 11.0.0.
Changes to `react-native/metro-config` — `0.72.1` (prepared but not depended on yet):
- Export `mergeConfig` util (removing direct `metro-config` dependency in consuming projects).
- Explicitly depend on `metro-react-native-babel-transformer` and `metro-runtime` (transitively included today).
Reviewed By: cortinico, blakef
Differential Revision: D44099691
fbshipit-source-id: 405635dd69fd50a1e9548279eaeda3c932b5b167
Original: react/react-native@c5a47ab
blakef pushed a commit to react-native-community/template that referenced this pull request Feb 29, 2024
Summary:
Pull Request resolved: react/react-native#36623
Changelog:
[General][Changed] - The default `metro.config.js` in apps now extends `react-native/metro-config`, and should be updated in existing apps.
~~`react-native/rn-get-polyfills.js` is removed and should be updated to `react-native/js-polyfills` in existing apps (this is part of the new default config).~~
#publish-packages-to-npm
## Context
### React Native Metro config → React Native repo (react/react-native#36502)
We (the React Native team) are aiming to relocate the default Metro config for React Native out of `react-native-community/cli-plugin-metro` and **into the React Native repo + app template** as a new `react-native/metro-config` package.
This is the first (and minimum viable) phase we can ship to separate the release process of Metro from RN CLI in order to reduce coupling and iterate faster for our users.
**See full motivation, design, and test plan (which previewed the CLI bump) here: react/react-native#36502
## Changes
NOTE: This PR is pending the inclusion of a bump to `react-native-community/cli`, and will be sequenced after react-native-community/cli#1875 is merged.
- Upgrade `react-native-community/cli` to `11.0.0`, upgrade all `metro` packages to `0.76.0` (version distributed in this CLI release).
- Update the `metro.config.js` file in `packages/react-native/template/`.
- Now merges defaults from `react-native/metro-config`, and can be used with CLI >= 11.0.0.
- Update the `metro.config.js` files for `packages/react-native/` and `packages/rn-tester/` (these are integration test locations).
- Now merges defaults from `react-native/metro-config`, and can be used with CLI >= 11.0.0.
Changes to `react-native/metro-config` — `0.72.1` (prepared but not depended on yet):
- Export `mergeConfig` util (removing direct `metro-config` dependency in consuming projects).
- Explicitly depend on `metro-react-native-babel-transformer` and `metro-runtime` (transitively included today).
Reviewed By: cortinico, blakef
Differential Revision: D44099691
fbshipit-source-id: 405635dd69fd50a1e9548279eaeda3c932b5b167
Original-Commit: react/react-native@c5a47ab
blakef pushed a commit to react-native-community/template that referenced this pull request Feb 29, 2024
Summary:
Pull Request resolved: react/react-native#36623
Changelog:
[General][Changed] - The default `metro.config.js` in apps now extends `react-native/metro-config`, and should be updated in existing apps.
~~`react-native/rn-get-polyfills.js` is removed and should be updated to `react-native/js-polyfills` in existing apps (this is part of the new default config).~~
#publish-packages-to-npm
## Context
### React Native Metro config → React Native repo (react/react-native#36502)
We (the React Native team) are aiming to relocate the default Metro config for React Native out of `react-native-community/cli-plugin-metro` and **into the React Native repo + app template** as a new `react-native/metro-config` package.
This is the first (and minimum viable) phase we can ship to separate the release process of Metro from RN CLI in order to reduce coupling and iterate faster for our users.
**See full motivation, design, and test plan (which previewed the CLI bump) here: react/react-native#36502
## Changes
NOTE: This PR is pending the inclusion of a bump to `react-native-community/cli`, and will be sequenced after react-native-community/cli#1875 is merged.
- Upgrade `react-native-community/cli` to `11.0.0`, upgrade all `metro` packages to `0.76.0` (version distributed in this CLI release).
- Update the `metro.config.js` file in `packages/react-native/template/`.
- Now merges defaults from `react-native/metro-config`, and can be used with CLI >= 11.0.0.
- Update the `metro.config.js` files for `packages/react-native/` and `packages/rn-tester/` (these are integration test locations).
- Now merges defaults from `react-native/metro-config`, and can be used with CLI >= 11.0.0.
Changes to `react-native/metro-config` — `0.72.1` (prepared but not depended on yet):
- Export `mergeConfig` util (removing direct `metro-config` dependency in consuming projects).
- Explicitly depend on `metro-react-native-babel-transformer` and `metro-runtime` (transitively included today).
Reviewed By: cortinico, blakef
Differential Revision: D44099691
fbshipit-source-id: 405635dd69fd50a1e9548279eaeda3c932b5b167
Original-Commit: react/react-native@c5a47ab
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@huntie@Saadnajmi@tido64@thymikee@jonthysell