Ts-toolbelt: O.Merge broke?

[mesqueeb]

🐞 Bug Report

Describe the bug

I just updated from ^6.4.2 → ^6.13.11

And a lot of types that used O.Merge broke because of changed behaviour.

Before & Expected:

After & Bug reproduction:

Possible Solution

I don't know :D

Edit:

I just double checked, it seems to work as expected until [email protected] and breaks after 13

[mesqueeb]

I looked at the differences between 12 and 13, and it's massive

🤯

Any chance you can revert the latest version back to the old behaviour and maybe do a major bump (v7) for the new behaviour?

Unless it's a bug, in which case just fixing it to the old behaviour is preferred. : )

[millsp]

Thanks for reporting this. While Merge type safety has been very improved
lately, it seems like there is a regression that has been introduced at the
same time.

Please check that you have --strictNullChecks flag enabled. If you cannot
enable it, then you could try to use Patch instead. It's like the old
Merge. This new Merge is able to handle unions of objects as well as
optional fields, effectively improving type safety as well as the merging
capabilities (can merge nested arrays too).

I'm sorry about this, I might have been too eager pushing this upgrade,
maybe this could have been kept for the v7.

Patch has the same construct as Merge with a different strategy so it would
be interesting if you could see if it fails similarly or not, this would
give me lots of insight for the bug fix of tomorrow.

Let me know :) Thanks!

Le dim. 19 juil. 2020 à 02:14, Luca Ban [e-mail] a écrit :

🐞 Bug Report Describe the bug

I just updated from ^6.4.2 → ^6.13.11

And a lot of types that used O.Merge broke because of changed behaviour.
Before & Expected:

[image: Screenshot 2020-07-19 09 11 48]
https://user-images.githubusercontent.com/3253920/87864180-16a3b800-c9a0-11ea-838d-2e027015c037.png
After & Bug reproduction:

[image: Screenshot 2020-07-19 09 09 07]
https://user-images.githubusercontent.com/3253920/87864183-23281080-c9a0-11ea-862a-f0f748b69fb6.png
Possible Solution

I don't know :D

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/millsp/ts-toolbelt/issues/130, or unsubscribe
https://github.com/notifications/unsubscribe-auth/AEMMUDOGRJ7W5UAUT2XX3QTR4I3FBANCNFSM4PARYWEA
.

[mesqueeb]

@millsp
Here are some reproductions for you.

• Yes "strict": true is enabled in tsconfig.
• The comment below each time is what VSCode showed me on hover.
• In both cases I feel like Patch is correct and Merge is incorrect.

type Test = { a?: boolean, b?: string | null }
type PlainObject = { [key: string]: any }

type Merge = O.Merge<Test, PlainObject>
// type Merge = {
//   [x: string]: string | boolean | null | undefined;
//   a?: any;
//   b?: any;
// }

type Patch = O.Patch<Test, PlainObject>
// type Patch = {
//   [x: string]: any;
//   [x: number]: any;
//   a?: boolean | undefined;
//   b?: string | null | undefined;
// }

type TestNoOptionalProps = { a: boolean, b: string | null }

type Merge2 = O.Merge<TestNoOptionalProps, PlainObject>
// type Merge2 = {
//   [x: string]: string | boolean | null;
//   a: boolean;
//   b: string | null;
// }

type Patch2 = O.Patch<TestNoOptionalProps, PlainObject>
// type Patch2 = {
//   [x: string]: any;
//   [x: number]: any;
//   a: boolean;
//   b: string | null;
// }

I can change all my types to O.Patch, but I use O.Merge throughout a looooot of libraries at this point... 😭

So if possible I'd prefer if it's possible to:

• publish v6.14 which reverts the behaviour back to v6.12
• publish the current v6.13 as v7.0

[millsp]

No worries, I will fix this.

[millsp]

I recommend that you use {[k in string]: any} instead of {[k: string]: any}. The first one plays better with type modifications.

[mesqueeb]

I don't understand why xO

[millsp]

I was also doubtful but it's explained here https://www.typescriptlang.org/docs/handbook/advanced-types.html#index-types-and-index-signatures

[millsp]

OK, while I was able to fix most complaints about Merge, there is one that stays and it seems normal to me. Let's explain, when you do, for instance:

type t0 = Merge<{
    a: string
    b?: string
}, {
    [K in string]: any
}>

// type t0 = {
//     [x: string]: any;
//     a: string;
//     b?: any;
// }

The field a stays string while b becomes any because it has the possibility of getting merged with any (if b is left empty). So b is any, and this is not a regression IMHO. Unfortunately, in TypeScript, unions like 1 | any widen to any, losing programmer metadata. Now you might wonder if there is a way to preserve this metadata, and the answer is yes. any can be replaced by {} since anything is an object is JS.

type t0 = Merge<{
    a: string
    b?: string
}, {
    [K in string]: {}
}>

// type t0 = {
//     [x: string]: {};
//     a: string;
//     b?: string | {};
// }

But I would understand that I bothers you to change this on all your libraries, just wanted to discuss this before I do the revert.

[millsp]

But it's good to note that your lib is not 100% typesafe if you choose to use Patch instead of Merge, which is essentially what will happen when I revert.

[mesqueeb]

@millsp thanks for trying to teach me.

First of all, I was reviewing my post above, and I realised I gave you examples but FORGOT TO INCLUDE THE TYPES I used in the examples:

type Test = { a?: boolean, b?: string | null }
type PlainObject = { [key: string]: any }

That's so unfortunate, and I'm surprised you were able to derive what I meant. I've edited the post above and added them now.

I still have difficulties understanding a couple of things. But I'll try to get some professional help for this, I don't expect you to keep teaching me for free lol.

my background

I often need to use the type Object when I make a library in which the dev can pass any keys.
Often a times using Object in TS causes too many issues, because the devs trying to access keys they defined themselves suddenly give errors that say "key doesn't exist on Object" etc.

That's why I often use

type PlainObject = { [key: string]: any }

Especially when it comes to allowing a function parameter to accept params and any other params, I merge the function param with the PlainObject.

your logic

• [K in string] over [K: string]
  you gave a link to the documentation here:
  

But this documentation doesn't say anything about

• when should we use [K in string] over [K: string]
• why should we use [K in string] over [K: string]
• what is the underlying difference between [K in string] and [K: string]
• what are the pros and cons of using one over the other.

I feel like in order to understand what you mean, I need to understand those 4 questions. Do you know any further documentation I can look at or read that delves into this?

• {} over any

In your specific example, I want a different behaviour than you showed me, so maybe we're having a mis-understanding:

type t0 = MergeTheseTwoTypes<{
    a: string
    b?: string
}, {
    [K: string]: any
}>

What I would want with the above case (whatever MergeTheseTwoTypes ends up being - self made or something from ts-toolbelt):

• b can be absent, but if it is present, it _should_ be a string
• b should never be anything but a string or absent entirely
• any other keys that are not named a or b can be any

(PS: I believe using {} can potentially confuse a lot of devs (including myself) trying to use my libraries but who are still new at TS, but in this conversation this is irrelevant)

[millsp]

• In this case, you will want to use Patch, but the problem with Patch is that:

type t0 = O.Merge<{
    a?: string
}, {
    a: number
}> // a: string | number

type t1 = O.Merge<{
    a?: string
}, {
    a?: number
}> // a?: string | number

type t2 = O.Patch<{
    a?: string
}, {
    a: number
}> // a?: string

type t3 = O.Patch<{
    a?: string
}, {
    a?: number
}> // a?: string

So what you would want, it's a Merge utility that does not alter the ? optional fields. So you want something between Patch and Merge. I could write such a type for you, but it won't be included in the ts-toolbelt. Or for now, I will alias Merge to Patch, and will undo this on the v7.

• The documentation does not explain why you should use [K in string] instead of [K: string]. According to the docs there is no difference, but when working with types there is. I get the feeling that [K: string] is more reserved for interfaces. If you work with types manipulation/computation, you'll just want to stick with types and not interfaces:

type t0 = Omit<{[k: string]: any}, 'a'>
// {
//     [x: string]: any;
//     [x: number]: any;
// }

type t1 = Omit<{[k: string]?: any}, 'a'>
// You cannot do this, compile error

type t2 = Omit<{[k in string]: any}, 'a'>
// {
//     [x: string]: any;
// }

type t3 = Omit<{[k in string]?: any}, 'a'>
// {
//     [x: string]?: any;
// }

As you can see, t0 was polluted with extra info that we don't really care about, we all know that in JS a number index or a string index is the same because the runtime will apply .toString() on any index you pass, you can index with anything. On top of that, in t1, you cannot add ?. So why use [k: string] at all? => Only when you work with interfaces, not type manipulation. For type manipulation, just use [k in string].

• There is no difference between {} and any, just that any | string becomes any and {} | string stays the same. Now I understand that this {} can be confusing. In fact {} just means any object, since everything is an object in JS, it works. Now we can find some ways for you to hide this:

interface Any {} // same as {} but with a nice name

type merged = Merge<{
    a: string
    b?: string
}, {
    [K in string]: Any
}>

// {
//     [x: string]: Any;
//     a: string;
//     b?: string | Any | undefined;
// }

declare const variable: merged

const t0 = variable['b'] // string | Any | undefined
const t1 = variable['x'] // Any

let t2: typeof t1 = 42 // works with anything

and

const t0: {} = 1

But this might still not be the solution you wanted, I think that you just wanted to allow extra keys while merging the other ones. So maybe, an easier solution is for you to Merge first and then Patch the merged objects with {[K in string]: any}. That sounds more like a good, generic solution!

IMO it's better for you to use Merge (after the v7 is published) because it will bring higher type safety. Patch does not take ? into consideration and that is a problem because you can display wrong types to your users. That's why I came up with the idea above.

• But for now, I understand that you would have to change all your libs for this breaking change. So I am aliasing Merge to use Patch (which is what you needed). It will be out in a few minutes. But you'll have to change this after the v7 is out.

• There is very little documentation in general around the type system. Well, there is the TypeScript handbook, which is excellent. But that's where it stops. If you seek professional advice, I am probably one of the top 10 persons with the deepest knowledge in this field. So if you want to thank me, you can always send me tips and I will continue giving you my help. Or I could point you to a paid course I'm writing ATM :)

This is also one of the goals of the ts-toolbelt, it's to share knowledge about the type system and its intricacies, document it for everyone.

Happy to help you. Cheers!

[millsp]

After the v7 is released you should do something like this:

type merged = Patch<Merge<{
    a: string
    b?: string
}, {
    b: number
    x: number
    y?: number
}>, {
    [K in string]: any
}>

// {
//     [x: string]: any;
//     a: string;
//     b?: string | undefined;
//     x: number;
//     y?: number | undefined;
// }

You cannot do this right now, because I reverted Merge to behave like it did in the past.

[mesqueeb]

@millsp this is really great stuff. If you have time one day you could definitely write a new short article on Medium focussing on what we discussed here this week. I'm sure it'll do great!

I got you a week's worth of coffee. ;) enjoy!!!

☕

PS, which do you recommend I use:

interface Any {}
type Any = {}

(BTW, for other types throughout my libraries I usually stick to only using type and never interface because I always felt type is much more flexible when combining with & | etc.)

[millsp]

Thank you so much!

You must use interface Any {} so that you can have an alternative that does not swallow other members in the same union, but it works the same as any. But I don't see the point of doing this, because you'll loose type safety. The only nice thing about using Any, is that it can be exactly like any and preserve other type hints, whereas using any would completely absorb other members in the same union. Don't go for this solution for the merge utilities, you will have no type safety left. Don't you think the solution just above is the right choice? (if I understood your problem well).