Uh oh!
There was an error while loading. Please reload this page.
Add empty tuple type - #14646
Conversation
f0385d3 to
0e7ffdaCompare| return !!getPropertyOfType(type, "0") || isEmptyTupleType(type); | ||
| } | ||
| function isEmptyTupleType(type: Type): boolean { |
There was a problem hiding this comment.
you should be to use === directly, so I think you should drop isEmptyTupleType and just add || type === emptyTupleType on line 8914
| // Grammar checking | ||
| const hasErrorFromDisallowedTrailingComma = checkGrammarForDisallowedTrailingComma(node.elementTypes); | ||
| if (!hasErrorFromDisallowedTrailingComma && node.elementTypes.length === 0) { | ||
| grammarErrorOnNode(node, Diagnostics.A_tuple_type_element_list_cannot_be_empty); |
There was a problem hiding this comment.
You can delete this error from diagnosticsMessages.json since this is the only usage.
| var et0 = et[0]; // never | ||
| var et0: never; | ||
| et = []; // Ok |
There was a problem hiding this comment.
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?
0e7ffda to
61ac5beCompareNobuhiro Nakamura (lefb766)
commented
Mar 20, 2017
Nathan Shively-Sanders (@sandersn) Your comments are already reflected |
| if (elementTypes.length) { | ||
| return createTupleType(elementTypes); | ||
| } | ||
| return createTupleType(elementTypes); |
There was a problem hiding this comment.
I think you should update the createTupleType function to return the emptyTupleType singleton if elementTypes is empty.
There was a problem hiding this comment.
Agreed. We need to make the condition persistent over future updates. A fix is pushed!
Nathan Shively-Sanders (sandersn)
left a comment
There was a problem hiding this comment.
Looks good to me. Ryan Cavanaugh (@RyanCavanaugh) or Anders Hejlsberg (@ahejlsberg) can you take a look too?
| function createTupleType(elementTypes: Type[]) { | ||
| function createTupleType(elementTypes: Type[]): TypeReference { | ||
| // We have to ensure that we get same instance for empty tuple type, |
There was a problem hiding this comment.
getTupleTypeOfArity caches the resulting tuple type, so not sure why we need special handling here.
Mohamed Hegazy (mhegazy)
left a comment
There was a problem hiding this comment.
usage of getTupleTypeOfArity should be sufficient
e02dcf3 to
1476eb4Comparekiara (KiaraGrouwstra)
commented
Aug 24, 2017
I've made an attempt to revive this PR taking into account the requested changes at #17762. |
@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) ? |
Nobuhiro Nakamura (@lefb766): right, I hadn't included those in there. To answer by sub-topic there:
|
Nobuhiro Nakamura (lefb766)
commented
Aug 25, 2017
@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. Your investigation on the sup-topic looks interesting though. Nice work. |
kiara (KiaraGrouwstra)
commented
Aug 25, 2017
Nobuhiro Nakamura (@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! |
Nobuhiro Nakamura (lefb766)
commented
Aug 26, 2017
@tycho01 Fair point. I'll continue this work. Sorry for bothering you and others. |
1476eb4 to
5cffdb2CompareNobuhiro Nakamura (lefb766)
commented
Aug 27, 2017
Should be ready for review now. Mohamed Hegazy (@mhegazy) can you take a look? I've selectively adopted @tycho01 's work. Note: #6524 can also be closed after merge this PR. |
kiara (KiaraGrouwstra)
commented
Feb 14, 2018
Mohamed Hegazy (@mhegazy): conflict aside, is there more that could be done to improve this PR? |
TypeScript Bot (typescript-bot)
commented
Jun 18, 2018
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. |
Fixes#13126
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
neverblocks 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
{}[]orany[].Is there any other problemic scenario?
Related discussion: #6229