Add return type to improve type coverage

LoganBarnett commented 6 years ago

The project I work on has a requirement for 100% type coverage. When making my own custom validators, I noticed properties hanging off context weren't something Flow could infer (they become any). Adding this return type is enough for Flow to achieve type coverage with usage of context.

andreypopp commented 6 years ago

Is it still relevant? (I know flow also fixed some bugs with coverage reporting).

LoganBarnett commented 6 years ago

@andreypopp sorry for disappearing! It appears to still be relevant. I'm trying to put together a simple reproduction here. I'm running into a problem though with $NonMaybeType:


✎ yarn flow
yarn run v1.3.2
warning package.json: No license field
$ /Users/logan/dev/validated-context-coverage/node_modules/.bin/flow
Error ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ node_modules/validated/lib/schema.js.flow:158:45

Missing type annotation for $NonMaybeType.

     155│   }
     156│ }
     157│
     158│ export class AnyNode<V: mixed> extends Node<$NonMaybeType<V>> {
     159│   validate(context: Context): ValidateResult<$NonMaybeType<V>> {
     160│     return context.unwrap(value => {
     161│       if (value == null) {

I'm a little puzzled at the moment and haven't been able to put a lot of cycles towards addressing it yet. I suspect it has to do with a recent version of Flow (currently 0.70.0). I will try downgrading and seeing if I can move forward from there.

LoganBarnett commented 6 years ago

Downgrading to flow-bin 0.69.0 got me moving. My fix doesn't seem to work given my reproduction, so I'll need to fix that. You can see the problem on master validated-context-coverage. If you check out coverage-fix and run yarn or npm install you can see the fix not working. I'll be working on that part ;)

LoganBarnett commented 6 years ago

Okay I've updated the PR that adds some type coverage that ultimately provides the needed coverage for the test repo to work at full coverage as well. This is ready again for review.

andreypopp / validated

Add return type to improve type coverage #15