Skip to content
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

[feature request(collections)] option to merge undefined in deepMerge #4752

Open
scarf005 opened this issue May 16, 2024 · 4 comments
Open

Comments

@scarf005
Copy link
Contributor

scarf005 commented May 16, 2024

Is your feature request related to a problem? Please describe.

when merging together deeply nested object (e.g react state), it's important to get 'nonempty' values when merging 'empty' values with 'nonempty' values.

for array, map and set it defaults to merging empty and non-empty values together:

deepMerge({ a: new Set([1]) }, { a: new Set() })
//=> { a: Set(1) { 1 } }

however, it doesn't work with nullish values. it'd be very useful to be able to output non-nullish value from merging nullish and non-nullish value together.

deepMerge({ a: 1 }, { a: undefined })
//=> { a: undefined }
// expected: { a: 1 }

Describe the solution you'd like

in DeepMergeOptions, add nullish: MergingStrategy that controls whether deepMerge will choose non-nullish value over nullish ones, such that

type State = {
  pos: {
    x: number
    y: number
  }
}
type UpdateState = Partial<{
  pos: Partial<{ 
    x: number
    y: number 
  }>
}>

const merge = (state: State, newState: UpdateState): State => deepMerge(state, newState, { nullish: "merge" })

always holds. (currently the return type of merge will be UpdateState)

Describe alternatives you've considered

make nullish: "merge” as default. this may cause breaking changes.

@kt3k
Copy link
Member

kt3k commented May 16, 2024

Does nullish: "merge" mean that null also has the same effect as undefined in the above context?

@scarf005
Copy link
Contributor Author

Does nullish: "merge" mean that null also has the same effect as undefined in the above context?

yes, it could have been nullish: "replace" | "mergeBoth" | "mergeNull" | "mergeUndefined" but the type would be too complex.

@kt3k
Copy link
Member

kt3k commented May 17, 2024

How about only doing this for undefined? (The option would be undefined: "replace" | "ignore"

Handling null in the same way feel a bit strange to me as { a: null } is not assignable to Partial<{ a: number }> for example.

@scarf005
Copy link
Contributor Author

sure, since Partial only works with undefined.

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

No branches or pull requests

2 participants