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

style(typings): export all props interfaces #1467

Merged
merged 5 commits into from
Mar 24, 2017
Merged

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented Mar 15, 2017

Fixes #1456.

@codecov-io
Copy link

codecov-io commented Mar 15, 2017

Codecov Report

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

@@           Coverage Diff           @@
##           master    #1467   +/-   ##
=======================================
  Coverage   99.74%   99.74%           
=======================================
  Files         141      141           
  Lines        2363     2363           
=======================================
  Hits         2357     2357           
  Misses          6        6

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 8d05722...d74e561. Read the comment docs.

@codecov-io
Copy link

codecov-io commented Mar 17, 2017

Codecov Report

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

@@           Coverage Diff           @@
##           master    #1467   +/-   ##
=======================================
  Coverage   99.74%   99.74%           
=======================================
  Files         141      141           
  Lines        2368     2368           
=======================================
  Hits         2362     2362           
  Misses          6        6

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 67c737f...ac8e190. Read the comment docs.

@layershifter
Copy link
Member Author

Almost done there, I want to check it with my typescript pet-project in evening.

@layershifter
Copy link
Member Author

@mihai-dinculescu @rokoroku @DanHarman I will be glad to hear your feebback there.

@rokoroku
Copy link
Contributor

Looks good 👍

@mihai-dinculescu
Copy link
Member

mihai-dinculescu commented Mar 17, 2017

I am now able to do

import { IconProps } from 'semantic-ui-react';

but I'm not able to do

import { Icon } from 'semantic-ui-react';

Same goes for the other components.

Thou I'm not sure that I have taken the right approach in testing. I have built the branch and copied dist and index.d.ts to my project's node_modules/semantic-ui-react folder.

@layershifter
Copy link
Member Author

@mihai-dinculescu I've added commit that updates exports again. Can you check?

I've tested with compiler in my project, project was built successfully without errors.

@levithomason
Copy link
Member

Changes look good, will merge once the fixes are confirmed.

@mihai-dinculescu
Copy link
Member

mihai-dinculescu commented Mar 20, 2017

@layershifter the TypeScript part works well but there's a React warning now.

Warning: Invalid argument supplied to oneOfType, expected an instance of array.

It seems to come out of ButtonOr.js:36

text: PropTypes.oneOfType(PropTypes.number, PropTypes.string)

[ ] is missing.

@layershifter
Copy link
Member Author

This was fixed in #1477.

@mihai-dinculescu
Copy link
Member

mihai-dinculescu commented Mar 20, 2017

@layershifter Then all is good!

@layershifter
Copy link
Member Author

@levithomason Seems we can merge this 👍

@Jameskmonger
Copy link

Thanks for this @layershifter, it is currently blocking me.

@levithomason levithomason merged commit 1493c55 into master Mar 24, 2017
@levithomason levithomason deleted the style/typings-props branch March 24, 2017 20:29
@levithomason
Copy link
Member

Very appreciative to everyone who tested and reported back here!

@levithomason
Copy link
Member

Released in semantic-ui-react@0.67.2.

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.

6 participants