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

[Discussion] Better Flow setup #21

Closed
radex opened this issue Sep 10, 2018 · 10 comments
Closed

[Discussion] Better Flow setup #21

radex opened this issue Sep 10, 2018 · 10 comments
Labels
help wanted Extra attention is needed

Comments

@radex
Copy link
Collaborator

radex commented Sep 10, 2018

In #19 I fixed support for using Flow in apps importing Watermelon.

But there's a couple of problems:

  1. We can't just export the source code, because we have to rewrite imports first (see [Discussion] Absolute vs relative paths #20), which complicates the build process
  2. Babel screws up the $FlowFixMe comments in the output files, so we have to re-check Flow in an app importing Watermelon
  3. We have to build a separate package for Flow. Use https://babeljs.io/docs/en/babel-plugin-transform-flow-comments maybe to use the same exported code as apps use?
  4. Flow in an app will flow-check the entire Watermelon, which takes more time and is likely to break if there's a mismatch of Flow version used in Watermelon and the app. We could manually write flow-typed, but that's extra maintenance burden.

Can we generate flow-typed automatically from source?.

Or, alternatively: Can apps configure Flow to only import types, but don't actually output errors for Flow issues inside Watermelon?

@radex radex added the help wanted Extra attention is needed label Sep 10, 2018
@jamesisaac
Copy link

Or, alternatively: Can apps configure Flow to only import types, but don't actually output errors for Flow issues inside Watermelon?

The [declarations] config section in Flow 0.76 should allow this: facebook/flow#4916 (PR talks about [silence] but it was renamed to [declarations] just before release).

@radex
Copy link
Collaborator Author

radex commented Sep 11, 2018

Oh, that's so cool! Thanks @jamesisaac. I will check it out on Friday if it actually works :)

@jamesisaac
Copy link

You're welcome! Cool library (just saw it on HN), looking forward to giving it a try. Great to see the focus on Flow / type safety.

@wcandillon
Copy link

My understanding is that [declarations] doesn't work at the moment. Am I correct? I'm also confused on how people are able to use latest versions of Flow successfully with React/React Native. The documentation doesn't seem to indicate any pairing between flow versions and React/RN versions.

The way I use flow is by ignoring type errors that come from dependencies. But this only works in versions of flow that use the old error reporting format. [declarations] seems to be the perfect solution but doesn't work at the moment.

I would love to hear from React developers who are able to use Flow successfully. And I would love to find out if I'm missing something.

@sienkashing
Copy link

I tried using [declarations] to no avail as well. In the end, when it comes to handling third party flow-typed definitions, I resorted to using flow-typed to do away with the headache of managing incompatible flow definitions.

What I normally do:

  1. Ignore .*/node_modules/.* in my .flowconfig file
  2. Run flow-typed install for flow-typed to scan my package.json and install all matching type definitions it has
  3. Run flow-typed crate-stub <packagename> for packages that are not available.
  4. Copy Pasta the type definitions from third party libraries if available into my stubs, or just define the stuff I want type checked as I go along.

Pros:
No more flow errors from third party modules. Yay!

Cons:

  1. When adding new packages, gotta manually run flow-typed install <packagename> or flow-typed create-stub <packagename> if it is not available.
  2. Gotta manually add type definitions you need into the stubs (you can skip this if you don't care about type checking from third party libraries)

I guess this is similar to using Typescript where you will need to define missing index.d.ts in the node-modules/@types folder for packages that don't have them, but at least flow-typed has some nice tooling to help with generating the basic stuff to get you going.

@jamesisaac
Copy link

Hmm yeah weird, [declarations] not seeming effective for me either. It's in the documentation so not sure what the issue is. I've left a comment in the original PR: facebook/flow#4916 (comment)

I think when a project is written with Flow types in the source code itself, there could be benefits to exporting those directly in the repo. They are just guaranteed to be up to date with the code, whereas flow-typed definitions are more of an approximation, not very granular down to individual releases.

I'm also confused on how people are able to use latest versions of Flow successfully with React/React Native. The documentation doesn't seem to indicate any pairing between flow versions and React/RN versions.

I normally go to the branch for the version I'm using on the RN repo, then look at their .flowconfig file, and use the same for my project.

@wcandillon
Copy link

@jamesisaac that's a great tip thks 👍🏻

@radex
Copy link
Collaborator Author

radex commented Sep 19, 2018

I think when a project is written with Flow types in the source code itself, there could be benefits to exporting those directly in the repo. They are just guaranteed to be up to date with the code, whereas flow-typed definitions are more of an approximation, not very granular down to individual releases.

Exactly! This seems so much better than trying to separately maintain the types in an external place...

@stale
Copy link

stale bot commented Dec 18, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Dec 18, 2018
@radex radex added WIP this is being worked on and removed wontfix This will not be worked on labels Dec 18, 2018
@stale
Copy link

stale bot commented Mar 18, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Mar 18, 2019
@radex radex removed WIP this is being worked on wontfix This will not be worked on labels Mar 20, 2019
@radex radex closed this as completed Apr 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants