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

fix: check for supportedExtensions after filtering ignored files #143

Merged
merged 1 commit into from
Nov 16, 2021

Conversation

ullasholla
Copy link
Contributor

If there are any staged files in directory containing a package.json (it doesn't need to be changed/staged), even if the directory is meant to be ignored, the function isSupportedExtension() is run. This can fail if the package.json file has invalid JSON for example (resolveConfig from prettier actually fails). Moving the filters around so that the ignored files are properly accounted for gets rid of the error.

I'm not sure if the unstaged files need to be filtered the same way: https://github.com/azz/pretty-quick/blob/f11faa91d5c570407c37ed77ac04ec0f5e0ef64e/src/index.js#L52-L59

Example repository to test this: https://github.com/ullasholla/pretty-quick-ignore-test
The templates directory contain ejs templates and the .json files are not yet valid JSON files.

To test, make a change to templates/core/config.json and stage it.

Tests are passing locally:
image

Could you please review and consider merging this?

Thanks!

@ullasholla ullasholla changed the title fix: check for supportedExtenions after filtering ignored files fix: check for supportedExtensions after filtering ignored files Nov 10, 2021
@ullasholla
Copy link
Contributor Author

Could I get a review for this please? @azz, maybe?

@azz azz merged commit 5cb47e6 into prettier:master Nov 16, 2021
@azz
Copy link
Member

azz commented Nov 16, 2021

Thanks!

@azz
Copy link
Member

azz commented Nov 16, 2021

🎉 This PR is included in version 3.1.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants