Add empty tuple type by lefb766 · Pull Request #14646 · microsoft/TypeScript

lefb766 · 2017-03-14T09:09:42Z

Open question: should a non-empty tuple be assignable to []?

People may think it should be, given that you can assign [number, string] to [number].
However, the index signature of [] which is never blocks such assignment.

I think this isn't going to be a problem at least for now before variadic tuples (#5453) lands.
If you need common type of all tuples, you can simply use {}[] or any[].

Is there any other problemic scenario?

Related discussion: #6229

sandersn · 2017-03-16T21:41:45Z

src/compiler/checker.ts

+            return !!getPropertyOfType(type, "0") || isEmptyTupleType(type);
+        }
+
+        function isEmptyTupleType(type: Type): boolean {


you should be to use === directly, so I think you should drop isEmptyTupleType and just add || type === emptyTupleType on line 8914

sandersn · 2017-03-16T21:43:18Z

src/compiler/checker.ts

            // Grammar checking
-            const hasErrorFromDisallowedTrailingComma = checkGrammarForDisallowedTrailingComma(node.elementTypes);
-            if (!hasErrorFromDisallowedTrailingComma && node.elementTypes.length === 0) {
-                grammarErrorOnNode(node, Diagnostics.A_tuple_type_element_list_cannot_be_empty);


You can delete this error from diagnosticsMessages.json since this is the only usage.

sandersn · 2017-03-16T21:48:53Z

tests/cases/compiler/tupleTypes.ts

+var et0 = et[0];  // never
+var et0: never;
+
+et = [];    // Ok


can you add this same check in a new file, something like tupleTypesStrictNullCheck.ts, with the first line // @strictNullChecks: true to turn on strict null checks?

lefb766 · 2017-03-20T05:14:39Z

@sandersn Your comments are already reflected

gcnew · 2017-03-20T08:57:00Z

src/compiler/checker.ts

-                    if (elementTypes.length) {
-                        return createTupleType(elementTypes);
-                    }
+                    return createTupleType(elementTypes);


I think you should update the createTupleType function to return the emptyTupleType singleton if elementTypes is empty.

Agreed. We need to make the condition persistent over future updates. A fix is pushed!

sandersn

Looks good to me. @RyanCavanaugh or @ahejlsberg can you take a look too?

mhegazy · 2017-05-23T21:51:22Z

src/compiler/checker.ts


-        function createTupleType(elementTypes: Type[]) {
+        function createTupleType(elementTypes: Type[]): TypeReference {
+            // We have to ensure that we get same instance for empty tuple type,


getTupleTypeOfArity caches the resulting tuple type, so not sure why we need special handling here.

mhegazy

usage of getTupleTypeOfArity should be sufficient

KiaraGrouwstra · 2017-08-24T12:06:32Z

I've made an attempt to revive this PR taking into account the requested changes at #17762.

lefb766 · 2017-08-24T14:20:33Z

@tycho01 oh, great! but my bad! I left this PR untouched for so long time and was even unnoticed the update on the original issue until today.

I appreciate you reused my code! Thanks!

Just wondering. Are you going to update them further and implement the additional restrictions you mentioned in #13126 (comment) ?
Currently, I'm getting difficulties finding functional update in your PR... looks they are just pure code cleaning and conflict resolution.

KiaraGrouwstra · 2017-08-24T15:44:53Z

@lefb766: right, I hadn't included those in there.

To answer by sub-topic there:

known length, fixing Proposal: strict and open-length tuple types #6229: I filed a separate PR for this at Make tuples have known length #17765. The reason I separated them was that adding empty tuples should be uncontroversial (explicitly marked as 'Accepting PRs' at Empty tuple type now meaningful (at least, in unions) #13126), so I didn't wanna lump them with another feature the TS team had so far been silent on.
ditch mutating methods: this was part of my original plan for that Make tuples have known length #17765, making a new Tuple interface extending ReadonlyArray with a length property. This caused the project to not compile anymore though, at which point I'd just quickly settled on letting it extend Array instead, as the length was my primary concern. I then got issues about it not seeing my new interface on half of the tests (wth), at which point I opted to just set that length directly from createTupleTypeForArity instead, foregoing the new interface as length had become its sole added feature.
no index signature: I wasn't initially convinced of this, largely in fear of breaking more still than at my ReadonlyArray attempt, though I've recently been swayed this change seems a good thing. I guess it means copying ReadonlyArray into a new Tuple interface, similar minus the index. I wish I understood why we have several copies of this interface in lib.d.ts instead of just having them extend one -- if extending is okay, I'd imagine ReadonlyArray could just be extending this Tuple interface to just add its index too.

lefb766 · 2017-08-25T13:56:06Z

@tycho01 Thanks for explaining. I see, you have another PR.

It turns out it will be harder to find difference between #17762 and this one if I happen to solve the conflict.
I don't have idea on how to deal with this type of situation. You have?

Your investigation on the sup-topic looks interesting though. Nice work.
I have a few opinion on the topics but it may be irrelevant to write them here.

KiaraGrouwstra · 2017-08-25T15:12:18Z

@lefb766: wanna just stick with your PR then? If you follow mhegazy's change you may be done. :)

I'd be interested in your take on the topic, but I think the others in #13126 would be too!

lefb766 · 2017-08-26T02:38:13Z

@tycho01 Fair point. I'll continue this work. Sorry for bothering you and others.

lefb766 · 2017-08-27T11:39:43Z

Should be ready for review now. @mhegazy can you take a look?

I've selectively adopted @tycho01 's work.

Note: #6524 can also be closed after merge this PR.

KiaraGrouwstra · 2018-02-14T21:57:37Z

@mhegazy: conflict aside, is there more that could be done to improve this PR?

typescript-bot · 2018-06-18T18:35:19Z

Thanks for your contribution. This PR has not been updated in a while and cannot be automatically merged at the time being. For housekeeping purposes we are closing stale PRs. If you'd still like to continue working on this PR, please leave a message and one of the maintainers can reopen it.

msftclas added cla-required and removed cla-required labels Mar 14, 2017

lefb766 force-pushed the empty-tuple branch from f0385d3 to 0e7ffda Compare March 14, 2017 09:25

sandersn reviewed Mar 16, 2017

View reviewed changes

sandersn requested changes Mar 16, 2017

View reviewed changes

lefb766 force-pushed the empty-tuple branch from 0e7ffda to 61ac5be Compare March 17, 2017 04:08

gcnew reviewed Mar 20, 2017

View reviewed changes

sandersn approved these changes Apr 5, 2017

View reviewed changes

mhegazy reviewed May 23, 2017

View reviewed changes

mhegazy suggested changes May 23, 2017

View reviewed changes

lefb766 force-pushed the empty-tuple branch from e02dcf3 to 1476eb4 Compare July 7, 2017 16:45

ditch empty tuple check

665f44f

KiaraGrouwstra mentioned this pull request Aug 13, 2017

allow empty tuple, fixes #13126 #17762

Closed

empty tuple tests all good

03bf0b8

lefb766 added 2 commits August 27, 2017 15:41

Add empty tuple type

bf14cfa

Merge branch 'tycho01:13126-empty-tuple' into empty-tuple

5cffdb2

lefb766 force-pushed the empty-tuple branch from 1476eb4 to 5cffdb2 Compare August 27, 2017 08:48

msftgits removed the cla-signed label Sep 26, 2017

microsoft deleted a comment from msftclas Sep 26, 2017

KiaraGrouwstra mentioned this pull request Jun 12, 2018

Tuples in rest parameters and spread expressions #24897

Merged

typescript-bot closed this Jun 18, 2018

microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025

Conversation

lefb766 commented Mar 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Open question: should a non-empty tuple be assignable to []?

Uh oh!

sandersn Mar 16, 2017

Choose a reason for hiding this comment

Uh oh!

sandersn Mar 16, 2017

Choose a reason for hiding this comment

Uh oh!

sandersn Mar 16, 2017

Choose a reason for hiding this comment

Uh oh!

lefb766 commented Mar 20, 2017

Uh oh!

gcnew Mar 20, 2017

Choose a reason for hiding this comment

Uh oh!

lefb766 Mar 20, 2017

Choose a reason for hiding this comment

Uh oh!

sandersn left a comment

Choose a reason for hiding this comment

Uh oh!

mhegazy May 23, 2017

Choose a reason for hiding this comment

Uh oh!

mhegazy left a comment

Choose a reason for hiding this comment

Uh oh!

KiaraGrouwstra commented Aug 24, 2017

Uh oh!

lefb766 commented Aug 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KiaraGrouwstra commented Aug 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lefb766 commented Aug 25, 2017

Uh oh!

KiaraGrouwstra commented Aug 25, 2017

Uh oh!

lefb766 commented Aug 26, 2017

Uh oh!

lefb766 commented Aug 27, 2017

Uh oh!

KiaraGrouwstra commented Feb 14, 2018

Uh oh!

typescript-bot commented Jun 18, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

lefb766 commented Mar 14, 2017 •

edited

Loading

lefb766 commented Aug 24, 2017 •

edited

Loading

KiaraGrouwstra commented Aug 24, 2017 •

edited

Loading