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

breaking: Remove fallback flow for Metro config defaults (0.73) #1972

Merged
merged 1 commit into from
Jul 19, 2023

Conversation

huntie
Copy link
Collaborator

@huntie huntie commented Jun 16, 2023

Summary

This follows up the @react-native/metro-config setup added in React Native 0.72. It removes the fallback to a replicated internal copy of Metro config defaults in cli-plugin-metro, effectively reverting #1896.

Rollout plan recap:

  • 0.72: Extending @react-native/metro-config is recommended, but falls back if users have not updated their metro.config.js, logging a warning.
  • 0.73 (this diff): Updating metro.config.js to the new format is required — log warning and (later on) fail.

Changelog: [Breaking] Remove fallback flow for Metro config defaults (React Native 0.73)

Test Plan

Using rn-tester configured with a locally yarn linked copy of RN CLI.

✅ Correct config logs no warning and builds successfully

image

✅ Incorrect config logs updated warning and fails

⬇️ FYI @cortinico @kelset, in case you have opinions for better DevX. It is marginally restrictive that we require a reference to @react-native/metro-config in the immediate metro.config.js file — perhaps if we detect this more strongly we can fail rather than warn. However I can't think of a simple way to do this — and we will have had the 0.72 release cycle for people to update their projects by this point. Addressed with more visible warning.

image

@thymikee
Copy link
Member

This ties the move of the cli-plugin-metro effort to the core with v0.73, right? Maybe we should start merging those changes to a parallel branch with the plugin removal?

@huntie
Copy link
Collaborator Author

huntie commented Jun 16, 2023

@thymikee It does — I was aiming to stack a sequence of simplification changes, including this PR, here in the CLI repo first. This is to slightly reduce the surface area that we are migrating into the RN repo (and from TypeScript into Flow 🙈).

A change like this also gets better visibility in this repo for now.

@kelset
Copy link
Member

kelset commented Jun 19, 2023

added a mention here reactwg/react-native-releases#64 so that we/the 73 release crew won't forget that there's BC on this angle

@huntie
Copy link
Collaborator Author

huntie commented Jun 20, 2023

@kelset Sidenote: I'm looking to merge this change ahead of work to migrate and collapse cli-plugin-metro back into the React Native repository, with the intent being to minimise the number of files/surface area before this migration.


With some over-the-weekend reflection — the flow in this PR should probably hard-error (helpful in particular to pin down this error cause in CI) ➡️ update incoming.

For the user, it might be better if we allowed the entire metro.config.js file to be optional (but reinsertable, for standalone metro commands) (especially after facebook/metro#977 ships). We can look at this post-migration, since it'll be possible to refer to @react-native/metro-config in the same repo.

@huntie
Copy link
Collaborator Author

huntie commented Jun 29, 2023

Latest push

  • This check remains a warning, but is made much more visible (as lack of this config will break builds except for rare advanced usages). cc @motiz88 for wording sign off.
  • Now includes the updated check from feat: Add alternative base config check to loadMetroConfig (11.x) #1990 — on main we can safely depend on the future version of @react-native/metro-config with the new detection mechanism present.

image

✅ Ready for review

@huntie huntie requested a review from motiz88 June 29, 2023 15:39
);
From React Native 0.73, your project's Metro config should extend '@react-native/metro-config'
or it will fail to build. Please copy the template at:
https://github.com/facebook/react-native/blob/main/packages/react-native/template/metro.config.js
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wonder if this should be pinned (at least to a branch) so it remains accurate in the future.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah fair. We don't have 0.73-stable yet — and the counter-risk is pointing to a stale copy. Will keep an eye on it.

@thymikee thymikee merged commit 1fc7eba into react-native-community:main Jul 19, 2023
@huntie huntie deleted the metro-config-0.73 branch July 19, 2023 16:49
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.

5 participants