The types of deep merged object isn't deep merged about ts-deepmerge HOT 22 CLOSED

After trying. You are right. Typescript can't know if the args expanded from an array. I have to explain the generic exactly. I'll compare iter the array then merge objs one by one or expand it directly

Thanks for your patience. Have a good day :D

from ts-deepmerge.

Regardless of the use case though, I'll look into getting a new version out that attempts to cater for these extra scenarios though because it should be more useful than resolving to never - will let you know once it's released 🙏

from ts-deepmerge.

But yeah, I added test coverage over the two scenarios you described here, plus made some of the existing tests more verbose, but other than that, none of the other tests changed and they're still passing so it seems it should be all good. I added it as a major version bump though just in case, because the changes involved completely re-writing the TS logic to infer the return type, which is fundamentally different.

How about a TMerged<T, Ignored = TPrimitives>?

The TMerged type isn't exported because it's only used internally, so it wouldn't make much sense to add a type param to it.

from ts-deepmerge.

It's working as designed currently. Keep in mind that we're trying to evaluate the end result, not each individual object. We know that the result of merge({ a: 1 }, { b: 2 }, { c: 3 }) is going to be { a: number, b: number, c: number }, so we don't want the end result to say that any of the properties are optional, because the end result guarantees their existence.

If you provide any values as undefined explicitly within any of your provided objects though, then it will add that to the joined union to make it potentially undefined. Hope that makes sense.

I have no idea what your last example is referring to though sorry 😅 you haven't provided any context as to what neither title nor it is, so I can't really help you there.

But yeah it's now working as designed whilst inferring all possible values for inconsistent object property values as requested (which is still a bad code smell btw - your provided objects should really be aligned to the same contract) - going to log off for the day now but good luck with your project!

from ts-deepmerge.

It's useful in some scenarios, but if you never declare explicit types for any of the data your application works with, it leaves your code subject to inconsistencies leaking in if you have no explicit guards over any of the data. If you're only using TypeScript for basic IDE autocompletion etc then that's one thing, but you'll miss out on some of the safety features that TypeScript can provide if you only infer everything. Gotta have a healthy balance and guard against any critical data contracts 🙏

from ts-deepmerge.

If there is some packet or form need the interface. I will do :D. But type like the deep merge is lovely and elegant :D.

Edit: Infer is as awesome as you!

from ts-deepmerge.

Hey - do you have a link to some code or some sample code you can paste that highlights the issue? Would be much easier to attempt to fix with something specific to replicate the issue and drop into a unit test 🙏

from ts-deepmerge.

Which c is never here. But it should be string or 'foo' | 'bar' or 'bar'

const A = { a: { b: 1, c: 'foo' } } as const // const A: {readonly a: {readonly b: 1, readonly c: "foo"}}
const B = { a: { c: 'bar' } } as const // const B: {readonly a: {readonly c: "bar"}}
const test = deepMerge(A, B) // const test: {readonly a: {readonly b: 1, readonly c: "foo"}} & {readonly a: {readonly c: "bar"}}
const c = test.a.c

from ts-deepmerge.

Hmmm that's a very weird way of declaring your types, but I'll attempt to replicate it.

Generally, you should either declare proper types/interfaces for your data, or just provide it as-is without the as const, which may result in some difficulties when attempting to infer the resulting type when provided to our merge function.

Do you have a real-world example of this occuring? Just wondering why you would be using as const rather than either letting it infer from the more general implicit type, or assigning more definite types using an interface or type.

from ts-deepmerge.

For example, this works totally fine:

const a = { a: { b: 1, c: "foo" } };
const b = { a: { c: "bar" } };
const test = merge(a, b);

console.log(test.a.c); // "bar"

Is there a reason you need to use the as const syntax?

from ts-deepmerge.

as const is for avoiding the type be string. So that the final type will be string that won't caused the problem. It's just for describe this issue
There is another example without as const. The b will be never. But it should be string | number or number.

const A = { a: { b: '1' } }
const B = { a: { b: 1 } }
const test = deepMerge(A, B)
const b = test.a.b

from ts-deepmerge.

Is there a specific reason you need it to be that strict? Also do you have a real-world scenario for why you would be merging objects that would have the same keys but completely different type of value like that last example? Seems like an awfully strange use case 🤔

from ts-deepmerge.

Due to https://github.com/voodoocreation/ts-deepmerge/blob/master/src/index.ts#L47. The last value should be accept. But the type says no

from ts-deepmerge.

Yes I know that - but keep in mind that it's doing some advanced TS in order to try to infer the value - it's not inferred by the runtime code. I'm going to have to try to update the types to be able to allow the resulting return type to not resolve to never due to the two objects having totally different value types for those properties.

I just have to be careful because it also needs to not resolve to undefined when comparing the partial objects. The code in the stackoverflow answer you linked to will allow it to resolve that property to string | number, but it will also assign undefined for properties that aren't defined in a partial object which isn't correct.

But yeah, generally speaking it's bad practice to be trying to merge things where each property can be more than one type, so it seems like whatever implementation you have which has that happening could potentially be improved. Generally you would be trying to merge several objects of the same structure, not merging several objects that have no relation to each other.

Anyway - I'll see what I can do. If it's possible to have the types work in the way you expect I'll get a new version out, but will let you know if I'm unable to. If I can't, you might need to improve your code so that you're using this function in the way that it was designed to be used, rather than passing in objects that have no relation to each other 🤔

from ts-deepmerge.

Have addressed this in version 5.0.0, which includes two tests to cover the scenarios you've mentioned here.

from ts-deepmerge.

Have addressed this in version 5.0.0, which includes two tests to cover the scenarios you've mentioned here.

Sry for my careless. Forget to say I've add a generic named Ignored to don't unpack some types such as Date. I think the Ignored is useful in this case

export type DeepMerged<T, Ignored = never> = [T] extends [Array<unknown>]
  ? { [K in keyof T]: DeepMerged<T[K], Ignored> }
  : [T] extends [object]
  ? PartialKeys<
      {
        [K in AllKeys<T>]: Index<T, K> extends Ignored
          ? Index<T, K>
          : DeepMerged<Index<T, K>, Ignored>
      },
      Exclude<AllKeys<T>, keyof T> | OptionalKeys<T>
    >
  : T

from ts-deepmerge.

Oh, it's seems that yours types is different from origin types. So the Date won't be unpack. Nvm

from ts-deepmerge.

You shouldn't need to manually ignore Date - have you tried version 5.0.0 yet? I've included Date in the list of generics to treat as-is rather than attempting to merge the properties.

from ts-deepmerge.

You shouldn't need to manually ignore Date - have you tried version 5.0.0 yet? I've included Date in the list of generics to treat as-is rather than attempting to merge the properties.

Yup, I saw it. Awesome!
How about a TMerged<T, Ignored = TPrimitives>?

from ts-deepmerge.

In my mind. An array {a: string} | {b: number} | {c: string}[]. The a, b, c should optional. But it isn't with current Merged? Is it as expected?

Edit:
What's my use case is interopImportCJSDefault(deepMerge)({ title }, ...it).

from ts-deepmerge.

No problem! Hope you can get there - I know TS can be a bit confusing at times.

It really helps if you can clearly define your types beforehand to declare some explicit types, which is always much tidier and more scalable than relying on the inferred return type to have all of the types declared for you by our merge function. It's definitely convenient for simple usages, but more advanced types should always be declared using interface or type (depending on the nature of the type you're declaring - basic objects should usually use interface) - this will allow you to structure your types in a much more organised and explicit way which will also make it easier to read what everything should be, rather than always relying on implicit inferred return types.

But yeah, just a little suggestion anyway just in case it's of some use 🙏

from ts-deepmerge.

Lol. I think inferring is cool. So I avoid describe interface or generic explicitly.

Below is how I like to declare types. It's funny. I love ts xddd. Infer from this and that. lul

export type ExtractorOperated<T extends Extractor<unknown, unknown>> =
  T extends Extractor<infer R, unknown> ? R : never

export type ExtractorExtracted<T extends Extractor<unknown, unknown>> =
  T extends Extractor<unknown, infer R> ? R : never

from ts-deepmerge.