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

flow-coverage-report should work with // @flow strict declaration #151

Open
benfjohnson opened this issue May 15, 2018 · 9 comments
Open

Comments

@benfjohnson
Copy link

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.

@zeorin
Copy link
Contributor

zeorin commented May 15, 2018

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

@zeorin
Copy link
Contributor

zeorin commented May 15, 2018

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

@zeorin
Copy link
Contributor

zeorin commented May 15, 2018

And that's because there's a bug in a dependency: ryan953/flow-annotation-check#55.

@benfjohnson
Copy link
Author

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

@ryan953
Copy link
Contributor

ryan953 commented May 16, 2018

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

@zeorin
Copy link
Contributor

zeorin commented May 16, 2018

@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 │
└──────────────┴─────────┴───────┴─────────┴───────────┘

@ryan953
Copy link
Contributor

ryan953 commented May 16, 2018

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

@smirnowld
Copy link

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

@LoganBarnett
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants