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

Allow Flow build-time type checking #5995

Closed
gino-m opened this issue Dec 8, 2018 · 6 comments
Closed

Allow Flow build-time type checking #5995

gino-m opened this issue Dec 8, 2018 · 6 comments

Comments

@gino-m
Copy link

gino-m commented Dec 8, 2018

Would it be possible to enable Flow type checks at build time (e.g., like this) without requiring an eject + custom webpack config (#1592)? Opening this issue since #4097 was flagged as a question instead of an feature request, and closed without much explanation.

It seems a pity that atm we can't get both the up-to-date integration goodness of cra, and the static type safety of Flow*. It would be a big win if this were supported, imo. Thanks!

*I assuming manual build steps don't count as part of the build process, since they're optional and entirely up to the developer to know about and execute.

@gino-m gino-m changed the title Allow Flow build-time type checking plugin to be enabled in webpack Allow Flow build-time type checking Dec 8, 2018
@bugzpodder
Copy link

You can run flow CLI to check for errors as a separate step, just like you do with jest tests. Most editors have plugins that shows flow errors inline within the editor, which is much more helpful than a console message from webpack.

@bugzpodder bugzpodder reopened this Dec 9, 2018
@gino-m
Copy link
Author

gino-m commented Dec 10, 2018

This assumes that we want to treat type errors as if they're lint warnings. What if we would like to elevate them to compile errors (or at least simulate the same behavior), as would happen if the type system were built into ES6 or Babel? My goal is approximate as closely as possible language-level type safety so that we can rely on the compiler to do type checking, instead of assuming the developer will always check for errors in their editor or via CLI command. Thanks for your help!

@bugzpodder
Copy link

bugzpodder commented Dec 10, 2018

Personally, I have a CI setup that runs jest tests and flow check, and they can also be added as a pre-commit hook.

I feel like enforcing build failures on flow errors is a bit stringent. While they should be enforced when code is checked in (just like how we don't want test failures), enforcing them before development builds might be a bit too much.
Even though there could be a flow error due to type mismatch, the program could still build/run successfully and even correctly. Also, flow errors are very dependent on the version of the flow installed. A perfectly valid program for flow 0.7 may result in tons of errors for flow 0.8.
Also, I think its useful to prototype something quickly in code without worrying about flow errors breaking the build.

@bugzpodder
Copy link

Also, flow itself has tons of issues with invalid library definitions and even react definitions itself:
facebook/flow#7219
This for example will prevent anyone from adopting React.lazy syntax with flow enabled. The only way to disable it to add $FlowFixMe before every React.lazy call...

@holloway
Copy link
Contributor

If you want compile errors then modify package.json like,

"build": "yarn flow && react-scripts build",

Build errors + editor Flow integration gets you most of the way there.

On the broader point of Flow not keeping up, I'm finding that too, sadly.

@gino-m
Copy link
Author

gino-m commented Dec 11, 2018

Personally, I have a CI setup that runs jest tests and flow check, and they can also be added as a pre-commit hook.

Also, I think its useful to prototype something quickly in code without worrying about flow errors breaking the build.

@bugzpodder I think this makes sense. It may be biases because I've spent a lot more time working with statically typed languages which enforce typing at compile-time. As long as we have types enforced before code is submitted, we'd get the best of both worlds: rapid prototyping, and type safety. I'll set up commit hooks instead. Closing this issue for now.

@holloway Thanks for this, I'll use this to add compile checks to builds to ensure we don't accidentally deploy to test servers w/o validating types.

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

No branches or pull requests

3 participants