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

Prevent Flow from checking node_modules, add Prettier config #114

Merged
merged 3 commits into from
Nov 16, 2020

Conversation

haydn
Copy link
Member

@haydn haydn commented Nov 13, 2020

Some tools try to run Flow checks on files in node_modules which have the @flow pragma. These errors aren't relevant. We can prevent this by using the declarations config:

Also baked in the Prettier config we're using to avoid tools falling back to some other config.

@vercel
Copy link

vercel bot commented Nov 13, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/sketchbook-js/sketchbook/ntkvgomgb
✅ Preview: https://sketchbook-git-flowconfig.sketchbook-js.vercel.app

@haydn haydn changed the title Prevent Flow from checking node_modules Prevent Flow from checking node_modules, add Prettier config Nov 15, 2020
@haydn haydn requested a review from Mxchaeltrxn November 15, 2020 06:13
@haydn
Copy link
Member Author

haydn commented Nov 15, 2020

@Mxchaeltrxn Can I get your eyes on this? Just want to check the changes I've made don't mess-up for you locally.

@Mxchaeltrxn
Copy link
Member

@haydn So I'm guessing you wanted me to try running that locally. I just tried using that configuration and running the yarn format script in package.json and had no issues!

Also, any chance this configuration would get changed in the future? I quite like having trailingComma and arrowParens to say the least (from a productivity perspective).

Anyway, I get that it might cause merge conflicts now, but just wanted to ask.

@haydn
Copy link
Member Author

haydn commented Nov 15, 2020

@Mxchaeltrxn Thanks for having a bit of a play on your machine. Yeah, I just wanted to check that still worked as expected with your setup.

I agree with both of those Prettier config options and I wish I set them from the start! Tell you what might work — we can use configuration overrides to keep the existing config for the existing files, but set new config for all new files. We can then slowly remove files from the list of exceptions as we work on them until we get to the point where all the files are using our preferred config (we should probably hold off on Editor.js for a bit because they're are lots of active PR's that touch it). I'll push another commit with that change. 👍

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