Show flow issues together with eslint issues
kasperpeulen opened this issue · 20 comments
I was wondering if better flow integration is something you guys consider.
Flow finally has Windows support: 🎉
https://www.flowtype.org/blog/2016/08/01/Windows-Support.html
I often notice flow issues too late, when I work on a create react app. The eslint issues are always in my face at the terminal, at the console, and in my editor (Webstorm). So I never miss out on them.
However, for flow issues, I have to run flow manually. Sadly, Webstorm doesn't show any flow issues. And also this project doesn't show any flow issues to me. Sometimes I find a bug after quite some time, and then I notice that flow would have already seen this for ages, but I just forgot to run flow.
I would love it if the flow issues are also seen in the terminal and console.
I would love this as well, especially if we can make it run in a way that is unobtrusive for people who are not using Flow.
I also would love this but I’m not totally sure how to approach this best. Proof of concept is welcome if you'd like to work on one.
@gaearon I would like to work on a POC. One question though. Do we want to include flow-bin or look for the global flow?
@torifat I think including flow-bin is the way to go. Note that I have close to zero experience with it.
@gaearon no problem. We can revert it easily if any flow veteran suggests otherwise in the future 😄
@gaearon It seems we need a .flowconfig in the root (template) directory. There is a open related issue.
For now this is the only way: https://github.com/facebookincubator/create-react-app/blob/master/template/README.md#add-flow
But let’s skip that and focus on integration. We can figure out how to “provide” the config later.
I was looking at the current lint code and found that we are using eslint-loader. So, it would be great if we can follow the similar approach for flow too. I was looking for a flow-loader and found this. But, this one doesn't pass any error or warning to the emitter and also it's not a loader.
In flow there's an option for getting json output of any command like flow status --json. So, I think I can make a separate flow-loader and integrate it here.
@gaearon what's your thought?
Related Issues:
This sounds reasonable.
@torifat - The --json output doesn't include the pretty-printed error, but I have written some JS to handle consuming the JSON output and pretty printing it. I really need to stick it into a npm package, but until then it lives here: https://github.com/facebook/flow/blob/master/tsrc/flowResult.js.
UPDATE
I have already started working with the loader: https://github.com/torifat/flowtype-loader
Any kind of idea/suggestions are welcome 😄
Hopefully I will be able to finish it within tomorrow.
@jntn It's working, you can check it by pulling my branch. I'm also waiting for feedbacks.
Any update on this?
@thejameskyle contacted @torifat, waiting for them to come up with a plan. Flow team is definitely interested in this and will pursue making this happen.
I'm working on an eslint plugin that reports flow errors as eslint errors:
@amilajack Awesome, I hope something like that will be used in create react app
As a stop-gap measure while this is being worked on, I came across this: https://www.npmjs.com/package/flow-watch. It's one more thing to kick off from the command line, but it alerts to errors at save time.
We're probably not going to pursue this. It doesn't seem like the Flow team is actively interested in such integrations, and trying to do something like this is like swimming against the current while APIs change.