Skip to content

Add formik to the user test suite #20926

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

Closed
DanielRosenwasser opened this issue Dec 28, 2017 · 9 comments
Closed

Add formik to the user test suite #20926

DanielRosenwasser opened this issue Dec 28, 2017 · 9 comments
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue Infrastructure Issue relates to TypeScript team infrastructure

Comments

@DanielRosenwasser
Copy link
Member

It seems that #20891 affects users of Formik, a React forms library.

It'd be good to have
some extra coverage https://github.com/jaredpalmer/formik

@DanielRosenwasser DanielRosenwasser added the Infrastructure Issue relates to TypeScript team infrastructure label Dec 28, 2017
@weswigham
Copy link
Member

@DanielRosenwasser Do you have an actual TS example project for that library? I don't think the reported issue surfaces when just checking the .d.ts file.

@DanielRosenwasser
Copy link
Member Author

I think both of the two examples will error

// (1) - explicit parameter annotation
interface FormTypes { firstName: string }
let el1 = <Formik
    initialValues={{ firstName: "Daniel"}}
    onSubmit={(values: FormTypes) => console.log("First name", values.firstName)} />


// (2) - parameter should be inferred, ends up as {}
let el2 = <Formik
    initialValues={{ firstName: "Daniel"}}
    onSubmit={values => console.log("First name", values.firstName)} />

@sandersn
Copy link
Member

sandersn commented Jan 2, 2018

If the only value of adding formik is as an end-to-end equivalent to the test for a fix to #20891, then I don't think it's worth it. Are there other properties of formik as a dependency that make it useful?

@jaredpalmer
Copy link

@sandersn IMHO this is more of a DX thing. On the one hand, adding tests for Apollo, Formik, etc might seem redundant. On the other hand, seems like a good idea to guarantee that the most popular libraries on GitHub are always compatible with the latest version of TypeScript (and are using the latest and greatest TS features).

@sandersn
Copy link
Member

sandersn commented Jan 3, 2018

Good point. I do think that we’ll need more code than our typical import x from "library" for this test.

@DanielRosenwasser DanielRosenwasser added the Bug A bug in TypeScript label Jan 18, 2018
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 2.7.1 milestone Jan 18, 2018
@mhegazy
Copy link
Contributor

mhegazy commented Jan 19, 2018

@sandersn are the two issues (jaredpalmer/formik#283 and jaredpalmer/formik#314) fixed in master now? or we are still breaking formik?

@mhegazy
Copy link
Contributor

mhegazy commented Jan 19, 2018

@weswigham can we ensure formik runs clean on master after #20995 is merged.

@sandersn sandersn removed their assignment Jan 22, 2018
@weswigham
Copy link
Member

This is in our user suite in master now. Provided nothing has changed since the PR, it's in a state where the .d.ts itself typechecks, but the example component does not.

@weswigham
Copy link
Member

formik typechecks nowadays, so I'm going to close this. 👍

@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Mar 3, 2018
@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue Infrastructure Issue relates to TypeScript team infrastructure
Projects
None yet
Development

No branches or pull requests

5 participants