fix(Divider|FormInput): fix the broken typings #1179

dennari · 2017-01-18T18:01:48Z

levithomason · 2017-01-18T18:05:07Z

src/collections/Form/index.d.ts

@@ -173,7 +173,7 @@ interface FormGroupProps {

 export const FormGroup: React.ComponentClass<FormGroupProps>;

-interface FormInputProps extends InputProps, ReactFormEvents<HTMLInputElement>, ReactFocusEvents<HTMLInputElement> {
+interface FormInputProps extends InputBaseProps, ReactFormEvents<HTMLInputElement>, ReactFocusEvents<HTMLInputElement> {


Hm, why exclude the onChange prop from the FormInputProps? All Input props are supported in the Form control shorthand components. I may be missing something here.

This is the error I got before:

> tsc 176 interface FormInputProps extends InputProps, ReactFormEvents<HTMLInputElement>, ReactFocusEvents<HTMLInputElement> { ~~~~~~~~~~~~~~ node_modules/semantic-ui-react/dist/commonjs/collections/Form/index.d.ts(176,11): error TS2320: Interface 'FormInputProps' cannot simultaneously extend types 'InputProps' and 'ReactFormEvents<HTMLInputElement>'. Named property 'onChange' of types 'InputProps' and 'ReactFormEvents<HTMLInputElement>' are not identical.

So ReactFormEvents<HTMLInputElement> already includes the onChange prop with an incompatible signature. This is probably not the cleanest possible fix, but it works at least.

I think we can just remove ReactFormEvents<HTMLInputElement> and ReactFocusEvents<HTMLInputElement> there, because we don't expose non SUIR props (#1072).

The signature for this:

<Form.Input onChange={ ... } />

Should be identical to this:

<Input onChange={ ... } />

Wihch is:

onChange(event: React.SyntheticEvent<HTMLInputElement>, data: InputOnChangeData) => void

So the onChange signature from ReactFormEvents<HTMLInputElement> is not correct for this case since it does not include the data argument. I'm not a ts user so I'm not exactly sure of the fix here, however, I wanted to note the actual signatures.

How about adding
onChange, onInput, onSubmit methods to the InputProps and removing extend ReactFormEvents? (If these methods are available)

@layershifter's suggestion seems to me to be the correct one in this case

codecov-io · 2017-01-18T18:08:21Z

Current coverage is 95.88% (diff: 100%)

Merging #1179 into master will not change coverage

@@             master      #1179   diff @@
==========================================
  Files           879        879          
  Lines          4888       4888          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           4687       4687          
  Misses          201        201          
  Partials          0          0

Powered by Codecov. Last update 9fe3fca...11bc3fc

layershifter · 2017-01-19T19:09:13Z

src/elements/Divider/index.d.ts

@@ -34,4 +34,4 @@ interface DividerProps {
  vertical?: boolean;
 }

-export const Container: React.StatelessComponent<DividerProps>;
+export const Divider: React.StatelessComponent<DividerProps>;


Thanks, copy-paste is evil.

layershifter

I've returned onChange on Input typings. LGTM, ready to merge 👍

levithomason · 2017-01-21T15:32:26Z

Will this not throw the same error in FormInput about duplicate onChange props with different signatures?

layershifter · 2017-01-21T17:00:26Z

@levithomason nope, problem was there:

(176,11): error TS2320: Interface 'FormInputProps' cannot simultaneously extend types 'InputProps' and 'ReactFormEvents<HTMLInputElement>'.
  Named property 'onChange' of types 'InputProps' and 'ReactFormEvents<HTMLInputElement>' are not identical.

FormInputProps extended InputProps and ReactFormEvents, while the last two have the same onChange method, but with different signatures. After changes in this PR, FormInputProps will extend only InputProps.

bychwa · 2017-01-23T15:44:04Z

Can someone merge this? Am stuck with this error! :-)

levithomason · 2017-01-24T21:04:09Z

Released in [email protected].

* Semantic-Org/master: (111 commits) fix(docs): fix usage of arrow function (Semantic-Org#1228) fix(Icon): Incorrect definition in typings (Semantic-Org#1225) fix(Button): fix `tabIndex` in typings (Semantic-Org#1224) fix(typings): add item prop to the DropdownProps (Semantic-Org#1223) docs(examples): add missing `key` props (Semantic-Org#1220) docs(changelog): update changelog [ci skip] 0.64.4 feat(Form): add `inverted` prop (Semantic-Org#1218) fix(ComponentExample): use explicit babel presets (Semantic-Org#1219) style(Button): update typings and propTypes usage (Semantic-Org#1216) style(Embed): update typings and propTypes usage (Semantic-Org#1217) chore(package): support for jsnext:main (Semantic-Org#1210) style(Step): update typings, tests and propTypes usage (Semantic-Org#1203) fix(Portal): do not take focus after first render (Semantic-Org#1215) style(Table|mixed): update typings, tests and propTypes usage (Semantic-Org#1200) fix(Dropdown): use `item` instead of `as={Menu.Item}` (Semantic-Org#659) fix(Dropdown): respect `closeOnBlur={false}` (Semantic-Org#1148) style(Breadcrumb): update typings and propTypes usage (Semantic-Org#1209) style(Dimmer): update typings (Semantic-Org#1208) fix(Divider|FormInput): fix the broken typings (Semantic-Org#1179) ...

* Fix the broken typings for FormInput and Divider * Typings: remove non SUIR props from FormIputProps and InputProps * revert(Input): revert removing of `onChange`

Fix the broken typings for FormInput and Divider

4b6189e

dennari mentioned this pull request Jan 18, 2017

The typings for FormInput and Divider are broken #1180

Closed

levithomason reviewed Jan 18, 2017

View reviewed changes

layershifter mentioned this pull request Jan 19, 2017

Problems with typescript definitions #1188

Closed

layershifter reviewed Jan 19, 2017

View reviewed changes

Typings: remove non SUIR props from FormIputProps and InputProps

58055ae

layershifter changed the title ~~Fix the broken typings for FormInput and Divider~~ fix(Divider|FormInput): fix the broken typings Jan 21, 2017

revert(Input): revert removing of onChange

11bc3fc

layershifter approved these changes Jan 21, 2017

View reviewed changes

layershifter added the ready for review label Jan 21, 2017

levithomason merged commit 3b9c90b into Semantic-Org:master Jan 23, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(Divider|FormInput): fix the broken typings #1179

fix(Divider|FormInput): fix the broken typings #1179

dennari commented Jan 18, 2017 •

edited by layershifter

Loading

levithomason Jan 18, 2017

dennari Jan 18, 2017

layershifter Jan 19, 2017

levithomason Jan 20, 2017

rokoroku Jan 20, 2017 •

edited

Loading

dennari Jan 20, 2017

codecov-io commented Jan 18, 2017 •

edited

Loading

layershifter Jan 19, 2017

layershifter left a comment

levithomason commented Jan 21, 2017

layershifter commented Jan 21, 2017

bychwa commented Jan 23, 2017

levithomason commented Jan 24, 2017

fix(Divider|FormInput): fix the broken typings #1179

fix(Divider|FormInput): fix the broken typings #1179

Conversation

dennari commented Jan 18, 2017 • edited by layershifter Loading

levithomason Jan 18, 2017

Choose a reason for hiding this comment

dennari Jan 18, 2017

Choose a reason for hiding this comment

layershifter Jan 19, 2017

Choose a reason for hiding this comment

levithomason Jan 20, 2017

Choose a reason for hiding this comment

rokoroku Jan 20, 2017 • edited Loading

Choose a reason for hiding this comment

dennari Jan 20, 2017

Choose a reason for hiding this comment

codecov-io commented Jan 18, 2017 • edited Loading

Current coverage is 95.88% (diff: 100%)

layershifter Jan 19, 2017

Choose a reason for hiding this comment

layershifter left a comment

Choose a reason for hiding this comment

levithomason commented Jan 21, 2017

layershifter commented Jan 21, 2017

bychwa commented Jan 23, 2017

levithomason commented Jan 24, 2017

dennari commented Jan 18, 2017 •

edited by layershifter

Loading

rokoroku Jan 20, 2017 •

edited

Loading

codecov-io commented Jan 18, 2017 •

edited

Loading