Strongly Typed Fields

[johnrom]

Note: Formik no longer appears to be maintained, but I am leaving this PR open for archival purposes.

closes #1334

Added strongly typed field opt-in by exposing Fields typed proxy via the FormikContext.

Edit: this PR now simply adds strong types to the fields, with the proxy moved to another package.

[johnrom]

FYI I'm thinking this doesn't go 100% of the way, as I don't think any of the handleChange interfaces, for example, are typed. I do plan to add that functionality as well when time permits.

[jaredpalmer]

Can we dogfood this in userland without making changes to Formik? eg. make import { Field } from 'typed-field'?

[johnrom]

I think the only way for that to make sense would be to fork the entire project into typed-formik. I would not want to maintain a separate strongly typed definition of the fields that will break when you push 2.0. What I've done is strengthen the types of <Field /> itself while leaving the default export Field to function like it does currently, seamlessly integrating it into the Formik API in a way for any user to opt-in to the types already defined by Formik, providing the functionality that is suggested by https://jaredpalmer.com/formik/docs/guides/typescript .

[johnrom]

Cancel the old response. I think I'd be okay with importing a Formik wrapper that provides Fields separately, if you are concerned about maintaining that. However, in order for that to happen, we'd still need to commit to a set of real typings for fields. If we cannot strongly type the fields themselves and commit to it, then this will be better as a fork.

[jaredpalmer]

Use alias .FC<T>

[johnrom]

@jaredpalmer just checking in on this. I know you're also thinking about revisiting the way Fields are handled as part of your v2 changes (I haven't looked at the specifics). Is this something you'd like me to apply over v2 instead, something you've already incorporated into v2, or something you're not interested in at all?

Strongly typing these fields wouldn't be a breaking change in any cases I could find, but I still think it'd make sense to ship with the v2 changes since it is a major version. I'd like to release my strongly typed field provider based on these changes.

[jaredpalmer]

With #1413 there is no dependency on Formik.tsx AFAICT. Correct?

[johnrom]

Correct there's no need to define the type any longer.

From my experience I do like when APIs' public methods expose a real type as opposed to an inferred type because it can be traced back to a single definition that I could then read and reuse as a consumer. But that's unrelated to this PR :).

[johnrom]

I think I misunderstood your question. Your PR does fix the conflict we would have had on getFormikContext which I initially had to change because it was typed as <any>. However, without this PR there is still a massive hole in the typings for Field. Basically the way Field currently works now is to allow any value to propagate into FormikState<MyForm>.values.myValue instead of expecting the type of MyForm.myValue. What this means is that you actually have to check if MyForm.myValue is the type that it says it is or not because the types inside of FormikState.values could be a lie.

If this isn't a situation you are expecting to fix within the context of Formik, then I recommend changing the value of onReset(values: Values), onSubmit(values: Values), and validate(values: Values) because the key-values are actually more like [index: key in keyof Values | string | number | symbol]: unknown.

If this is a situation you are expecting to fix eventually, this PR gets us halfway there by adding strong typing, and the last thing to do is provide a value converter prop for onChange which expects a function that takes an input of unknown and outputs a value of ValueType. But before going that far, I wanted to have a discussion on the proper way to really type these Fields. For backwards compatibility, this would just be a noop unknown => any;

[hegelstad]

Cool to see that this is in active development.

I have to decide whether to invest more time in using Formik with types or if I rather would be better of waiting for Hooks version, which should be much easier to type anyway? What are your foresights?

[johnrom]

@hegelstad you can still use Formik with TypeScript (I do), but you need to make sure your change callbacks or submit function accept unknown or any (if you use an old TS version) instead of expecting them to already be the correct type. This pull is about making that clearer to TS users or finding a way to correctly type the value before it enters the form's values and causing TypeScript errors when that it hasn't been correctly converted.

[hegelstad]

@johnrom Thanks for explaining.

[adamscybot]

This looks great -- really need this. As mentioned above, is this likely to land in v2 or is it going to be merged in v1?

[johnrom]

(edited 2019-05-18)

Hi all,

I've created a wrapper that uses this specific PR to demonstrate an example use case for this change. You can check that out here, and try it out with npm install formik-typed.

https://github.com/johnrom/formik-typed/

There are examples under the /examples folder of how you would use this typed wrapper. Ultimately, I think there's a bit of a performance hit for the way it (the typed wrapper above) is currently implemented, but it is pretty useful. There is one more step I want to do in this PR. I think the performance hit is acceptable for me at the moment but I'd love feedback for optimizing it.

Basically, when you type the field, you get access to what the type should be, and you can easily make sure you convert the string value that an HTML field would always return to that type. However, I'd like to make this part of the API. In order to do that, I want to determine how we can sometimes require a prop based on a generic. During Formik's handleChange, we'll see if a strongly-typed valueConverter callback exists on the fieldConfig, and call on event.target.value or value. I tried to implement this but ended up having to move on because React didn't like any of my attempts. Here is an example of what I'd like to implement, and I'll leave this here for now for feedback or suggestions on implementation:

export type FieldConfig<Values = any, ValueType = any, Props = {}> =  {
   // ... the rest of FieldConfig
} & ValueType extends string ? 
    // string doesn't require a value conversion because inputs return string
    // of course you should be able to format it anyway
    { valueConverter?: (value: string) => ValueType }
    // if ValueType is not a string, it must be converted from the string value the HTML field emits
    : { valueConverter: (value: string) => ValueType }

This didn't seem to work at all, even though I assumed there would be no problem. I might try again and see if I just had a mistake. I also tried another way, changing valueConverter to ValueConverterCallback | undefined, but that didn't actually make the react prop optional -- you were required to pass undefined or the callback.

Anyway, I wanted to leave this info here as I'll be away from this PR until there is a real interest in getting it merged in.

[jaredpalmer]

I like this since it is backwards compat. Is the perf hit a typechecking thing or actual functionality?

I wonder if using typescript’s ReturnType type to inference the result of that would solve the issue with the value converter

[johnrom]

There is (probably) a performance implication to using the typed wrapper I linked to above, since it uses a proxy and may or may not sometimes re-run that function depending on how you use it. I edited my comment to make that sentence make a bit more sense. Do you think I should mess with a separate value converter PR after these typings are merged in? I'm glad you think these types could make it into the API. Do you think it would be better to wait until after v2, to rebase these changes on v2 to launch with it, or to continue with this PR and possibly merge these types up into v1 since they are backwards compatible?

[jaredpalmer]

I’m a little confused about the Proxy stuff. Does this need proxies to work?

[johnrom]

Edit: To recap, you should only worry about the current PR's files changed, as I originally implemented something that I've moved to a different project. In its current state, the PR only modifies the Typescript definitions of Formik, and updates TypeScript because without that update everything will have to use Values extends object in order to use object destructuring.

[Andreyco]

Does IE understand proxies?

[johnrom]

This PR no longer has any proxies in it. The proxy was moved to johnrom/formik-typed

I added your question there, so I can keep a note of it. This PR is strictly about eliminating "any" where it makes sense, specifically from Fields. to unlock more possibilities on the TypeScript side. This PR should see no change to plain JS and be backwards compatible.

I can re-do this PR if it makes sense, to get rid of all mentions of the Proxy as I initially got ahead of myself.