Skip to content

Conversation

Lms24
Copy link
Member

@Lms24 Lms24 commented Oct 15, 2024

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:

  • bail if we're not in a git repo
  • bail if no unchanged/untracked files are detected (could happen if users commit the changes while the wizard is runningI)
  • only then prompt users if they want to run prettier

@Lms24 Lms24 requested review from a team, andreiborza, onurtemizkan and stephanie-anderson and removed request for a team and stephanie-anderson October 15, 2024 08:54
@Lms24 Lms24 self-assigned this Oct 15, 2024
Comment on lines +692 to +712
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;
}

Copy link
Member Author

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(
Copy link
Member Author

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.

Copy link
Collaborator

@onurtemizkan onurtemizkan left a 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!

@Lms24 Lms24 merged commit 730d3d7 into master Oct 15, 2024
15 checks passed
@Lms24 Lms24 deleted the lms/ref-prettier-no-git-repo branch October 15, 2024 10:56
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.

2 participants