Skip to content

Fix conditional type in FormikTouched typings #2273

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hpohlmeyer
Copy link

I was running into an issue where FormikTouched did not produce the expected typings. Here is an example:

type Lesson = {
  name: string;
  instructors?: Array<{ id: string }> | null;
}

type Result = FormikTouched<Lesson>;

I would have expected that Result is typed like this:

type Result = {
    name?: boolean;
    instructors?: Array<{ id: boolean } | boolean;
}

but the actual Result looks like this:

type Result = {
    name?: boolean;
    instructors?: boolean;
}

The problem is, that conditional types fail to distribute in properties of mapped types. This can be fixed in two ways:

  1. Introduce an auxiliary type
  2. Infer the value before using it

For this PR I have decided to go with the second solution to avoid adding an auxiliary type.

Conditional types fail to distribute in properties of mapped types as
described in microsoft/TypeScript#33669
#issuecomment-536493169. This uses the infer approach to avoid
needing an auxiliary type to fix the issue.
@vercel
Copy link

vercel bot commented Feb 6, 2020

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/jared/formik-docs/remlmb253
✅ Preview: https://formik-docs-git-fork-hpohlmeyer-fix-conditional-in-fo-19f9d8.jared.now.sh

@hpohlmeyer
Copy link
Author

While creating the PR, I’ve also noticed, that FormikTouched changed from

type FormikTouched<Values> = {
    [K in keyof Values]?: Values[K] extends object ? FormikTouched<Values[K]> : boolean;
};

to

export type FormikTouched<Values> = {
  [K in keyof Values]?: Values[K] extends any[]
    ? Values[K][number] extends object // [number] is the special sauce to get the type of array's element. More here https://github.com/Microsoft/TypeScript/pull/21316
      ? FormikTouched<Values[K][number]>[]
      : boolean
    : Values[K] extends object
    ? FormikTouched<Values[K]>
    : boolean;
};

The first one allowed undefined as an array value, the second does not. Is allowing undefined not the intended way to go here? If you only touch the second entry of an array the first value has to be undefined, right? That would not be possible with the second definition.

Should I change this PR to work with the old definition istead?

@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 6, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 6834009:

Sandbox Source
Formik TypeScript Playground Configuration

@hpohlmeyer
Copy link
Author

hpohlmeyer commented Feb 6, 2020

Oh and when I was commiting the changes I’ve got this error:

husky > pre-commit (node v12.13.0)
No input files specified, defaulting to src test

/Users/blackwoodb/dev/react/formik/packages/formik-native/src/index.ts
  1:1  error  Definition for rule '@typescript-eslint/consistent-type-assertions' was not found  @typescript-eslint/consistent-type-assertions
  1:1  error  Definition for rule '@typescript-eslint/no-unused-expressions' was not found       @typescript-eslint/no-unused-expressions

/Users/blackwoodb/dev/react/formik/packages/formik-native/test/blah.test.ts
  1:1  error  Definition for rule '@typescript-eslint/consistent-type-assertions' was not found  @typescript-eslint/consistent-type-assertions
  1:1  error  Definition for rule '@typescript-eslint/no-unused-expressions' was not found       @typescript-eslint/no-unused-expressions

✖ 4 problems (4 errors, 0 warnings)

husky > pre-commit hook failed (add --no-verify to bypass)

I could not find out how to fix it, so I’ve commited with --no-verify. If you have any hints how to fix it, I can do that. It looks like I am not the only one having that problem.

@stale stale bot added the stale label Apr 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant