-
Notifications
You must be signed in to change notification settings - Fork 930
fix: Add runtime error when Metro config cannot be resolved #1889
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
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. |
thymikee
left a comment
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.jsas expected — printing obscure errors due to missing config items.This PR updates the CLI to throw a runtime error when
resolveConfigfails for the target working directory (ctx.root). This is an important DevX enhancement, since a completemetro.config.jsfile will be required from React Native 0.72.Test Plan:
Pre-steps:
yarn linkin local clone of RN CLI.yarn link @react-native-community/cli-plugin-metroinsidepackages/rn-tester(localfacebook/react-nativerepo clone).✅ Now throws for
yarn startinpackages/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 startcommand is fixedconsole.log()to print the config file name)