Skip to content

Add SetNonNullableDeep type #1117

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

som-sm
Copy link
Collaborator

@som-sm som-sm commented Apr 30, 2025

Closes #795

Notes:

  • Currently, there’s no way to remove nullables just from the rest element in an array type. For instance, you can’t get [string | null, …number[]] from [string | null, …(number | null)[]].

    Furthermore, it’s not very obvious what KeyPaths should be used to target the rest element. Using number would target all elements, not just the rest element (refer to the next point).

  • Rest elements in array types behave slightly differently across versions when KeyPaths includes number. Refer the example below:

    // In v5.3 and below
    // `number` in `KeyPaths` DOESN'T affect the rest element
    type Test = SetNonNullableDeep<{a: [string | null, number | null, ...Array<boolean | null>]}, `a.${number}`>;
    //=> { a: [string, number, ...(boolean | null)[]]; }
    // In v5.4 and above
    // `number` in `KeyPaths` DOES affect the rest element
    type Test = SetNonNullableDeep<{a: [string | null, number | null, ...Array<boolean | null>]}, `a.${number}`>;
    //=> { a: [string, number, ...NonNullable<boolean | null>[]]; }
  • SetNonNullableDeep currently doesn't deeply remove all nullables if KeyPaths is any. This will require a separate NonNullableDeep type, which I'll create it a separate PR.

@som-sm som-sm force-pushed the feat/add-set-non-nullable-deep-type branch 5 times, most recently from 42cd002 to c91a019 Compare May 1, 2025 13:14
@ogroppo
Copy link

ogroppo commented May 1, 2025

Thanks for looking into this!

@som-sm som-sm force-pushed the feat/add-set-non-nullable-deep-type branch 4 times, most recently from 7a21325 to 3427654 Compare May 1, 2025 16:33
@som-sm som-sm requested a review from sindresorhus May 1, 2025 16:56
@som-sm som-sm marked this pull request as ready for review May 1, 2025 16:56

// === Arrays ===
expectType<[string, number | null, boolean]>({} as SetNonNullableDeep<[string | null, number | null, boolean | undefined], '0' | '2'>);
expectType<{a?: [string, number, (boolean | null)?] | null}>({} as SetNonNullableDeep<{a?: [string | undefined, number | undefined, (boolean | null)?] | null}, 'a.0' | 'a.1'>);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something really strange is happening here. If I remove the test on line 106, some tests in set-required-deep.ts start failing—tests that are completely unrelated to this. And there's no changes in SetRequiredDeep related files.

And this doesn't just fail locally, it also fails in CI.

@sindresorhus @Emiyaaaaa Do you have any idea why this could be happening?

weird-compressed.mov

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea. Probably some compiler quirk.

The minimum. Any more simplifications of this and it triggers the issue.

expectType<{a?: [string]}>({} as SetNonNullableDeep<{a?: [string]}, 'a.0'>);

@som-sm som-sm force-pushed the feat/add-set-non-nullable-deep-type branch from 3427654 to 8a52c78 Compare May 1, 2025 18:01
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.

NonNullableDeep
3 participants