-
Notifications
You must be signed in to change notification settings - Fork 905
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: Add runtime error when Metro config cannot be resolved #1889
Conversation
Thanks for the fix 🎉 I see that you changed the command from direct call |
@szymonrybczak Yes! This PR is basically a test plan for that |
Cool! So in future we can get rid of |
Updated to address feedback. Ready to merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@huntie I've updated the wording and replaced console.warn
with logger.warn
. LGTM
Hi folks, can we pick this into |
Summary: Related: react-native-community/cli#1889. Changelog: **[Types]** Fix return type on `'metro-config'.resolveConfig` Reviewed By: motiz88 Differential Revision: D44540075 fbshipit-source-id: 182c3d7ac3c75446a905cb2988bef8044dea2a76
hi @huntie if (options.reporter) {
overrideConfig.reporter = options.reporter;
} loadConfig() already set reporter from options loadConfig({cwd: ctx.root, ...options}) |
Summary:
Relates to:
While preparing facebook/react-native#36623, we discovered that (post monorepo) a few scripts in the React Native repo were starting RN CLI in directories outside of the project dir and weren't reading
metro.config.js
as expected — printing obscure errors due to missing config items.This PR updates the CLI to throw a runtime error when
resolveConfig
fails for the target working directory (ctx.root
). This is an important DevX enhancement, since a completemetro.config.js
file will be required from React Native 0.72.Test Plan:
Pre-steps:
yarn link
in local clone of RN CLI.yarn link @react-native-community/cli-plugin-metro
insidepackages/rn-tester
(localfacebook/react-native
repo clone).✅ Now throws for
yarn start
inpackages/rn-tester
.(Note: I'm open to suggestions for an improved error message/method of erroring to improve the CLI output.)
✅ Does not throw when
yarn start
command is fixedconsole.log()
to print the config file name)