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

test(typings): initial tests for typings #1611

Merged
merged 1 commit into from
Apr 24, 2017
Merged

test(typings): initial tests for typings #1611

merged 1 commit into from
Apr 24, 2017

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented Apr 23, 2017

We have constant problems with typings (#1609 and more), it's time to solve them ✌️


This PR replaces #1181 and offers initial tests for typings. What it does:

  • tests that typings file Component.d.ts exists for each component;
  • tests that definition file has ComponentProps interface and it's exported;
  • tests that definition contains all props from Component.propTypes and props that are required in definitions are really required.

Also, it fixes all found problems 🥂 Our typiscript users will be satisfied 👍

@@ -64,7 +64,7 @@ export { default as Flag, FlagProps } from './dist/commonjs/elements/Flag'

export { default as Header, HeaderProps } from './dist/commonjs/elements/Header'
export { default as HeaderContent, HeaderContentProps } from './dist/commonjs/elements/Header/HeaderContent'
export { default as HeaderSubheader, HeaderSubHeaderProps } from './dist/commonjs/elements/Header/HeaderSubheader'
export { default as HeaderSubheader, HeaderSubheaderProps } from './dist/commonjs/elements/Header/HeaderSubheader'
Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed interface name

@@ -15,6 +15,9 @@ export interface BreadcrumbSectionProps {
/** Additional classes. */
className?: string;

/** Shorthand for primary content. */
content?: any;

Copy link
Member Author

Choose a reason for hiding this comment

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

Missing definition

@codecov-io
Copy link

codecov-io commented Apr 23, 2017

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1611   +/-   ##
=======================================
  Coverage   99.74%   99.74%           
=======================================
  Files         141      141           
  Lines        2398     2398           
=======================================
  Hits         2392     2392           
  Misses          6        6
Impacted Files Coverage Δ
src/collections/Message/Message.js 100% <ø> (ø) ⬆️
src/views/Item/Item.js 100% <ø> (ø) ⬆️
src/modules/Checkbox/Checkbox.js 100% <ø> (ø) ⬆️
src/elements/Step/StepTitle.js 100% <ø> (ø) ⬆️
src/elements/Step/StepContent.js 100% <ø> (ø) ⬆️
src/elements/Button/Button.js 100% <ø> (ø) ⬆️
src/modules/Dropdown/Dropdown.js 100% <ø> (ø) ⬆️
src/elements/Step/StepDescription.js 100% <ø> (ø) ⬆️
src/elements/Image/Image.js 100% <ø> (ø) ⬆️

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 54e490e...14915fb. Read the comment docs.

@@ -45,6 +45,8 @@ describe('Modal', () => {
})

common.hasSubComponents(Modal, [ModalHeader, ModalContent, ModalActions, ModalDescription])
common.hasValidTypings(Modal)
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this might already be covered by the common.isConformant() test, true?

Copy link
Member Author

@layershifter layershifter Apr 24, 2017

Choose a reason for hiding this comment

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

Modal-test.js doesn't contain isConformant test

Copy link
Member

Choose a reason for hiding this comment

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

Of course, thanks!

@levithomason
Copy link
Member

Released in [email protected].

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

Successfully merging this pull request may close these issues.

3 participants