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

fix: Drop @react-native/metro-config dep for RN 0.72 config fallback #1899

Merged
merged 1 commit into from
Apr 6, 2023

Conversation

huntie
Copy link
Collaborator

@huntie huntie commented Apr 4, 2023

Context

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

This is a follow up to #1896.

Unfortunately the most recent CLI release was auto-consumed in React Native's CI, exposing a blocker where we could not consume the (repo-)circular dependency react-native@react-native-community/cli-plugin-metro@react-native/metro-config — I 100% jumped the gun on this setup(!).

While this is now mitigated by the new CLI release process, and possible in practice, we're opting to remove this circular setup as it would introduce maintenance headaches while we are working across two repos.

Changelog: Internal


This is an architectural simplification targeting React Native main (0.73). However it is also worth cherry picking this commit to RN CLI 10.x (targeting RN 0.72), so that we can once again bump all local Metro dependencies here (and replicate those bumps in both 11.x and 10.x). cc @robhogan @cipolleschi

Open as #1901.


Changes

  • Drop @react-native/metro-config dependency from cli-plugin-metro.
  • Restore reproduced @react-native/metro-config default config within cli-plugin-metro (to be used in the fallback case only).
    • This is now equivalent to the original (abandoned) version of #1875.
    • Ideally, we will only keep these values in this repo for the next 1-2 versions of React Native. Once removed, we will still have the throw-descriptive-error fallback for unmigrated projects.

Note: An alternative approach for this is peerDependencies, but given the recent issues and how this might affect strict internal and open source CI, I'm opting for the absolutely bulletproof solution.

Test Plan

(Repeats test plan instructions from #1896).

New 0.72 config setup (project extends @react-native/metro-config) Fallback 0.71 config setup (project does not extend @react-native/metro-config)
image image

✅ Identical configs are produced
(With only path changes from yarn link)

image

Comment on lines +17 to +19
"metro-react-native-babel-transformer": "0.76.0",
"metro-resolver": "0.76.0",
"metro-runtime": "0.76.0",
Copy link
Collaborator Author

@huntie huntie Apr 4, 2023

Choose a reason for hiding this comment

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

Comment on lines +61 to +62
getPolyfills: () =>
require(path.join(ctx.reactNativePath, 'rn-get-polyfills'))(),
Copy link
Collaborator Author

@huntie huntie Apr 4, 2023

Choose a reason for hiding this comment

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

Restored (https://github.com/react-native-community/cli/pull/1875/files#diff-3eb3e1e81b924a57b22a630c633912f2f8777b5a201992591c8a3517fb63f525L76-L77).

Deviates from @react-native/metro-config, which references the standalone package @react-native/js-polyfills (technically, this was/is the first circular dep edge case!). These are equivalent, but allows this repo to not reference the RN-hosted package (see removal in yarn.lock).

https://github.com/facebook/react-native/blob/bece6500f7d6374eab09917c9afd3bf6142f0b61/packages/metro-config/index.js#L47-L49

Comment on lines -2676 to -2690
"@react-native/js-polyfills@^0.73.0":
version "0.73.0"
resolved "https://registry.yarnpkg.com/@react-native/js-polyfills/-/js-polyfills-0.73.0.tgz#d1becae77740c705392122684a55b373e610ab63"
integrity sha512-3/+q+l86xvV8xjx29On9/Dn0eWjo5MHYuuALHqyOlQIs3vLk/rQB4rn4izObiurhH3utXxqVnbOk+TB9yJo4ng==

"@react-native/metro-config@^0.73.0":
version "0.73.0"
resolved "https://registry.yarnpkg.com/@react-native/metro-config/-/metro-config-0.73.0.tgz#7601d2f4ba2252a4e359f8fda21008daf5f31eb2"
integrity sha512-b+q8Ot6nxPE2PXEua20Q0vhXYpVtVjUaTX/hv+7mL5hLzKeIWq/bIxF04GLWT93BBjGSlL0xaZ7iGIQNN75Hog==
dependencies:
"@react-native/js-polyfills" "^0.73.0"
metro-config "0.76.0"
metro-react-native-babel-transformer "0.76.0"
metro-runtime "0.76.0"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

All dependencies on packages published from facebook/react-native are dropped.

@thymikee thymikee merged commit fbd12ea into react-native-community:main Apr 6, 2023
@huntie huntie deleted the drop-metro-config-dep branch May 25, 2023 14:13
Sign up for free to 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.

3 participants