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

Fix form types and add type test #7245

Merged
merged 1 commit into from Aug 18, 2017
Merged

Fix form types and add type test #7245

merged 1 commit into from Aug 18, 2017

Conversation

@mention-bot
Copy link

@yesmeck, thanks for your PR! By analyzing the history of the files in this pull request, we identified @simaQ, @afc163 and @benjycui to be potential reviewers.

@afc163
Copy link
Member

afc163 commented Aug 17, 2017

What is the benefits?

@yesmeck
Copy link
Member Author

yesmeck commented Aug 17, 2017

@afc163 You can try compile the test file using the old type.

@yesmeck yesmeck force-pushed the form-create-type branch 2 times, most recently from 3a1833f to f7da361 Compare August 17, 2017 12:14
@afc163
Copy link
Member

afc163 commented Aug 17, 2017

components/form/__tests__/type.test.tsx(38,24): error TS2339: Property 'name' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<{}, ComponentState>> & Readonly<{ childr...'.

Just for record.

@yesmeck yesmeck force-pushed the form-create-type branch 2 times, most recently from 24c2948 to b3a8cf7 Compare August 18, 2017 05:47
@afc163
Copy link
Member

afc163 commented Aug 18, 2017

rebase

@codecov
Copy link

codecov bot commented Aug 18, 2017

Codecov Report

Merging #7245 into master will increase coverage by 2.55%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7245      +/-   ##
==========================================
+ Coverage   85.73%   88.29%   +2.55%     
==========================================
  Files         231      231              
  Lines        4893     4946      +53     
  Branches     1332     1412      +80     
==========================================
+ Hits         4195     4367     +172     
+ Misses        698      579     -119
Impacted Files Coverage Δ
components/form/Form.tsx 89.09% <ø> (ø) ⬆️
components/table/Table.tsx 94.13% <0%> (+0.9%) ⬆️
components/transfer/list.tsx 91.11% <0%> (+1.11%) ⬆️
components/table/SelectionCheckboxAll.tsx 93.82% <0%> (+1.23%) ⬆️
components/back-top/index.tsx 94.28% <0%> (+1.42%) ⬆️
components/tabs/index.tsx 95.38% <0%> (+1.53%) ⬆️
components/radio/group.tsx 93.22% <0%> (+1.69%) ⬆️
components/date-picker/createPicker.tsx 87.27% <0%> (+1.81%) ⬆️
components/alert/index.tsx 82.35% <0%> (+1.96%) ⬆️
components/breadcrumb/Breadcrumb.tsx 86.66% <0%> (+2.22%) ⬆️
... and 26 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 f089d18...0fe8e7b. Read the comment docs.

@yesmeck
Copy link
Member Author

yesmeck commented Aug 18, 2017

😳这个覆盖率。。。

@benjycui benjycui merged commit 145ed77 into master Aug 18, 2017
@benjycui benjycui deleted the form-create-type branch August 18, 2017 09:21
@afc163
Copy link
Member

afc163 commented Aug 18, 2017

没道理。

@mitchelldemler
Copy link

mitchelldemler commented Aug 21, 2017

This is a breaking change for me.

I am using types/react@15.0.24 (upgrading react types breaks my build when importing antd components)

How I am currently implementing FormComponentProps:

import * as React from 'react';
import { Form } from 'antd';
import { FormComponentProps } from 'antd/lib/form/Form';

export interface IOwnProps {
    prop1: string;
    prop2: string;
}

class LoginFormComponent extends React.Component<IOwnProps & FormComponentProps, {}> {
    public render() {
        return (
            <Form />
        );
    }
}

export default Form.create<IOwnProps>({})(LoginFormComponent);

(I'm sorry I don't know how to enforce types in Codepen)

When upgrading to antd 2.12.7 I get 'Property 'form' is missing in type...' when consuming the Form:

<LoginForm prop1="aaaa" prop2="bbbb" />

@yesmeck
Copy link
Member Author

yesmeck commented Aug 21, 2017

@miaojiuchen Try interface IOwnProps extends FormComponentProps.

@mitchelldemler
Copy link

Same error. I will try upgrade my project to use latest types. Can you confirm what version of react types you are using?

@yesmeck
Copy link
Member Author

yesmeck commented Aug 22, 2017

Oh. export default Form.create<IOwnProps>({})(LoginFormComponent);

You don't need pass IOwnProps to Form.create.

@mitchelldemler
Copy link

No luck :(

@yesmeck
Copy link
Member Author

yesmeck commented Aug 22, 2017

I'm using @types/react@15.6.1

@rdvojmoc
Copy link

I get same error as @mitchelldemler when trying to consume my form:

import { Row, Col, Spin, Card, Badge, Button, Input, Form } from 'antd'
import { FormComponentProps } from 'antd/lib/form';

interface IContainerProps extends FormComponentProps {
    container: ContainerViewModel;
}
 class ContainerDetail extends React.Component<IContainerProps, any> {
    public render() {
        return <Card>
        <Spin >
            <Form layout="horizontal">
                <Form.Item label="Name:">
                     <Input defaultValue={this.props.container.Name} />
                </Form.Item>
                <Form.Item label="Description:">
                    {this.props.form.getFieldDecorator('', { initialValue: this.props.container.Description })}
                    (
                        <Input />
                    )
                </Form.Item>
            </Form>
        </Spin>
    </Card>;
    }
}

export default Form.create<IContainerProps>()(ContainerDetail);

When I try to consume ContainerDetail form:
<ContainerDetail container={this.props.container}/>;

Error thrown:
Property 'form' is missing in type '{ container: ContainerViewModel; }'.

Libraries used:

"typescript": "2.4.1",
"antd": "^3.1.0",
"@types/react": "16.0.34"
"react": "16.2.0",

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

6 participants