Questioning the way how you deep merge a partial JSON into the original JSON?

[binarykitchen]

Buona sera,

I'm experiencing a TypeScript conflict while deep merging a partial JSON into the original JSON using your latest version 3.2.0.

Error Message

Type 'PartialObjectDeep<VideomailRecorderOptions, { recurseIntoArrays: false; allowUndefinedInNonTupleArrays: false; }>' is not assignable to type 'VideomailRecorderOptions' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
  Property 'apiUrl' is optional in type 'PartialObjectDeep<VideomailRecorderOptions, { recurseIntoArrays: false; allowUndefinedInNonTupleArrays: false; }>' but required in type 'VideomailRecorderOptions'.

Why should I add undefined to the property apiURL to VideomailRecorderOptions when it must remain defined during a deep merge from a partial one? The target already has it defined and would crash my app. No, thanks.

Code

That code is:

const newOptions: VideomailRecorderOptions = deepmerge({
  onlyDefinedProperties: true,
})(state, action.payload);

Link:
https://github.com/binarykitchen/videomail-recorder/blob/main/src/features/options/reducers.ts#L15

As you can see, it's Redux, merging the payload back to the original state the immutable way. Only with defined properties.

Steps to reproduce

1. Git clone git@github.com:binarykitchen/videomail-recorder.git
2. npm install
3. npm run types
4. Errors in the console ...

Summary

All I am after is just to deep merge partial options in JSON format into the original state in JSON, returning a new JSON.

The partial option typings stem from the original option typings this way:

type PartialVideomailRecorderOptions = PartialDeep<VideomailRecorderOptions>;

This PartialDeep helper function comes from the type-fest library, which is well-tested:
https://github.com/sindresorhus/type-fest

Appreciate any feedback.

Grazie 🙇

[mcollina]

Hi @binarykitchen,

I've opened a PR to fix this issue: #83

The fix adds proper TypeScript types for the onlyDefinedProperties: true option, so that merging Partial<T> into T now correctly returns T instead of a type with optional properties.

Could you test this PR against your videomail-recorder project to confirm it resolves the TypeScript error you were seeing?

You can install the fix directly from the PR branch:

npm install github:fastify/deepmerge#fix/onlyDefinedProperties-types

Then run npm run types and let us know if the error is resolved.

Thanks!

[binarykitchen]

Ciao @mcollina

Nice TypeScript kung foo here. Yep, confirming, your PR produces no more TS errors.

Just one thing I wonder and I am questioning:

  const newOptions = deepmerge({
    onlyDefinedProperties: true,
  })(state, action.payload);

state is of type VideomailRecorderOptions. But the returned value newOptions does not have this type, nor any typings at all.

I expect the return value to be the same type as the first parameter, state which should be VideomailRecorderOptions?

For reproduction, I've pushed the latest to master, here:
https://github.com/binarykitchen/videomail-recorder/blob/main/src/features/options/reducers.ts#L15

Grazie

PS: proper syntax is with quotation marks ;)
npm install "github:fastify/deepmerge#fix/onlyDefinedProperties-types"

[mcollina]

Thanks for testing @binarykitchen!

I found the issue with the function types - functions were being treated as mergeable objects, causing callbacks.adjustFormDataBeforePosting to resolve to {} instead of the correct function type.

I've pushed a fix that excludes functions from the mergeable object check. Can you test again?

npm install "github:fastify/deepmerge#fix/onlyDefinedProperties-types"

Regarding the hover showing a complex computed type instead of VideomailRecorderOptions - that's expected TypeScript behavior. The computed type should now be fully assignable to VideomailRecorderOptions, so you can add an explicit return type annotation for cleaner hovers:

function deepMergeOptions(
  state: VideomailRecorderOptions,
  action: PayloadAction<PartialVideomailRecorderOptions | undefined>,
): VideomailRecorderOptions {  // <-- explicit return type
  // ...
}

(And thanks for the syntax tip on the npm install command! 😄)

[binarykitchen]

Biongiorno!

Right, there are callback function in that options object.
Now, with your latest PR it looks perfetto.

so you can add an explicit return type annotation for cleaner hovers:

Well, I'm not a fan to define return types on function level which can cause more problems in my experience.
I've set the type on variable level instead:

const newOptions: VideomailRecorderOptions = deepmerge({
  onlyDefinedProperties: true,
})(state, action.payload);

So yes, that's all. Think you can merge and ship the good stuff 🚀

Grazie mille