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

feat(Flow): Type definition improvements #222

Merged
merged 4 commits into from
Aug 26, 2017
Merged

feat(Flow): Type definition improvements #222

merged 4 commits into from
Aug 26, 2017

Conversation

bhough
Copy link
Contributor

@bhough bhough commented Jul 30, 2017

  • Updates all modules with explicit return types.
  • Fixes a variety of arity issues with types for modules with optional params.
  • Remove unnecessary suppressions with latest versions of Flow.
  • Moves to standard //$FlowFixMe for zero config suppressions.
  • Updates typing for _statefulSelectors-based modules.
  • Updates to Flow .51
  • Adds /lib to ignore in .flowconfig.

Partially addresses #197

@codecov
Copy link

codecov bot commented Jul 30, 2017

Codecov Report

Merging #222 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #222   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          67     67           
  Lines         370    370           
  Branches      100    100           
=====================================
  Hits          370    370
Impacted Files Coverage Δ
src/internalHelpers/_statefulSelectors.js 100% <ø> (ø) ⬆️
src/shorthands/borderWidth.js 100% <ø> (ø) ⬆️
src/mixins/placeholder.js 100% <ø> (ø) ⬆️
src/mixins/retinaImage.js 100% <ø> (ø) ⬆️
src/color/shade.js 100% <ø> (ø) ⬆️
src/shorthands/borderColor.js 100% <ø> (ø) ⬆️
src/mixins/selection.js 100% <ø> (ø) ⬆️
src/mixins/clearFix.js 100% <ø> (ø) ⬆️
src/color/mix.js 100% <ø> (ø) ⬆️
src/shorthands/buttons.js 100% <ø> (ø) ⬆️
... and 23 more

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 2753236...b436e4c. Read the comment docs.

@codecov
Copy link

codecov bot commented Jul 30, 2017

Codecov Report

Merging #222 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #222   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          68     68           
  Lines         388    386    -2     
  Branches      101    102    +1     
=====================================
- Hits          388    386    -2
Impacted Files Coverage Δ
src/mixins/ellipsis.js 100% <ø> (ø) ⬆️
src/color/opacify.js 100% <ø> (ø) ⬆️
src/internalHelpers/_nameToHex.js 100% <ø> (ø) ⬆️
src/shorthands/animation.js 100% <ø> (ø) ⬆️
src/color/mix.js 100% <ø> (ø) ⬆️
src/internalHelpers/_hslToHex.js 100% <ø> (ø) ⬆️
src/internalHelpers/_statefulSelectors.js 100% <ø> (ø) ⬆️
src/shorthands/buttons.js 100% <ø> (ø) ⬆️
src/mixins/triangle.js 100% <ø> (ø) ⬆️
src/color/shade.js 100% <ø> (ø) ⬆️
... and 35 more

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 b776076...e9b45aa. Read the comment docs.

@bhough bhough requested review from mxstbr and nikgraf July 30, 2017 05:54
typeof color.red === 'number' &&
typeof color.green === 'number' &&
typeof color.blue === 'number' &&
// $FlowIgnoreNextLine not sure why this complains
typeof color.alpha !== 'number'
(typeof color.alpha !== 'number' || typeof color.alpha === 'undefined')
Copy link
Contributor

Choose a reason for hiding this comment

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

 || typeof color.alpha === 'undefined'

Does this just silence the Flow error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, though I'm still working my way through this file, as it is the sole one left keeping us from having full flow support. It is proving...troublesome.

@bhough bhough merged commit 6e8e9a5 into master Aug 26, 2017
@bhough bhough deleted the flow-improvements branch August 26, 2017 01:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants