GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Make tuples have known length by KiaraGrouwstra · Pull Request #17765 · microsoft/TypeScript

Conversation

@KiaraGrouwstra

@KiaraGrouwstra kiara (KiaraGrouwstra) commented Aug 13, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Add fixed length for tuples following Aleksey-Bykov's a suggestion in #13239.
Fixes #6229, so [number, number] would not match [number] (length 2 wouldn't match 1).
I put this behind a strictTuples flag so people can also still use the old behavior.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

michaeljota, SlurpTheo, Akiq2016, aluanhaddad, trotyl, yanchith, jcalz, MIrinkov, and krryan reacted with hooray emoji aluanhaddad, Akiq2016, yanchith, gigobyte, and krryan reacted with heart emoji
All reactions
  • 🎉 9 reactions
  • ❤️ 5 reactions
@msftclas Microsoft Contribution License Agreements (msftclas) added the cla-signed label Aug 13, 2017
@KiaraGrouwstra kiara (KiaraGrouwstra) mentioned this pull request Aug 13, 2017
Proposal: strict and open-length tuple types #6229
Closed
@aluanhaddad

Aluan Haddad (aluanhaddad) commented Aug 13, 2017

Copy link
Copy Markdown
Contributor

Shouldn't the length property of the new interface be readonly?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@zpdDG4gta8XKpMCd

ZpdDG4gta (zpdDG4gta8XKpMCd) commented Aug 13, 2017

Copy link
Copy Markdown

practically it doesn't have to be, consider:

interface X { length: 0; }
const x: X = { length: 0 };
x.length = Math.random(); // <-- type error
x.length = 1; // <-- type error
x.length = 0; // <-- allowed, but why?

aluanhaddad and sandersn reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@KiaraGrouwstra

kiara (KiaraGrouwstra) commented Aug 14, 2017

Copy link
Copy Markdown
Contributor Author

It hadn't occurred to me; Array was missing readonly from length too. Adding it there makes it not compile anymore, but on Tuple it's fine, and seems consistent with most other interfaces with length-like properties. I'll go along then.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@KiaraGrouwstra kiara (KiaraGrouwstra) mentioned this pull request Aug 14, 2017
add compiler flag to enable granular inference outside var/let #17785
Closed
@dead-claudia

Claudia Meadows (dead-claudia) commented Aug 15, 2017

Copy link
Copy Markdown

I'd probably add readonly length: TLength just to be a little more semantically descriptive of the type, even if it doesn't really affect anything in practice.

Note that for dumb reasons, readonly is assignable to read-write types, but that's already been reported by someone else.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@KiaraGrouwstra

kiara (KiaraGrouwstra) commented Aug 15, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

@isiahmeadows: Yeah, changed it.

I feel a bit on the fence about #13347; I hope assignability from this Tuple -> ReadonlyArray -> arrays, if viable, could facilitate granular inference through #17785.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

kiara (KiaraGrouwstra) added a commit to KiaraGrouwstra/TypeScript that referenced this pull request Aug 16, 2017
@KiaraGrouwstra
try testing with flag on
da19598
tuple-array assignability blocked by microsoft#17765
@KiaraGrouwstra kiara (KiaraGrouwstra) mentioned this pull request Aug 17, 2017
Fantasy-land compatibility gcanti/fp-ts#204
Closed
@KiaraGrouwstra
add strictTuples flag giving tuples known length
1f77317
@KiaraGrouwstra
kiara (KiaraGrouwstra) force-pushed the 6229-known-length-tuples branch from fa530ff to 1f77317 Compare August 19, 2017 18:56
@KiaraGrouwstra kiara (KiaraGrouwstra) changed the title prepare Tuple interface for tuples add strictTuples flag giving tuples known length Aug 19, 2017
@KiaraGrouwstra

kiara (KiaraGrouwstra) commented Aug 19, 2017

Copy link
Copy Markdown
Contributor Author

Update: should be good now, ready for feedback.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@KiaraGrouwstra

kiara (KiaraGrouwstra) commented Aug 20, 2017

Copy link
Copy Markdown
Contributor Author

Travis randomly complaining:

npm ERR! code EPEERINVALID
npm ERR! peerinvalid The package [email protected] does not satisfy its siblings' peerDependencies requirements!
npm ERR! peerinvalid Peer [email protected] wants typescript@>=2.1.0 || >=2.1.0-dev || >=2.2.0-dev || >=2.3.0-dev || >=2.4.0-dev || >=2.5.0-dev || >=2.6.0-dev
npm ERR! peerinvalid Peer [email protected] wants typescript@~2.0.3 || >=2.0.0-dev || >=2.1.0-dev || >=2.2.0-dev || >=2.3.0-dev || >=2.4.0-dev || >=2.5.0-dev
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ikatyang

Ika (ikatyang) commented Aug 20, 2017

Copy link
Copy Markdown
Contributor

Tests should be fixed by ivogabe/gulp-typescript#536.

KiaraGrouwstra reacted with thumbs up emoji KiaraGrouwstra reacted with hooray emoji
All reactions
  • 👍 1 reaction
  • 🎉 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@KiaraGrouwstra kiara (KiaraGrouwstra) mentioned this pull request Aug 20, 2017
loosen number index check, fixes #15768 #17767
Merged
@ikatyang Ika (ikatyang) mentioned this pull request Aug 21, 2017
Proposal: Support spread operator for tuples in function calls #17934
Closed
kiara (KiaraGrouwstra) added 2 commits August 22, 2017 13:27
@KiaraGrouwstra
Merge branch 'master' into 6229-known-length-tuples
841afa1
@KiaraGrouwstra
Merge branch 'master' into 6229-known-length-tuples
fe45f62
@KiaraGrouwstra

kiara (KiaraGrouwstra) commented Aug 24, 2017

Copy link
Copy Markdown
Contributor Author

Merged for retry, Travis seems good again. Thanks Ika (@ikatyang).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This was referenced Aug 24, 2017
Add empty tuple type #14646
Closed
Empty tuple type now meaningful (at least, in unions) #13126
Closed
@msftgits Microsoft GitHub User (msftgits) removed the cla-signed label Sep 26, 2017
@microsoft Microsoft (microsoft) deleted a comment from Microsoft Contribution License Agreements (msftclas) Sep 27, 2017
@Igorbek Igor Oleinikov (Igorbek) mentioned this pull request Oct 31, 2017
Tuple types accepting incorrect data #19463
Closed
Nathan Shively-Sanders (sandersn) added 3 commits November 1, 2017 11:51
@sandersn
Merge branch 'master' into 6229-known-length-tuples
b228736
@sandersn
Add "strictTuples" to list of strict flags
5d7e87a
@sandersn
Move strict tuple test and add a couple of cases
defd32f
sandersn
Nathan Shively-Sanders (sandersn) approved these changes Nov 1, 2017
View reviewed changes

@sandersn Nathan Shively-Sanders (sandersn) left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I like this change, but I'm going to try implementing tuple-freshness first to see if it works well too.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread src/compiler/checker.ts Outdated
if (strictTuples) {
const lengthSymbol = createSymbol(SymbolFlags.Property, "length" as __String);
lengthSymbol.type = getLiteralType(arity);
lengthSymbol.checkFlags = CheckFlags.Readonly;

@sandersn Nathan Shively-Sanders (sandersn) Nov 1, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I don't see how adding Readonly helps if the type is literally 2, because only 2 can be assigned to it. If the intent is future-proofing in case Readonly will someday propagate through assignments, I think that it's as likely to harm as it is to help.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@KiaraGrouwstra kiara (KiaraGrouwstra) Nov 1, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Yeah, fair enough. I had no special intentions in adding it. Feel free to adjust as you see fit! :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@sandersn

Nathan Shively-Sanders (sandersn) commented Nov 7, 2017

Copy link
Copy Markdown
Member
  • Only one additional error in firebase, but it's the same usage that was already flagged by the strict-length, discussed above.
  • No changes in Definitely Typed.
  • No changes in internal MS projects.
  • A few tests changes to assert that push and pop do not work on tuples.

Given that the total impact of this change is 3 good breaks (firebase, leaflet and highcharts), plus incorrect array syntax (9), I think that it is small enough to ship without a flag.

SlurpTheo reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sandersn Nathan Shively-Sanders (sandersn) mentioned this pull request Nov 7, 2017
Add DefinitelyTyped runner #19815
Merged
@sandersn
Revert "Add TupleBase with unusable mutating Array methods"
c1c7926
This reverts commits
4385444,
2399d58,
es55de3,
888da3c
@sandersn

Nathan Shively-Sanders (sandersn) commented Nov 8, 2017

Copy link
Copy Markdown
Member

We had one more meeting, and decided to ship this PR without a flag and without TupleBase. Shipping without a flag allows everybody to get the better checking with no work, and more easily unlocks future features based on fixed-length tuples. TupleBase is a nice idea in theory, but it's too cute in relying on the specifics of never assignability, and we suspect it will get us in trouble later.

KiaraGrouwstra, aluanhaddad, and Artazor reacted with thumbs up emoji
All reactions
  • 👍 3 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sandersn
Nathan Shively-Sanders (sandersn) merged commit 90f87ef into microsoft:master Nov 8, 2017
@sandersn

Nathan Shively-Sanders (sandersn) commented Nov 8, 2017

Copy link
Copy Markdown
Member

Thanks @tycho01!

mightyiam, Igorbek, KiaraGrouwstra, aluanhaddad, SlurpTheo, leonard-thieu, and Akiq2016 reacted with hooray emoji
All reactions
  • 🎉 7 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mhegazy

Mohamed Hegazy (mhegazy) commented Nov 8, 2017

Copy link
Copy Markdown
Contributor

Nathan Shively-Sanders (@sandersn) please add a note about this change in the breaking change section for TS 2.7.

We need also some guidance for the firebase use case

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mightyiam

Shahar "Dawn" Or (mightyiam) commented Nov 9, 2017

Copy link
Copy Markdown

Hooray!

aluanhaddad reacted with hooray emoji
All reactions
  • 🎉 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Igorbek

Igor Oleinikov (Igorbek) commented Nov 9, 2017

Copy link
Copy Markdown
Contributor

@tycho01 Nathan Shively-Sanders (@sandersn) you are the best! Thank you! Nathan Shively-Sanders (@sandersn) now this should unblock #5453, right? :)

KiaraGrouwstra and aluanhaddad reacted with heart emoji
All reactions
  • ❤️ 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mhegazy Mohamed Hegazy (mhegazy) changed the title add strictTuples flag giving tuples known length Make tuples have known length Nov 9, 2017
@sandersn Nathan Shively-Sanders (sandersn) mentioned this pull request Nov 9, 2017
SpecWatchFilter type in Firestore tests will not compile with upcoming Typescript 2.7 firebase/firebase-js-sdk#309
Closed
@KiaraGrouwstra

kiara (KiaraGrouwstra) commented Nov 9, 2017

Copy link
Copy Markdown
Contributor Author

Thanks for the merge! At this rate I may get the motivation to finish those last few PRs too. :)

Igorbek, leonard-thieu, Akiq2016, yanchith, and michaeljota reacted with thumbs up emoji aluanhaddad, s-ve, and Akiq2016 reacted with hooray emoji s-ve, leonard-thieu, Akiq2016, and hax reacted with heart emoji
All reactions
  • 👍 5 reactions
  • 🎉 3 reactions
  • ❤️ 4 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sandersn Nathan Shively-Sanders (sandersn) mentioned this pull request Nov 9, 2017
Fix incorrect tuple usage and syntax DefinitelyTyped/DefinitelyTyped#21387
Merged
Giulio Canti (gcanti) added a commit to gcanti/io-ts that referenced this pull request Nov 10, 2017
@gcanti
fix tuples fixed length bug (ts v2.7)
787f5fb
Reference: microsoft/TypeScript#17765
Giulio Canti (gcanti) added a commit to gcanti/io-ts that referenced this pull request Nov 10, 2017
@gcanti
fix tuples fixed length bug (ts v2.7)
8c4e7e5
Reference: microsoft/TypeScript#17765
@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) mentioned this pull request Nov 17, 2017
tuple type don't check number of elements #5259
Closed
Giulio Canti (gcanti) added a commit to gcanti/io-ts that referenced this pull request Nov 27, 2017
@gcanti
fix tuples fixed length bug (ts v2.7)
efaef31
Reference: microsoft/TypeScript#17765
Giulio Canti (gcanti) added a commit to gcanti/io-ts that referenced this pull request Nov 27, 2017
@gcanti
fix tuples fixed length bug (ts v2.7)
38d7c90
Reference: microsoft/TypeScript#17765
Giulio Canti (gcanti) added a commit to gcanti/io-ts that referenced this pull request Nov 27, 2017
@gcanti
fix tuples fixed length bug (ts v2.7)
dc7a7e3
Reference: microsoft/TypeScript#17765
@ghost ghost mentioned this pull request Dec 7, 2017
Types of property length are incompatible when extending tuple types. #20539
Closed
@michaeljota

Michael De Abreu (michaeljota) commented Dec 24, 2017

Copy link
Copy Markdown

This should be added to the January release notes roadmap.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@michaeljota Michael De Abreu (michaeljota) mentioned this pull request Dec 27, 2017
Poor tuple support #20899
Closed
@KiaraGrouwstra kiara (KiaraGrouwstra) mentioned this pull request Dec 28, 2017
path methods typing typed-typings/npm-ramda#269
Open
@sandersn

Nathan Shively-Sanders (sandersn) commented Jan 2, 2018

Copy link
Copy Markdown
Member

Michael De Abreu (@michaeljota) I added it to the roadmap.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@dotansimha Dotan Simha (dotansimha) mentioned this pull request Feb 5, 2018
Field in type IntrospectionResultData defined incorrectly apollographql/apollo-client#2959
Closed
@ghost ghost mentioned this pull request Feb 7, 2018
Useless compile error on triple-equal 1|2 against 0 #21724
Closed
@ahejlsberg Anders Hejlsberg (ahejlsberg) mentioned this pull request Mar 24, 2018
Only make inferences from tuple types of right arity #22850
Merged
@mhegazy Mohamed Hegazy (mhegazy) mentioned this pull request Apr 23, 2018
Feature Request: strictier array and string using parenthesis modifiers #23617
Closed
@aboyton Andrew Boyton (aboyton) mentioned this pull request May 23, 2018
Assume arity of tuples when declared as literal #24350
Open
3 of 4 tasks
@microsoft Microsoft (microsoft) locked and limited conversation to collaborators Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@sandersn Nathan Shively-Sanders (sandersn) sandersn approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Proposal: strict and open-length tuple types

Uh oh!

There was an error while loading. Please reload this page.

13 participants

@KiaraGrouwstra @aluanhaddad @zpdDG4gta8XKpMCd @dead-claudia @ikatyang @gcnew @sandersn @mhegazy @mightyiam @Igorbek @michaeljota @msftclas @msftgits
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.