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

Initial Flow support #340

Merged
merged 1 commit into from
Jun 19, 2016
Merged

Initial Flow support #340

merged 1 commit into from
Jun 19, 2016

Conversation

bryannaegele
Copy link
Contributor

@bryannaegele bryannaegele commented Jun 19, 2016

Issue #281

There are currently 59 errors remaining to be fixed for Flow pass weak checks. The vast majority of these are for required modules which do not have a publicly available declaration that I can find.

I pushed this up so you can make some decisions to get this passing and get some help since my time is limited right now and there's a lot of errors to fix. I'm open to anything on moving forward. It may make sense to create a branch in gatsby and we merge this to there.

There are two main resources for type definitions - iflow (which is slowly being deprecated in favor of...) flow-typed. Flow-typed is the new hawtness and official. It uses a CLI to do some really great stuff. Sadly, both are lacking in the number of libraries covered.

For anything not covered, you have to either suppress the warning or "fix" it with your own declaration. These are under interfaces and are covered in the docs https://flowtype.org/docs/third-party.html#_

The declarations can be as good or bad as you want to make them, i.e. they can actually cover pieces of the API you use or they can default to very broad :any or empty object return declarations from what I can surmise.

The other issues are mainly around not having types yet for what is returned by build-pages (wrappers in particular). This could be a good segue into nailing down that interface for that refactor.

And the best part is once you fix some of these Flow will likely find some more. :)

I also added flow ESLint warnings which would be required for flow to run without the weak setting. Running flow suggest for a file will usually give you the first bit of tips for resolving these.

@KyleAMathews
Copy link
Contributor

Hot dang! Looks great! Looking forward to playing with this soon.

@bryannaegele
Copy link
Contributor Author

Updated the initial comment with status of this. There's still much to do and I didn't want to hold anyone up from jumping in to help.

I will say that one really good benefit I see to adding Flow to larger open source projects such as this is how it can drive consistent API and design decisions. Hopefully, it's worth the pain of layering it into an existing project.

@KyleAMathews
Copy link
Contributor

So after thinking about this, I think the best approach moving forward would be to merge this into master but disable flow tests from npm test until we fix all the errors. This way people working on internals could start using Flow to guide implementations of internal APIs while we gradually add interfaces to 3rd-party libraries.

For anyone following along, fixing Flow errors, either internal or with 3rd-party libraries, would be a great & simple way to start contributing.

@KyleAMathews KyleAMathews merged commit d76736c into gatsbyjs:master Jun 19, 2016
@jlengstorf
Copy link
Contributor

Hiya @bryannaegele! 👋

This is definitely late, but on behalf of the entire Gatsby community, I wanted to say thank you for being here.

Gatsby is built by awesome people like you. Let us say “thanks” in two ways:

  1. We’d like to send you some Gatsby swag. As a token of our appreciation, you can go to the Gatsby Swag Store and log in with your GitHub account to get a coupon code good for one free piece of swag. (We’ve got t-shirts and hats, plus some socks that are really razzing our berries right now.)
  2. If you’re not already part of it, we just invited you to join the Gatsby organization on GitHub. This will add you to our team of maintainers. You’ll receive an email shortly asking you to confirm. By joining the team, you’ll be able to label issues, review pull requests, and merge approved pull requests.

If you have questions, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’.

Thanks again! 💪💜

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.

3 participants