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

Spread type #13288

Closed
wants to merge 11 commits into from
Closed

Spread type #13288

wants to merge 11 commits into from

Conversation

sandersn
Copy link
Member

@sandersn sandersn commented Jan 4, 2017

Fixes #10727
Fixes #11100

Follow-up to #11150 — most discussion has already taken place there; this PR just updates the previously-unreleased spread type code.

@sandersn
Copy link
Member Author

sandersn commented Jan 4, 2017

@ahejlsberg this is the PR I was talking about.

@@ -11673,7 +11867,7 @@ namespace ts {
typeFlags = 0;
}
const type = checkExpression((memberDecl as SpreadAssignment).expression);
if (!isValidSpreadType(type)) {
if (type.flags & (TypeFlags.NumberLike | TypeFlags.StringLike | TypeFlags.BooleanLike | TypeFlags.EnumLike | TypeFlags.ESSymbol)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

should just change the definition of isValidSpreadType

@dxiao
Copy link

dxiao commented May 2, 2017

Any chance this can merge in sometime for 2.3.3? Proving kinda annoying to deal with...

@ccschmitz
Copy link

Same for us, @dxiao. We get a pile of new errors on 2.3. Holding out on the upgrade because of it. We are holding out on the upgrade in hopes a fix like this makes it out in a patch release.

@mhegazy
Copy link
Contributor

mhegazy commented May 4, 2017

This is a new feature, and will not be added in a patch release.

@alex-sherwin
Copy link

alex-sherwin commented May 8, 2017

This is a new feature, and will not be added in a patch release.

I think people who were already using the spread operator in TSX would argue, purely from a consumer of TypeScript perspective, this is not a new feature, but a regression.

If it's not going to make it into a patch release, then we're waiting until at least 2.4, which is how far off? So anyone who wants to use spread operators in TSX (which is pretty much a requirement for React + react-router) we're going to have to skip 2.3 entirely

@prencher
Copy link

prencher commented May 8, 2017

@mhegazy This use case broke with 2.3 from 2.2, even if the behavior may not have been correct prior to 2.3. I would highly urge you to fix it in 2.3, it is a major regression.

@jvbianchi
Copy link

Is this ready for merge?

@sandersn
Copy link
Member Author

@jvbianchi No. See preceding discussion.

@robyoder
Copy link

robyoder commented Jan 24, 2018

@sandersn Your July 18 comment doesn't address positive extend scenarios, or maybe I'm misunderstanding. If I have T extends { a: string } I should be able to do const newThing: T = { ...oldThing, a: "something" }; for oldThing: T. Right?

Your comment seems to acknowledge the difficulties with assigning properties to completely unknown Ts, but if you have expressed what T extends, you should be able to spread T with those properties because the types are defined.

@sandersn
Copy link
Member Author

@robyoder what if oldThing: { a: "foo" | "bar" } ? Then the assignment is incorrect because "something" isn't foo or bar.

@robyoder
Copy link

Yeah that's unfortunate. Too bad there's no way to force invariance in certain places.

@robyoder
Copy link

robyoder commented Jan 24, 2018

@sandersn but variance isn't really a holdup, right? TS already fails there:

const func = <T extends { a: string }>(param: T): T => {
	const cloned: T = clone(param);
	cloned.a = "something";
	return cloned;
};

interface Foo {
	a: "foo" | "bar";
}
const data: Foo = { a: "foo" };
const data2: Foo = func(data); // this type checks, but `a` is now "something", not "foo" or "bar"

@sandersn
Copy link
Member Author

@robyoder Good point. What is the scenario you would like to work? Constraining T to require the properties you are about to add doesn't help the React HOC scenario as far as I understand it, because the whole point is that the caller doesn't have those properties.

@robyoder
Copy link

Yeah, I'm not using React HOCs atm. In this specific case, I have an object with a default id of 0, and I'm saving it to storage and getting it back with the id set properly. So something like this:

const addEntry = <T extends { id: number }>(entry: T): T => {
    const id = entry.id > 0 ? entry.id : getTheNextId();
    const newEntry = { ...entry, id };
    return saveToStorage(newEntry); // returns newEntry after saving it
}

And right now that obviously gives me the error Spread types may only be created from object types.

@DJWassink
Copy link

Just a little headsup for people ending up here. It seems like Object.assign does work with generic method, little example I found msyself doing in my code:

    public addSomethingToBar<T extends IBar>(bar: T): T & {foo: number[]} {
        return Object.assign({}, bar, {foo: [1, 2]});
    }

@burabure
Copy link

burabure commented Apr 2, 2018

any updates on this?, we really need something better than

// Cant use spread here because of TS issue, see TypeScript #10727 #13288
// tslint:disable-next-line:prefer-object-spread
this.setState(prevState => Object.assign({}, prevState, newState))
...

@zheeeng
Copy link

zheeeng commented Apr 9, 2018

expecting on progress.

@lukescott
Copy link

lukescott commented Apr 12, 2018

I am hitting this issue all the time since I work with immutable objects frequently with Redux.

@Ky6uk
Copy link

Ky6uk commented Apr 13, 2018

Hmm. I am also getting the issue when trying to use generics for React HOCs with spread params.

interface InjectedProps {
  readonly p1: string;
  readonly p2: number;
}

function hoc<P extends InjectedProps>(Component: React.ComponentType<P>) {
  // ERROR: Rest types may only be created from object types.
  return ({ p1, p2, ...rest } : P) =>
    <Wrapper p1={p1} p2={p2}>
      <Component {...rest} />
    </Wrapper>
  ;
}

Do we have an alternative version for solving the issue?

@lukescott
Copy link

Besides using Object.assign, you can tack on as any, but then you lose the strict typing. Not ideal.

@goodmind
Copy link

Use this

/**
 * Get all names of properties with types that include undefined.
 */
export type OptionalPropNames<T> = { [K in keyof T]: undefined extends T[K] ? K : never }[keyof T];

/**
 * Common properties from L and R with undefined in R[K] replaced by type in L[K]
 */
export type SpreadProps<L, R, K extends keyof L & keyof R> =
    { [P in K]: L[P] | Exclude<R[P], undefined> };

/**
 * Type of `{ ...L, ...R }` / `Object.assign(L, R)`.
 */
export type Spread<L, R> =
    /** properties in L that don't exist in R */
      Pick<L, Exclude<keyof L, keyof R>>
    /** properties in R with types that exclude undefined */
    & Pick<R, Exclude<keyof R, OptionalPropNames<R>>>
    /** properties in R, with types that include undefined, that don't exist in L */
    & Pick<R, Exclude<OptionalPropNames<R>, keyof L>>
    /** properties in R, with types that include undefined, that exist in L */
    & SpreadProps<L, R, OptionalPropNames<R> & keyof L>;

@joaovieira
Copy link

Bump. Where is this going? Still waiting.. 😕

@ColCh
Copy link

ColCh commented Jun 8, 2018

<offtop>
#13288 (comment) yeah we have generic jsx, conditional types, other kill-features, but no rest props type, even in TS 3.0 in master branch by this very moment

https://github.com/Microsoft/TypeScript/blob/f17fe8713e05756babc72720e091bbefc83f5135/package.json#L5

original commit in this PR was created as TS was 2.2

https://github.com/Microsoft/TypeScript/blob/c2562b43ddb2e3df01b8892302579e3d26dace6f/package.json#L5

it seems that this is simply not a priority :(
</offtop>

hey @sandersn @ahejlsberg, I'm sorry for mentioning you, but it seems that this PR got lost in stream of other issues? Any possibility to see this in TS@3 or later?

@mfferreira
Copy link

Bump

@perjerz
Copy link

perjerz commented Aug 10, 2018

any update?

@RyanCavanaugh
Copy link
Member

As everyone can probably tell, this PR didn't get merged and is now painfully out of date.

Mapped and conditional types meet a very large proportion of the use cases here. Here's an example of combining two types including exclusion and overwriting:

type Type1 = { x: number, y: number };
type Type2 = { name: string, kind: string, y: string; };

type Spread<T1, T2, TExclude> = { [K in Exclude<keyof T1, TExclude | keyof T2>]: T1[K] } & { [K in Exclude<keyof T2, TExclude>]: T2[K] };

type Type3 = Spread<Type1, Type2, "name">
declare const t3: Type3;

t3.

image

We'd like people bumping/thumbing/pinging this PR to try using these sorts of types in their scenarios to see what other use cases still need to be designed around if we do include an explicit spread type syntax - see the original PR comments for links. Thanks!

@microsoft microsoft locked and limited conversation to collaborators Aug 10, 2018
@jakebailey jakebailey deleted the spread-type branch November 7, 2022 17:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet