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

Tuples should be clonable with Array.prototype.slice() #4988

Closed
vjau opened this issue Sep 26, 2015 · 15 comments
Closed

Tuples should be clonable with Array.prototype.slice() #4988

vjau opened this issue Sep 26, 2015 · 15 comments
Assignees
Labels
Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Revisit An issue worth coming back to

Comments

@vjau
Copy link

vjau commented Sep 26, 2015

Let's say i define a tuple type like this :

type MyTuple = [number, number]

Now when i do this:

let a: MyTuple = [1, 2]
let b: MyTuple = a.slice();

The compiler complains that TS2322: Type 'number[]' is not assignable to type '[number, number]'. Property '0' is missing in type 'number[]'.

I think it should work, slice without argument (or slice(0) or even slice(0,2)) should allow to clone a tuple.

@DanielRosenwasser
Copy link
Member

Great idea - after #4910 goes in we can add an overload in lib.d.ts so that slice with no arguments returns the this type.

@DanielRosenwasser DanielRosenwasser added Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Help Wanted You can do this Good First Issue Well scoped, documented and has the green light labels Sep 27, 2015
@ahejlsberg
Copy link
Member

Just added the necessary changes to #4910. When it goes in, all we need is the extra overload in lib.d.ts.

@mhegazy mhegazy added this to the TypeScript 1.7 milestone Oct 3, 2015
@DanielRosenwasser DanielRosenwasser removed their assignment Oct 3, 2015
@DanielRosenwasser DanielRosenwasser modified the milestones: Community, TypeScript 1.7 Oct 3, 2015
@DanielRosenwasser
Copy link
Member

Actually, I'm not so sure about this anymore; if you subclass array, then you definitely aren't going to get the expected behavior when you .slice().

@mhegazy
Copy link
Contributor

mhegazy commented Oct 3, 2015

I would not say Subclassing array is the common scenario.

@DanielRosenwasser
Copy link
Member

In our compiler we have NodeArray<T>. I'm a little ambivalent about this, but I get the use-case.

@DanielRosenwasser
Copy link
Member

If we used an alternate interface instead of Array, then at least this would be slightly less deceptive. Something like

interface TupleArray<T> extends Array<T> {
    slice(): this;
}

@sandersn
Copy link
Member

sandersn commented Oct 7, 2015

As far as I understand, the deception (pun?) is built in to the re-purposing of arrays as tuples. This PR just makes the pun applicable in more places.

I don't think the result of new NodeArray<string>().slice() : NodeArray<string> is that bad either, but maybe I'm missing something.

@ahejlsberg
Copy link
Member

@sandersn The issue is that the implementation of slice isn't going to clone any additional properties you might have added in a derived class. Thus, it strictly isn't true that slice returns this. Indeed, the only time it is true is when a method actually returns the this instance.

If we change slice() to return this we're basically requiring subclasses of Array<T> to override slice and do the appropriate thing. Not necessarily a bad thing, but a bit subtle.

@DanielRosenwasser
Copy link
Member

It's also worth stating that if a user is totally sure that this is what they want, they can always add the overload themselves.

@mhegazy
Copy link
Contributor

mhegazy commented Oct 7, 2015

So the right thing is to do what @DanielRosenwasser suggested and have a new type to represent the Tuple that extends Array. one thing to note, we would not want to require users to update their library as they update the compiler (as some users have a hand crafted version of the library) so we need to come up with a solution that does not violate this constraint.

@sandersn
Copy link
Member

sandersn commented Oct 7, 2015

I need to go read the spec for tuple types and assignment. Right now I don't understand well enough how the type system supports it. I'll see if I can come up with a proposal afterwards.

@sandersn
Copy link
Member

sandersn commented Oct 8, 2015

After looking at the spec, the relevant section is 3.3.3. It gives an example of a named tuple type:

interface KeyValuePair<K, V> extends Array<K | V> { 0: K; 1: V; }

It also says that a type is said to be a tuple-like type if it has a property with the numeric name '0'. Combining these two, I guess we could change the spec to include an explicit tuple type that is the parent of all tuple types:

interface TupleArray<T> extends Array<T> { 0: any }

Then KeyValuePair becomes

interface KeyValuePair<K,V> extends TupleArray<K | V> { 0: K, 1: V }

The problem with this is that 0's type in TupleArray is any. If it's T, then KeyValuePair's must have a property 0: K | V, even though it should be 0: K. I think any is OK here because it represents what's going on with Array -- it's not a homogenous array anymore.

Now we can take @DanielRosenwasser 's proposal and put slice inside TupleArray. But to avoid promising that slice returns properties from tuple subclasses, per @ahejlsberg, we can actually make it return TupleArray<T>:

interface TupleArray<T> extends Array<T> {
    0: any;
    slice: TupleArray<T>;
}

The only hole here is that I don't know whether this satisfies @mhegazy 's constraint that users not update their lib.d.ts. Thoughts?

@DanielRosenwasser DanielRosenwasser removed Help Wanted You can do this Good First Issue Well scoped, documented and has the green light labels Oct 17, 2015
@DanielRosenwasser
Copy link
Member

Assuming you meant slice(): TupleArray<T> that doesn't work because you lose the other numeric properties that make the tuple useful (i.e. 1, 2, etc.). You'd just end up with a type that has a property 0 whose type is the union of all types in the original tuple.

@sandersn
Copy link
Member

Closing for now -- we should revisit once we have variadic kinds for tuple types so that there is at least a chance to write a type like:

interface Array<T> {
   slice<...U>(): ...U;
}

...although I'm still not sure how to relate T and ...U here.

@sandersn sandersn added the Revisit An issue worth coming back to label Nov 11, 2015
@Igorbek
Copy link
Contributor

Igorbek commented Sep 21, 2016

...although I'm still not sure how to relate T and ...U here.

You would not (T is a union of constituent types in ...U), unless tuples get first-class support like here #6229

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Revisit An issue worth coming back to
Projects
None yet
Development

No branches or pull requests

6 participants