Make the generated app work if you flowcheck it

It would be nice if it were possible to add flow to the default project. I tried a minimal flow setup, adding a blank .flowconfig to the root and // @flow to App.css. Four errors:

For the first two I might be configuring things wrong? The second two seem like flow doesn't know how to handle the non-js imports that webpack is handling for us. I'm not sure what the right way to resolve this.

I was using flow version 0.29.0

I think the first two are fine if we setup .*/node_modules/.* as an ignored path in .flowconfig. The second two require a name mapper: https://flowtype.org/docs/modules.html#css-modules-with-webpack

Indeed our case we’d need to map .css to modules that don’t export anything, and all other files (e.g. images) to modules that export strings. This is similar to what React Native does.

I am, however, not very happy with creating configs on user’s behalf right in their directory even if they don’t use Flow. The point of this project is that there is no user configuration, at least for as long as you can avoid it.

What if flow init could somehow “autodiscover” our initial config, and use it instead of the default one? This way we wouldn’t create any files by default, but when the user runs Flow for the first time, they get it correctly preconfigured. Since this is done on demand, we have a chance to keep improving the initial config, and even people who created apps a long time ago would get an up-to-date initial config when they start using Flow.

Add Flow stubs

As a temporary workaround, I added these stubs: 6885094.
Now I see no errors with a config like this:

[libs]
./node_modules/fbjs/flow/lib

[options]
esproposal.class_static_fields=enable
esproposal.class_instance_fields=enable

module.name_mapper='^\(.*\)\.css$' -> 'react-scripts/config/flow/css'
module.name_mapper='^\(.*\)\.\(jpg\|png\|gif\|eot\|svg\|ttf\|woff\|woff2\|mp4\|webm\)$' -> 'react-scripts/config/flow/file'

suppress_type=$FlowIssue
suppress_type=$FlowFixMe

And I wish this config was inside react-scripts somehow, at least until flow init.

What about having a command like npm run flow which adds that config and does flow init? Maybe too much?

I’d like something like npm run add flow, npm run add relay, etc, for the future.
For now I’ll document this workaround in How To.

Eject Flow stubs

I added some instructions to the howto guide for now: https://github.com/facebookincubator/create-react-app/blob/master/template/README.md#adding-flow

Longer term, I think we should include Flow by default and Flow should have a way of extending configs like

import 'react-scripts/configs/flow'

for basic settings.

Eject Flow stubs

I would love flow being included by default 👍. Does that mean that we would also get flow errors in the chrome console and terminal? Flow helps me a lot with fixing bugs in javascript.

People that don't like flow (for some reason) probably wouldn't even notice it, as they don't include the /* @flow */ comment on top of their file.

@gaearon

So there are a number of tasks that we have created in order to simplify the setup of Flow in create-react-app.

Right now the .flowconfig that you need to add to get started is a bit difficult. As @gaearon mentioned before it looks like this:

[libs]
./node_modules/fbjs/flow/lib

[options]
esproposal.class_static_fields=enable
esproposal.class_instance_fields=enable

module.name_mapper='^\(.*\)\.css$' -> 'react-scripts/config/flow/css'
module.name_mapper='^\(.*\)\.\(jpg\|png\|gif\|eot\|svg\|ttf\|woff\|woff2\|mp4\|webm\)$' -> 'react-scripts/config/flow/file'

suppress_type=$FlowIssue
suppress_type=$FlowFixMe

There's a couple things we want to do to get it down to zero config.

Ignoring node_modules by default, and have a happy path for installing flow-typed definitions. This eliminates the need to have the path in [libs] and the surpress_type's.
Having sensible defaults for different file extensions. We can't support 100% code coverage on something like css-modules but we can support the way all these files are used within create-react-app. This eliminates the module.name_mapper fields.
Potentially enable >= stage 2 esproposals by default.

We also want to improve the experience of Flow other than the initial setup.

Integrate with npm start for display type warnings like eslint does right now.
Hook into npm install for adding flow-typed definitions automagically without pain (or maybe some other solution than npm install, idk yet)

We discussed allowing the repo to have no .flowconfig file, but having this constraint improves the text editor experience so we're punting on that for now.

As for enabling Flow in create-react-app projects themselves, we would be interested in seeing that happen. As for if it's enabled on all files by default is up to the community to decide on what they want most. We don't want to force ourselves on anyone, but adding // @flow on the top of every file is tedious.

If anyone has questions about this let me know, @gabelevi and I will be working through these.

Integrate with npm start for display type warnings like eslint does right now.

😍😍😍 There is someone already investigating this I believe:
#324

Hook into npm install for adding flow-typed definitions automagically without pain (or maybe some other solution than npm install, idk yet)

😍😍😍

We don't want to force ourselves on anyone, but adding // @flow on the top of every file is tedious.

This is actually what I do in all my flow projects, is there another way?

We don't want to force ourselves on anyone, but adding // @flow on the top of every file is tedious.

This is actually what I do in all my flow projects, is there another way?

Yes, you can add --all to the cli command or all=true to your .flowconfig under [options]

I find it tedious to put // @flow at the top of files, but I also find it tedious when I run Flow with --all and it complains about things that aren't really problems. It's especially bothersome if that's someone's first experience with Flow.

What we ended up doing with eslint in create-react-app is configuring things in a very conservative mode, only reporting problems in cases where we can be highly confident it's a real problem. Would something like that be possible with Flow?

Also I would like to volunteer to help out testing any development build with a "flow defaults to on" experience!

I'd definitely be interested. There is a PR with a proof of concept in #350. I reached out to the Flow team several times asking for their feedback on the Webpack loader that this PR used but unfortunately they never got back to me.

If you're interested in taking this proof of concept and making it production-ready (maybe looking at other official Flow integrations and taking lessons from them) this is a good contribution opportunity.

This also means we would provide a default .flowconfig

Nah. We'd create it from the loader the first time it sees a flow annotation.

@gaearon

@gaearon Well, I'll take a look at all of this... One last question, so if you don't want the .flowconfig right away, I imagine, we'll only download them from the loader if we see a flow annotation as well, right?

@gaearon

@gaearon I don't know either how the flow team would approach this problem, but I would work around flow-bin and not use a loader. Flow works project-wide and, to me, it doesn't make much sense as a loader. Maybe a webpack plugin would work, but I want to see first where I can go without it...

That way we only depend on flow-bin, a facebook-supported dependency with custom code wrapping its execution in an another facebook-supported project. So you don't end up depending on my weird custom package or whatever...

@rricard

@rricard It being a loader would help integrating into npm start IMO. I don't advocate it actually behaving like a loader, but rather using that as an opportunity to know when to trigger re-checks. You'd still debounce its calls and wouldn't really use the sources.

But yea, WP plugin could work too. As long as you have a way to know when to trigger re-checks (or does Flow do this automatically anyway? I have no idea.)

@gaearon

@gaearon I absolutely see why a loader has its advantages. Plugins in my opinion would work too.

I don't know when flow updates exactly but once it's started, you just ask for the typechecking results and it's right there with no delay. So the question is when to add the flow errors to the output.

@rricard

@rricard And when to "re-trigger" the output due to Flow even if webpack didn't re-trigger it by itself.

@gaearon

@gaearon Starting to see a bit more precisely what is going on here. I'll try something using flow-bin and a plugin that I'll place in react-dev-utils. From there, I think I'll be able to get you a POC and probably something publishable soon after. If the plugin approach doesn't work, i think that creating a loader from my plugin attempt will not be hard.

Cool, thanks for looking at it!

Going to close as it should be fine now.

Navigation Menu

Repository navigation

Make the generated app work if you flowcheck it #72

27 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Make the generated app work if you flowcheck it #72

Description

Activity

keyz commented on Jul 22, 2016

gaearon commented on Jul 22, 2016

gaearon commented on Jul 22, 2016

mxstbr commented on Jul 22, 2016

gaearon commented on Jul 22, 2016

gaearon commented on Jul 22, 2016

kasperpeulen commented on Jul 22, 2016

jamiebuilds commented on Aug 2, 2016

kasperpeulen commented on Aug 2, 2016

jamiebuilds commented on Aug 2, 2016

lacker commented on Aug 2, 2016

27 remaining items

gaearon commented on Nov 20, 2016

gaearon commented on Nov 20, 2016

rricard commented on Nov 20, 2016

rricard commented on Nov 20, 2016

rricard commented on Nov 20, 2016

gaearon commented on Nov 20, 2016

gaearon commented on Nov 20, 2016

rricard commented on Nov 20, 2016

rricard commented on Nov 20, 2016

gaearon commented on Nov 20, 2016

rricard commented on Nov 20, 2016

gaearon commented on Nov 20, 2016

gaearon commented on Feb 11, 2017

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions