-
-
Notifications
You must be signed in to change notification settings - Fork 76
ref: Handle edge cases in formatting step more gracefully #687
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
const packageJson = await getPackageDotJson(); | ||
const prettierInstalled = hasPackageInstalled('prettier', packageJson); | ||
|
||
Sentry.setTag('prettier-installed', prettierInstalled); | ||
|
||
if (!prettierInstalled) { | ||
return; | ||
} | ||
|
||
// prompt the user if they want to run prettier | ||
const shouldRunPrettier = await abortIfCancelled( | ||
clack.confirm({ | ||
message: | ||
'Looks like you have Prettier in your project. Do you want to run it on your files?', | ||
}), | ||
); | ||
|
||
if (!shouldRunPrettier) { | ||
return; | ||
} | ||
|
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.
No behavior changes here, just made a small refactoring to avoid the double-nested if block
} catch (e) { | ||
prettierSpinner.stop('Prettier failed to run.'); | ||
clack.log.error( | ||
clack.log.warn( |
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.
I think it's ok to downgrade this to a warning because it won't (shouldn't) cause the wizard to error. It'll just continue with the next step.
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.
Looks good to me!
Just noticed while running a repro sveltekit app in a new directory where I didn't yet initialize git that the auto-format step wasn't behaving correctly. For some reason, the child process promise when running
npx prettier ...
never resolved and I had to quit the wizard.This PR makes a couple of changes to the prettier formatting step: