Skip to content
This repository was archived by the owner on Aug 1, 2018. It is now read-only.

Improve Flow type definitions for exported functions#93

Merged
alxmyth merged 1 commit intofusionjs:masterfrom
alxmyth:improve-type-definitions
Apr 16, 2018
Merged

Improve Flow type definitions for exported functions#93
alxmyth merged 1 commit intofusionjs:masterfrom
alxmyth:improve-type-definitions

Conversation

@alxmyth
Copy link
Copy Markdown
Member

@alxmyth alxmyth commented Apr 12, 2018

Fixes #685

#### Problem/Rationale

Fusion.js packages, with the exception of `fusion-core` do not export [libdef](https://flow.org/en/docs/libdefs/) and consumers rely on the Flow type definitions embedded in the source code.  Given this, it is a pain point for consumers when type definitions are missing or incomplete.

We should also strive for full Flow coverage to minimize issues within our own packages.

#### Solution/Change/Deliverable

Two-fold:
* Reach 100% Flow coverage on exported type definitions.
* Strive for 100% Flow coverage internally for each package.

screen shot 2018-04-12 at 2 00 16 pm

(compare to before)

@alxmyth alxmyth self-assigned this Apr 12, 2018
@alxmyth
Copy link
Copy Markdown
Member Author

alxmyth commented Apr 12, 2018

Given the lower test coverage here, I opted to type the exports with the types identified in README.md. Some of these may need to change to meet edge cases, and so I avoided investing time in typing this entire package.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 12, 2018

Codecov Report

Merging #93 into master will increase coverage by 0.62%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #93      +/-   ##
==========================================
+ Coverage    77.3%   77.93%   +0.62%     
==========================================
  Files           8        9       +1     
  Lines         141      145       +4     
  Branches       36       36              
==========================================
+ Hits          109      113       +4     
  Misses         21       21              
  Partials       11       11
Impacted Files Coverage Δ
src/middleware.js 20% <100%> (+20%) ⬆️
src/index.js 100% <100%> (ø)
src/dispatched.js 12.5% <0%> (-12.5%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 63a4c2d...d9fbc1d. Read the comment docs.

Comment thread src/index.js
@@ -2,15 +2,54 @@
*
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the relevant changes take place in this file.

Copy link
Copy Markdown
Contributor

@KevinGrandon KevinGrandon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting approach! Happy to try this for now.

@alxmyth alxmyth merged commit 06f2ad5 into fusionjs:master Apr 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants