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

intersection depth and non strict merge/Object.assign #10532

Closed
AlexGalays opened this issue Aug 25, 2016 · 12 comments
Closed

intersection depth and non strict merge/Object.assign #10532

AlexGalays opened this issue Aug 25, 2016 · 12 comments
Assignees
Labels
Suggestion An idea for TypeScript

Comments

@AlexGalays
Copy link

AlexGalays commented Aug 25, 2016

TypeScript Version: 1.8.0 and 2.0.0 beta

Code

const obj = Object.assign({}, { a: 'IamAString' }, { a: 33 })

Expected behavior:
We need a stricter behavior when we want to transform an existing object in a way that respect its type. Object.assign has a broader scope that this alone, but if one were to write his own merge function that used an intersection of types as the result, the same would happen.

Given a source object of type T and an update of type U, perhaps what's missing is being able to express the constraint "U is a sub/partial type of T"; Writing T extends U is too strong a constraint; for instance if the source object T has an optional property, we can never update it as { prop?: string } is not a valid sub type of { prop: string }

Actual behavior:
The compiler won't complain even though the second object's a key has an incompatible type with the first object's a key. T & U also intersects nested properties resulting in a lot of subsequent silent errors (when reading properties of a now too broad type) and strange type combinations (string & number ?_?)

@kitsonk
Copy link
Contributor

kitsonk commented Aug 25, 2016

This is really type operators and variadic kinds and/or programmatic access to types and/or mixin support. See: #5453, #4490, and #2919

@mhegazy mhegazy added Duplicate An existing issue was already created Suggestion An idea for TypeScript and removed Duplicate An existing issue was already created labels Aug 25, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Aug 25, 2016

as noted by @kitsonk, there is not a way to express this today in the type system. this would be done as part of the work to enable object literal reset and spread (#2103). the type operation required by spreading in an object literal is the same one needed to for a better definition for Object.assign.

@kitsonk
Copy link
Contributor

kitsonk commented Aug 25, 2016

Since there are a lot of related issues (though this isn't deemed a duplicate now) I will comment here.

the type operation required by spreading in an object literal is the same one needed to for a better definition for Object.assign.

There are still challenges though, because if it is a simple spread then you end up with duplicate members being merged and you end up with something like { a: string & number }. To properly model it, we would need an operator that reflected the actual behaviour of the code, which would result in a { a: number }.

@mhegazy
Copy link
Contributor

mhegazy commented Aug 25, 2016

To properly model it, we would need an operator that reflected the actual behaviour of the code, which would result in a { a: number }.

that is what i meant. so:

type A = { x: number };
type B = { x: string };

type C = { x: boolean, ...A, ...B } // { x: string} 

@AlexGalays
Copy link
Author

AlexGalays commented Aug 25, 2016

Just to be sure and clarify, I'm only trying to solve this problem:
Fully type safe, immutable, object transformation into another object of the same exact type

In languages with proper classes, you could use lenses or similar for that but I would prefer to stay with simple JSON structures and merge updates if possible. Until now, I was using something close to Object.assign, but was surprised to realize it was completely incorrect. Also, the code snipets found here seemed promising but all have not so edgy edge cases.

@mhegazy
Copy link
Contributor

mhegazy commented Aug 25, 2016

Fully type safe, immutable, object transformation into another object of the same exact type

but that is not what Object.assign do at runtime.

if all what you want is the error, you can define assign as:

declare function assign<T, U extends T, V extends U>(o: T, o1: U, o2: V): V;

@AlexGalays
Copy link
Author

AlexGalays commented Aug 25, 2016

Yes, Object.assign was just an example. It does much more than just type-safe merging.
I'm actually using my own function, similar to your assign snippet.

It looks like this:

export function merge<T extends Object, U extends Object>(obj: T, other: U): T & U {
  let result: any = copy(obj)
  Object.keys(other).forEach(key => result[key] = (<any>other)[key])
  return <T & U>result
}

it suffers from the bug mentionned in my first post.

Borrowing the code snippet found in that other thread:

function merge<B, U, T extends B & U>(obj: T, update: U): T {
    const result: any = {}
    for (let key in update) {
        result[key] = update[key];
    }

    return result;
}

Then we hit several edge cases, like this one (both with TS 1.8 and 2.0):

interface IFoo {
    data: number | string;
}
const a: IFoo = {data: 5};
const res = merge(a, {data: 6});

results in this error:

error TS2345: Argument of type 'IFoo' is not assignable to parameter of type '{} & { data: number; }'.
  Type 'IFoo' is not assignable to type '{ data: number; }'.
    Types of property 'data' are incompatible.
      Type 'number | string' is not assignable to type 'number'.
        Type 'string' is not assignable to type 'number'.

Of course, number | string is not something I've ever seen, but we would get this error for many type combinations.

Is it correct to say this merge/assign function cannot be expressed in a fully typesafe way with TS 2.0 ?

@mhegazy
Copy link
Contributor

mhegazy commented Aug 25, 2016

there is not a concept of a "subset" of a type. you want the second parameter to be of a type that is an all optional properties of the first parameter type. e.g. merge<T>(o:T, d: optional T): T. I think #7265 is the correct issue here, and you can find more discussion in #7004.

@AlexGalays
Copy link
Author

Thanks for the references; this need was probably expressed in quite a few others bug tickets I suspect. It's basically implementing Flow's $Shape isn't it?

@AlexGalays
Copy link
Author

As @RyanCavanaugh adviced, I'm taking the discussion of an hypothetical deepPartial or deepOption operator here.

The goal would be to make something like this function type check:

function deepUpdate<T>(obj: T, updateSpec: deepOption T)

You would then use it like this :

const initState = { a: 1, b: { c: '2' }, d: [] }
const newState = deepUpdate(initState, { b: { c: '3' } })

This is related to Immutable-js's mergeDeepIn function on Maps, see https://facebook.github.io/immutable-js/docs/#/Map/mergeDeepIn

Except theirs is not type checkable at all and can only update one key path in one go.

Also, see https://github.com/AlexGalays/immupdate though, should this change be made, its Typescript api would have to be a bit simpler, less polymorphic; this change would make it possible to type check deep, immutable updates.

Would that be feasible and something you would like to see in typescript ?

@RyanCavanaugh
Copy link
Member

New type combination operators should account for all scenarios here except for spread, which we're tracking separately

@davidfarinha
Copy link

Hi. Sorry, which type operators are you referring to? I'm also hoping to better strongly type functions that shallow/deep clone a pair of objects. Cheers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

6 participants