Skip to content

Restore base config merge in metro-config - #38092

Closed
huntie wants to merge 1 commit into
react:mainfrom
huntie:export-D47055973
Closed

Restore base config merge in metro-config#38092
huntie wants to merge 1 commit into
react:mainfrom
huntie:export-D47055973

Conversation

@huntie

@huntiehuntie commented Jun 27, 2023

Copy link
Copy Markdown
Collaborator

Summary

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 react/metro#1010 (comment) for further notes.

Fixes:

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

Test Plan

Steps

  • Using a new React Native 0.72 project.
    • Use yarn link to substitute @react-native/metro-config with local version.
    • Use yarn link to substitute @react-native-community/cli-plugin-metro with local fork.
      • This is modified to log the final config to the terminal.
    • Expand watchFolders in Metro config.
    • yarn start

Produces functionally identical final config

(Diff before and after changes)

App builds

image

Differential Revision: D47055973

@facebook-github-botfacebook-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 Jun 27, 2023
@facebook-github-bot

Copy link
Copy Markdown
Contributor

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

@huntie

Copy link
Copy Markdown
CollaboratorAuthor

@kelset@cipolleschi@hoxyq Do you know if package version bumps are performed as part of the release process? Or should I increment the version here (to be subsequently bumped in the template)?

@analysis-bot

analysis-bot commented Jun 27, 2023

Copy link
Copy Markdown
PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a9,045,226-1
androidhermesarmeabi-v7a8,294,498-1
androidhermesx869,561,418-2
androidhermesx86_649,403,753+0
androidjscarm64-v8a9,604,418-1
androidjscarmeabi-v7a8,731,133-1
androidjscx869,691,465+0
androidjscx86_649,937,808+0

Base commit: c803a5b
Branch: main

@kelset

Copy link
Copy Markdown
Contributor

@kelset@cipolleschi@hoxyq Do you know if package version bumps are performed as part of the release process? Or should I increment the version here (to be subsequently bumped in the template)?

yes they are done as part of the release process, please don't bump here.

@kelset

Copy link
Copy Markdown
Contributor

separate question: how does this change affect existing users on 0.72? will everything keep working as expected, or would they need to change anything?

'cause that will affect if we can cherry-pick in 0.72.

@huntie

Copy link
Copy Markdown
CollaboratorAuthor

@kelset Ah yeah of course, ty 👍🏻

@robhogan

Copy link
Copy Markdown
Collaborator

separate question: how does this change affect existing users on 0.72? will everything keep working as expected, or would they need to change anything?
'cause that will affect if we can cherry-pick in 0.72.

This is non-breaking as I see it, so should be fine to pick - we'll just be exporting a complete config object rather than a loose, partial one.

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 react/metro#1010 (comment) for further notes.
Fixes:
- react/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: eedc4698e651645ada46a013d3945a16965bff22
@facebook-github-bot

Copy link
Copy Markdown
Contributor

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

@github-actions

Copy link
Copy Markdown

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

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

@github-actionsgithub-actionsBot added the Merged This PR has been merged. label Jun 28, 2023
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 react/metro#1010 (comment) for further notes.
Fixes:
- react/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
@huntie
huntie deleted the export-D47055973 branch August 29, 2023 16:15
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.fb-exportedMergedThis PR has been merged.p: FacebookPartner: FacebookPartner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@huntie@facebook-github-bot@analysis-bot@kelset@robhogan