Fix MergeDeep to remove unintentional undefined from optional properties (#1314)
#1319
+38
−6
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Close #1314
(Updated the entire comment.)
This PR is a small change, but it consists of two parts.
Part 1
MergeDeep<Pokemon, Partial<Pokemon>>should be equivalent toPartial<Pokemon>according to this comment inmerge-deep.d.ts:type-fest/source/merge-deep.d.ts
Line 386 in c671aa2
However, before this PR, the result type was:
which causes the following error (in the issue):
(Note: The above is a direct quote from an issue comment. In this case, it seems appropriate to type it as
Partial<Pokemon>instead ofPokemon.)This extra
| undefinedis 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:
type-fest/test-d/merge-deep.ts
Line 325 in c671aa2
Requiredis 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.