Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop internal base config merge in metro-config #36777

Closed
wants to merge 2 commits into from

Conversation

huntie
Copy link
Member

@huntie huntie commented Apr 3, 2023

Summary:
Changelog: [Internal]

Remove internal merge of getDefaultConfig (Metro base defaults) from @react-native/metro-config. This is unnecessary given the config loading setup of RN CLI and Expo CLI, which use (or replicate) Metro's loadConfig function — which will itself apply defaults appropriately.

This relates to a previously-breaking behaviour documented in the test plan of react-native-community/cli#1896 (independently fixed and no longer load-bearing) (read: no need to cherry pick this change).

image

While this has no effect under the fixed RN CLI setup, this is a worthwhile simplification to this package that better-aligns with current Metro tooling expectations.

Notes

  • getDefaultConfig no longer returns ConfigT (full config), and instead returns MetroConfig (partial config). This is non-breaking with the expected API of a given metro.config.js file.

Differential Revision: D44630645

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner fb-exported labels Apr 3, 2023
@facebook-github-bot
Copy link
Contributor

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

@analysis-bot
Copy link

analysis-bot commented Apr 3, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,620,406 +0
android hermes armeabi-v7a 7,933,092 +0
android hermes x86 9,106,413 +0
android hermes x86_64 8,961,212 +0
android jsc arm64-v8a 9,186,942 +0
android jsc armeabi-v7a 8,377,228 +0
android jsc x86 9,244,529 +0
android jsc x86_64 9,503,012 +0

Base commit: f7dc24c
Branch: main

@facebook-github-bot
Copy link
Contributor

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

@huntie huntie force-pushed the export-D44630645 branch from 858bb56 to 94b1c99 Compare April 18, 2023 11:00
huntie added a commit to huntie/react-native that referenced this pull request Apr 18, 2023
Summary:
Pull Request resolved: facebook#36777

Changelog: [Internal]

Remove internal merge of `getDefaultConfig` (Metro base defaults) from `react-native/metro-config`. This is unnecessary given the config loading setup of RN CLI and Expo CLI, which use (or replicate) Metro's [`loadConfig`](https://github.com/facebook/metro/blob/1e47cb5b3cc289530fb18e402891f9d2816611dd/packages/metro-config/src/loadConfig.js#L182-L190) function — which will itself apply defaults appropriately.

This relates to a previously-breaking behaviour documented in the test plan of react-native-community/cli#1896 (independently fixed and no longer load-bearing) (**read: no need to cherry pick this change**).

https://pxl.cl/2B8NS

While this has no effect under the fixed RN CLI setup, this is a worthwhile simplification to this package that better-aligns with current Metro tooling expectations.

## Notes

- `getDefaultConfig` no longer returns `ConfigT` (full config), and instead returns `MetroConfig` (partial config). This is non-breaking with the expected API of a given `metro.config.js` file.

Differential Revision: D44630645

fbshipit-source-id: e85247b145e87d7bac9eca0bcb90e85e6fec98ae
@github-actions
Copy link

github-actions bot commented Apr 18, 2023

Warnings
⚠️ 🔒 package.json - Changes were made to package.json. This will require a manual import by a Facebook employee.

Generated by 🚫 dangerJS against e74ba1e

jacdebug and others added 2 commits April 18, 2023 04:03
Summary:
This is follow up of Metro release 0.76.2 to Bump versions in RN.

Metro version: 0.76.2
Metro release notes: https://github.com/facebook/metro/releases/tag/v0.76.2
CLI Bump PR: react-native-community/cli#1910
CLI npm version: 12.0.0-alpha.3

Took changelog and testplan from facebook#36793

## Changelog:

[General][Fixed] - Bump CLI to 12.0.0-alpha.3 and Metro to 0.76.2

Pull Request resolved: facebook#36948

Test Plan: CircleCI

Differential Revision: D45081058

Pulled By: jacdebug

fbshipit-source-id: 0326fc407fe52809f75d07f26b23f5a7e69a3900
Summary:
Pull Request resolved: facebook#36777

Changelog: [Internal]

Remove internal merge of `getDefaultConfig` (Metro base defaults) from `react-native/metro-config`. This is unnecessary given the config loading setup of RN CLI and Expo CLI, which use (or replicate) Metro's [`loadConfig`](https://github.com/facebook/metro/blob/1e47cb5b3cc289530fb18e402891f9d2816611dd/packages/metro-config/src/loadConfig.js#L182-L190) function — which will itself apply defaults appropriately.

This relates to a previously-breaking behaviour documented in the test plan of react-native-community/cli#1896 (independently fixed and no longer load-bearing) (**read: no need to cherry pick this change**).

https://pxl.cl/2B8NS

While this has no effect under the fixed RN CLI setup, this is a worthwhile simplification to this package that better-aligns with current Metro tooling expectations.

## Notes

- `getDefaultConfig` no longer returns `ConfigT` (full config), and instead returns `MetroConfig` (partial config). This is non-breaking with the expected API of a given `metro.config.js` file.

Differential Revision: D44630645

fbshipit-source-id: fa2c9f81767659f1c4fa5a8abf267173b182b3d9
@facebook-github-bot
Copy link
Contributor

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

@huntie huntie force-pushed the export-D44630645 branch from 94b1c99 to e74ba1e Compare April 18, 2023 11:06
@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Apr 18, 2023
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 1c4a505.

@huntie huntie deleted the export-D44630645 branch April 19, 2023 10:36
huntie added a commit to huntie/react-native that referenced this pull request Apr 19, 2023
huntie added a commit to huntie/react-native that referenced this pull request Apr 19, 2023
jeongshin pushed a commit to jeongshin/react-native that referenced this pull request May 7, 2023
Summary:
Pull Request resolved: facebook#36777

Changelog: [Internal]

Remove internal merge of `getDefaultConfig` (Metro base defaults) from `react-native/metro-config`. This is unnecessary given the config loading setup of RN CLI and Expo CLI, which use (or replicate) Metro's [`loadConfig`](https://github.com/facebook/metro/blob/1e47cb5b3cc289530fb18e402891f9d2816611dd/packages/metro-config/src/loadConfig.js#L182-L190) function — which will itself apply defaults appropriately.

This relates to a previously-breaking behaviour documented in the test plan of react-native-community/cli#1896 (independently fixed and no longer load-bearing) (**read: no need to cherry pick this change**).

https://pxl.cl/2B8NS

While this has no effect under the fixed RN CLI setup, this is a worthwhile simplification to this package that better-aligns with current Metro tooling expectations.

## Notes

- `getDefaultConfig` no longer returns `ConfigT` (full config), and instead returns `MetroConfig` (partial config). This is non-breaking with the expected API of a given `metro.config.js` file.

Reviewed By: cipolleschi

Differential Revision: D44630645

fbshipit-source-id: 472c3967449dfb99f845a82d9e9c49efc343021c
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
Summary:
Pull Request resolved: facebook#36777

Changelog: [Internal]

Remove internal merge of `getDefaultConfig` (Metro base defaults) from `react-native/metro-config`. This is unnecessary given the config loading setup of RN CLI and Expo CLI, which use (or replicate) Metro's [`loadConfig`](https://github.com/facebook/metro/blob/1e47cb5b3cc289530fb18e402891f9d2816611dd/packages/metro-config/src/loadConfig.js#L182-L190) function — which will itself apply defaults appropriately.

This relates to a previously-breaking behaviour documented in the test plan of react-native-community/cli#1896 (independently fixed and no longer load-bearing) (**read: no need to cherry pick this change**).

https://pxl.cl/2B8NS

While this has no effect under the fixed RN CLI setup, this is a worthwhile simplification to this package that better-aligns with current Metro tooling expectations.

## Notes

- `getDefaultConfig` no longer returns `ConfigT` (full config), and instead returns `MetroConfig` (partial config). This is non-breaking with the expected API of a given `metro.config.js` file.

Reviewed By: cipolleschi

Differential Revision: D44630645

fbshipit-source-id: 472c3967449dfb99f845a82d9e9c49efc343021c
huntie added a commit to huntie/react-native that referenced this pull request Jun 27, 2023
Summary:
Reverts facebook#36777.

This is motivated by reducing user friction when the widespread assumption is for `react-native/metro-config` to export a complete Metro config, as with Expo/rnx-kit, and like previously understood uses of `metro-config`. See facebook/metro#1010 (comment) for further notes.

Fixes:
- facebook/metro#1010
- facebook#38069
- kristerkari/react-native-svg-transformer#276

Note that we do not intend for `react-native/metro-config` to directly export `assetExts` etc — these can be accessed on the `resolver` property from the full config object.

Changelog: [General][Changed] `react-native/metro-config` now includes all base config values from `metro-config`

Differential Revision: D47055973

fbshipit-source-id: 78b59d925be72aa42b4b9d901c6f8d174f2dbae0
huntie added a commit to huntie/react-native that referenced this pull request Jun 28, 2023
Summary:
Pull Request resolved: facebook#38092

Reverts facebook#36777.

This is motivated by reducing user friction when the widespread assumption is for `react-native/metro-config` to export a complete Metro config, as with Expo/rnx-kit, and like previously understood uses of `metro-config`. See facebook/metro#1010 (comment) for further notes.

Fixes:
- facebook/metro#1010
- facebook#38069
- kristerkari/react-native-svg-transformer#276

Note that we do not intend for `react-native/metro-config` to directly export `assetExts` etc — these can be accessed on the `resolver` property from the full config object.

Changelog: [General][Changed] `react-native/metro-config` now includes all base config values from `metro-config`

Reviewed By: robhogan

Differential Revision: D47055973

fbshipit-source-id: eedc4698e651645ada46a013d3945a16965bff22
facebook-github-bot pushed a commit that referenced this pull request Jun 28, 2023
Summary:
Pull Request resolved: #38092

Reverts #36777.

This is motivated by reducing user friction when the widespread assumption is for `react-native/metro-config` to export a complete Metro config, as with Expo/rnx-kit, and like previously understood uses of `metro-config`. See facebook/metro#1010 (comment) for further notes.

Fixes:
- facebook/metro#1010
- #38069
- kristerkari/react-native-svg-transformer#276

Note that we do not intend for `react-native/metro-config` to directly export `assetExts` etc — these can be accessed on the `resolver` property from the full config object.

Changelog: [General][Changed] `react-native/metro-config` now includes all base config values from `metro-config`

Reviewed By: robhogan

Differential Revision: D47055973

fbshipit-source-id: 5ad23cc9700397110de5c0e81c7d76299761ef0a
kelset pushed a commit that referenced this pull request Jun 28, 2023
Summary:
Pull Request resolved: #38092

Reverts #36777.

This is motivated by reducing user friction when the widespread assumption is for `react-native/metro-config` to export a complete Metro config, as with Expo/rnx-kit, and like previously understood uses of `metro-config`. See facebook/metro#1010 (comment) for further notes.

Fixes:
- facebook/metro#1010
- #38069
- kristerkari/react-native-svg-transformer#276

Note that we do not intend for `react-native/metro-config` to directly export `assetExts` etc — these can be accessed on the `resolver` property from the full config object.

Changelog: [General][Changed] `react-native/metro-config` now includes all base config values from `metro-config`

Reviewed By: robhogan

Differential Revision: D47055973

fbshipit-source-id: 5ad23cc9700397110de5c0e81c7d76299761ef0a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants