flow-coverage-report icon indicating copy to clipboard operation
flow-coverage-report copied to clipboard

`flow-coverage-report` should work with `// @flow strict` declaration

Open benfjohnson opened this issue 7 years ago • 9 comments

When I run the following command:

flow-coverage-report -i 'src/**/*.js' -i 'src/**/*.jsx' -t html -t json -t text

I get this error for any file with a // @flow strict header:

Error while generating Flow Coverage Report: Error: Unexpected missing flow annotation on src/.../foo.js Error: Unexpected missing flow annotation on src/.../foo.js

When I remove 'strict' from that header comment, the errors moves on to another file with it. This is syntax officially supported by flow (https://flow.org/en/docs/strict/) so expected it to work with this tool.

benfjohnson avatar May 15 '18 21:05 benfjohnson

I actually just submitted a PR for this: #150.

zeorin avatar May 15 '18 21:05 zeorin

Incidentally, using @flow strict-local doesn't cause an error.

zeorin avatar May 15 '18 21:05 zeorin

And that's because there's a bug in a dependency: https://github.com/ryan953/flow-annotation-check/issues/55.

zeorin avatar May 15 '18 21:05 zeorin

Just left a comment on that PR 🙂. I'll plan on using @flow strict-local for now until your fix is merged. Cheers!

benfjohnson avatar May 15 '18 21:05 benfjohnson

I just pushed up v1.8.1-0 of flow-annotation-check, so give that a spin and see if that helps along with #150

ryan953 avatar May 16 '18 07:05 ryan953

@ryan953 thanks for jumping on this so quickly! This works well in the coverage report of my updated PR:

┌──────────┬───────────────────┬─────────┬───────┬─────────┬───────────┐
│ filename │        annotation │ percent │ total │ covered │ uncovered │
│ index.js │ flow strict-local │   100 % │    13 │      13 │ 0         │
└──────────┴───────────────────┴─────────┴───────┴─────────┴───────────┘
┌─────────────────────────┬──────────────────────────────────────────┐
│ included glob patterns: │ **/*.js?(x)                              │
│ excluded glob patterns: │ node_modules/**, **/_book/**             │
│              threshold: │ 100                                      │
│       concurrent files: │ 4                                        │
│           generated at: │ Wed May 16 2018 11:29:00 GMT+0200 (SAST) │
│           flow version: │ 0.72.0                                   │
│      flow check passed: │ yes (0 errors)                           │
└─────────────────────────┴──────────────────────────────────────────┘
┌──────────────┬─────────┬───────┬─────────┬───────────┐
│ project      │ percent │ total │ covered │ uncovered │
│ my-framework │   100 % │    13 │      13 │         0 │
└──────────────┴─────────┴───────┴─────────┴───────────┘

zeorin avatar May 16 '18 09:05 zeorin

Cool, I just pushed that branch up to npm as v1.8.1 for merging.

ryan953 avatar May 16 '18 17:05 ryan953

This can be closed now that 0.6.0 is released. I've tested it and it seems to work.

smirnowld avatar Oct 04 '18 14:10 smirnowld

We just switched our codebase over to @flow strict and verified flow-coverage-report works great with it. Thanks!

LoganBarnett avatar Oct 05 '18 19:10 LoganBarnett