Skip to content

MergeDeep: Remove extra undefined from optional properties#1319

Merged
sindresorhus merged 3 commits intosindresorhus:mainfrom
taiyakihitotsu:issue-1314
Jan 14, 2026
Merged

MergeDeep: Remove extra undefined from optional properties#1319
sindresorhus merged 3 commits intosindresorhus:mainfrom
taiyakihitotsu:issue-1314

Conversation

@taiyakihitotsu
Copy link
Contributor

@taiyakihitotsu taiyakihitotsu commented Dec 23, 2025

Close #1314

(Updated the entire comment.)

This PR is a small change, but it consists of two parts.

Part 1

import type {MergeDeep} from 'type-fest';

interface Pokemon {
      name: string;
      type: string;
    }

MergeDeep<Pokemon, Partial<Pokemon>> should be equivalent to Partial<Pokemon> according to this comment in merge-deep.d.ts:

- Properties that exist in both objects are merged if possible or replaced by the one of the source if not.

However, before this PR, the result type was:

{
  name?: string | undefined;
  type?: string | undefined;
}

which causes the following error (in the issue):

error TS2322: Type '{ name?: string | undefined; type?: string | undefined; }' is not assignable to type 'Pokemon'.

(Note: The above is a direct quote from an issue comment. In this case, it seems appropriate to type it as Partial<Pokemon> instead of Pokemon.)

This extra | undefined is unnecessary here.
This PR removes that redundancy with the following change:
https://github.com/sindresorhus/type-fest/pull/1319/files#diff-5ce63d84bd0497ef6dd72ef89147e7bf80f464a1185b1a34139c6c1cb6c37ee5R36

Part 2

After the change in Part 1, this test doesn't pass:

expectType<true>({} as IsEqual<RecordPartial, testRecordReplacedByOptionalRecord>);

Required is added to fix this:
https://github.com/sindresorhus/type-fest/pull/1319/files#diff-5ce63d84bd0497ef6dd72ef89147e7bf80f464a1185b1a34139c6c1cb6c37ee5R61

Note

The issue described above may be related to PR #1209, which has been suspended since August 31.
I intentionally did not include any changes related to that PR in order to keep this change minimal.

If these changes are considered inappropriate due to the context of PR #1209, I’m fine with closing this PR.

Copy link
Collaborator

@som-sm som-sm left a comment

Choose a reason for hiding this comment

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

@taiyakihitotsu Added few comments!

@som-sm som-sm changed the title Fix MergeDeep to remove unintentional undefined from optional properties (#1314) MergeDeep: Remove extra undefined from optional properties Jan 13, 2026
@taiyakihitotsu taiyakihitotsu requested a review from som-sm January 13, 2026 22:01
@taiyakihitotsu
Copy link
Contributor Author

@som-sm
Thanks for your review!
I’ve addressed all of your comments.

@sindresorhus sindresorhus merged commit a6af489 into sindresorhus:main Jan 14, 2026
7 checks passed
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.

merge-deep: wrong types inferred when using partial interfaces

3 participants