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

Generic spread expressions in object literals #28234

Merged
merged 8 commits into from
Oct 31, 2018
Merged

Conversation

ahejlsberg
Copy link
Member

With this PR we permit generic types in spread expressions within object literals. Object literals with generic spread expressions now produce intersection types, similar to the Object.assign function and JSX literals. For example:

function taggedObject<T, U extends string>(obj: T, tag: U) {
    return { ...obj, tag };  // T & { tag: U }
}

let x = taggedObject({ x: 10, y: 20 }, "point");  // { x: number, y: number } & { tag: "point" }

Property assignments and non-generic spread expressions are merged to the greatest extent possible on either side of a generic spread expression. For example:

function foo1<T>(t: T, obj1: { a: string }, obj2: { b: string }) {
    return { ...obj1, x: 1, ...t, ...obj2, y: 2 };  // { a: string, x: number } & T & { b: string, y: number }
}

Non-generic spread expressions continue to be processed as before: Call and construct signatures are stripped, only non-method properties are preserved, and for properties with the same name, the type of the rightmost property is used. This contrasts with intersection types which concatenate call and construct signatures, preserve all properties, and intersect the types of properties with the same name. Thus, spreads of the same types may produce different results when they are created through instantiation of generic types:

function spread<T, U>(t: T, u: U) {
    return { ...t, ...u };  // T & U
}

declare let x: { a: string, b: number };
declare let y: { b: string, c: boolean };

let s1 = { ...x, ...y };  // { a: string, b: string, c: boolean }
let s2 = spread(x, y);    // { a: string, b: number } & { b: string, c: boolean }
let b1 = s1.b;  // string
let b2 = s2.b;  // number & string

An alternative to using intersections would be to introduce a higher-order type operator { ...T, ...U } along the lines of what #10727 explores. While this appears attractive at first, it would take a substantial amount of work to implement this new type constructor and endow it with all the capabilities we already implement for intersection types, and it would produce little or no gain in precision for most scenarios. In particular, the differences really only matter when spread expressions involve objects with overlapping property names that have different types. Furthermore, for an unconstrained type parameter T, the type { ...T, ...T } wouldn't actually be assignable to T, which, while technically correct, would be pedantically annoying.

Having explored both options, and given that intersection types are already used for Object.assign and JSX literals, we feel that intersection types strike the best balance between accuracy and complexity for this feature.

@hatamov
Copy link

hatamov commented Oct 30, 2018

Awesome, this is my most anticipated feature! One thing I don't quite understand is how will this work with rest spread operator? For example what inferred return type this function will have?

function foo<T extends { x: string }>(obj: T) {
    const { x, ...rest } = obj;
    return rest;
}

@ahejlsberg
Copy link
Member Author

@hatamov This PR doesn't change the behavior of rest properties. However, I'm considering a separate PR that would use the Pick and Exclude predefined types to compute rest types:

function foo<T extends { a: string, b: string }>(obj: T) {
    let { a, b, ...rest } = obj;  // let rest: Pick<Exclude<T, 'a' | 'b'>>
}

@ajafff
Copy link
Contributor

ajafff commented Oct 30, 2018

@ahejlsberg Would it be possible to use Pick and Exclude to more precisely construct the resulting type of spreading an instantiable type?

function spread<T, U>(t: T, u: U) {
    return { ...t, ...u };  // Pick<T, Exclude<keyof T, keyof U>> & U
}

Of course this needs to be a little bit more complex to correctly handle optional properties.

@ahejlsberg
Copy link
Member Author

Would it be possible to use Pick and Exclude to more precisely construct the resulting type of spreading an instantiable type?

It's possible, see for example my comment here. However, it ends up generating very complex types (and commensurately complex error messages), and even then we still lack accurate representation of which properties were declared optional and which properties are "own enumerable" properties.

Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Test cleanup
  2. Higher-order rest should be an error until we address it too.

@ahejlsberg
Copy link
Member Author

@sandersn Your comments should be addressed now. In some cases I moved the tests, in others I just updated the comments.

@sandersn sandersn self-assigned this Oct 31, 2018
@@ -17697,9 +17718,8 @@ namespace ts {
}

function isValidSpreadType(type: Type): boolean {
return !!(type.flags & (TypeFlags.AnyOrUnknown | TypeFlags.NonPrimitive) ||
return !!(type.flags & (TypeFlags.AnyOrUnknown | TypeFlags.NonPrimitive | TypeFlags.Object | TypeFlags.InstantiableNonPrimitive) ||
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this InstantiableNonPrimitive case, shouldn't this be checking that the constraint of type, if it exists, isValidSpreadType?

@jack-williams
Copy link
Collaborator

Thus, spreads of the same types may produce different results when they are created through instantiation of generic types

Is the following behaviour also grouped under this description?

function ripoff<T>(augmented: T): T {
  const { ...original } = augmented;
  return original;
}

interface Foo {
  x: number;
  (y: string): number;
}

const foo = (x: string) => x.length;
foo.x = 42;
const x: Foo = ripoff(foo);
x("string") // TypeError: x is not a function

Did the team consider returning the type Pick<T, keyof T> instead?

@sandersn
Copy link
Member

@jack-williams Yes, except that's rest, not spread. The principle is the same though. We think that pattern of spreading a callable object is rare, especially in generic functions like ripoff. (Non-generic uses are already correct.)

@jack-williams
Copy link
Collaborator

@sandersn

Yes, except that's rest, not spread.

Yep sorry, silly mistake.

We think that pattern of spreading a callable object is rare, especially in generic functions like ripoff.

I agree that the callable case is rare (and contrived) but there are other object invariants someone might expect to be preserved by a function of type <X>(x: X) => X. The current choice seems to be invested in either having rest preserve all these invariants (current and future new object features) or having the violation of these invariants being suitable rare or low-impact as to not matter.

I expect my concerns are unfounded, it just seems that types like Pick are exactly what you want in this situation.

@JasonKleban
Copy link

Is this expected? Playground

function func2<T extends { a: string, b: string }>(param: T)
    : { a: string, b: string } {
    const { a, ...rest } = param;
    const x = rest.b; // Property 'b' does not exist on type 'Pick<T, Exclude<keyof T, "a">>'.
    return { a, ...rest }; // Property 'b' is missing in type '{ a: string; } & Pick<T, Exclude<keyof T, "a">>' 
                           // but required in type '{ a: string; b: string; }'.
}

@ahejlsberg
Copy link
Member Author

@JasonKleban That was fixed by #29121, give it a try with typescript@next.

@Porges
Copy link
Member

Porges commented Jun 18, 2019

A bit of an aside to Anders' comment:

Would it be possible to use Pick and Exclude to more precisely construct the resulting type of spreading an instantiable type?

It's possible, see for example my comment here. However, it ends up generating very complex types (and commensurately complex error messages), and even then we still lack accurate representation of which properties were declared optional and which properties are "own enumerable" properties.

I was looking at typing Object.assign better, and there's a few modifications that could be made to
Omit make it generate (somewhat) nicer-looking types, which helps with intellisense inspection of inferred types, etc.

Given:

let x_1 = { x: 1 };
let x_true = { x: true };
let x_string = { x: "hello" };

If we wanted to type assign 'more precisely' (but not 'most precisely'?) we could write:

const assign3 =
    <T, U, V> (first: T, second: U, third: V)
        : (Omit<T, keyof (U&V)> & Omit<U, keyof V> & V) =>
        Object.assign(first, second, third) as any;

The library Omit type is:

type Omit<T, K extends keyof any> = Pick<T, Exclude<keyof T, K>>

The generated result type looks pretty bad:

image

To improve it, we can handle the "exclude everything" case like so:

type Omit<T, K extends keyof any> =
    Exclude<keyof T, K> extends never
    ? {}
    : Pick<T, Exclude<keyof T, K>>;

This greatly simplifies the generated type:
image

However, if we add back more properties...

let x_1 = { x: 1, j: 2 };
let x_true = { x: true, k: 2 };
let x_string = { x: "hello", l: 2 };

We are again in "scary type" territory:
image

To improve this we can inline Pick into the definition:

type Omit<T, K extends keyof any> =
    Exclude<keyof T, K> extends never
    ? {}
    : { [k in Exclude<keyof T, K>]: T[k] };

And we end up with something a bit nicer (although there seems to be no way to force & to reduce):
image

I'm not sure if this is worth doing to Omit or if there should be some attribute that can be applied to types like Pick saying that they are "transparent" and should always be fully expanded, never shown.

@WilliamPeralta
Copy link

WilliamPeralta commented Oct 13, 2021

interface InputProps {
    name: string
    value: any
}


interface SelectProps extends InputProps{
    items: any[]
}

export const Input =  (props : InputProps) => {
    return <div>Sono un input</div>
}

export const Select =  (props : SelectProps) => {
    return <div>Sono un Select</div>
}



type PropsOf<T> = T extends React.ComponentType<infer Props> ? Props : object

// type BPropsOf<T> = PropsOf<BaseFieldProps<T>>

type BaseFieldProps<T> = {
    field1: string
    field2: boolean
    component: React.ComponentType<PropsOf<T>>
} & {
    [Property in keyof PropsOf<T>]: PropsOf<T>[Property]
}


// type FieldProps = BaseFieldProps<InputProps>  // & PropsOf<BaseFieldProps<InputProps>['component']>

//  & PropsOf<FieldProps['component']> 
function Field < C >(props : BaseFieldProps<C>){
    return <div>Sono un field</div>
}


export const Home =  () => {
    return (
        <div>
            <h2>I am body</h2>
            <Field<typeof Select>
                field1="field1"
                field2    
                component={Select}
               {/* Here all the properties of the component */}
            />
        </div>
    )
}

There is a way to avoid having to write the generic "typeof Select" near "Field" and extract it from the prop "component"??

MajorLift added a commit to MetaMask/core that referenced this pull request Feb 22, 2024
…,Config}`

- Previous code is inconsistent with jsdoc description: "Both initial state and initial configuration options are merged with defaults upon initialization."
- Generic spread expressions for object literals are supported by TypeScript: microsoft/TypeScript#28234
MajorLift added a commit to MetaMask/core that referenced this pull request Feb 22, 2024
…,Config}`

- Previous code is inconsistent with jsdoc description: "Both initial state and initial configuration options are merged with defaults upon initialization."
- Generic spread expressions for object literals are supported by TypeScript: microsoft/TypeScript#28234
MajorLift added a commit to MetaMask/core that referenced this pull request Feb 22, 2024
…,Config}`

- Previous code is inconsistent with jsdoc description: "Both initial state and initial configuration options are merged with defaults upon initialization."
- Generic spread expressions for object literals are supported by TypeScript: microsoft/TypeScript#28234
MajorLift added a commit to MetaMask/core that referenced this pull request Feb 22, 2024
- Generic spread expressions for object literals are supported by TypeScript: microsoft/TypeScript#28234
MajorLift added a commit to MetaMask/core that referenced this pull request Feb 26, 2024
- Generic spread expressions for object literals are supported by TypeScript: microsoft/TypeScript#28234
MajorLift added a commit to MetaMask/core that referenced this pull request Feb 27, 2024
## Explanation

- For runtime property assignment, use `as unknown as` instead of `as
any`.
- Change the types for `BaseControllerV1` class fields `initialConfig`,
`initialState` from `C`, `S` to `Partial<C>`, `Partial<S>`.
- Initial user-supplied constructor options do not need to be complete
`C`, `S` objects, since `internal{Config,State}` will be populated with
`default{Config,State}`.
- For empty objects, prefer no type assertions or `as never` (`never` is
assignable to all types).
- Fix code written based on outdated TypeScript limitation.
- Generic spread expressions for object literals are supported by
TypeScript: microsoft/TypeScript#28234

## References

- Closes #3715 

## Changelog

###
[`@metamask/base-controller`](https://github.com/MetaMask/core/pull/3959/files#diff-a8212838da15b445582e5622bd4cc8195e4c52bcf87210af8074555f806706a9)

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've highlighted breaking changes using the "BREAKING" category
above as appropriate
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

Successfully merging this pull request may close these issues.

9 participants