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

Fix incorrect tuple usage and syntax #21387

Merged
merged 4 commits into from
Nov 9, 2017
Merged

Fix incorrect tuple usage and syntax #21387

merged 4 commits into from
Nov 9, 2017

Conversation

sandersn
Copy link
Contributor

@sandersn sandersn commented Nov 9, 2017

Lots of packages use singleton tuple syntax in place of array syntax. [number] was previously almost compatible with number[], so it slipped through in a lot of places. Now, with fixed-length tuples, this reliably fails.

The fixed-length tuple also found a couple of errors, in highchart and leaflet. I changed the highchart test to be correctly-typed, although the intent of the test is probably a bit changed. I am 99% sure the test is wrong, though, based on the highchart documentation.

The leaflet test assumes that LatLng marks whether altitude was provided in its type, which it does not, so converting a LatLng returns [number, number] | [number, number, number], which was previously equivalent to [number, number].

It's relatively simple to add a type parameter LatLng<T extends number | undefined>, but it's a big change that's outside the scope of this set of simple fixes.

@dt-bot
Copy link
Member

dt-bot commented Nov 9, 2017

types/aws-lambda/index.d.ts

to authors (@darbio/aws-lambda-typescript @skarum @StefH/DefinitelyTyped @tobyhede @buggy @y13i @wwwy3y3 @OrthoDex @daniel-cottone). Could you review this PR?
👍 or 👎?


types/fingerprintjs2/index.d.ts

to author (@curtstate). Could you review this PR?
👍 or 👎?


types/heredatalens/index.d.ts

to author (@denyo). Could you review this PR?
👍 or 👎?


types/highcharts/index.d.ts

to authors (@damianog @baltie @AlbertOzimek @hanssens). Could you review this PR?
👍 or 👎?


types/validate.js/index.d.ts

to author (@HillTravis). Could you review this PR?
👍 or 👎?


types/xml2js/index.d.ts

to authors (@michelsalib @jasonrm @ccurrens @EdwardHinkle). Could you review this PR?
👍 or 👎?

@sandersn sandersn requested a review from a user November 9, 2017 22:10
@ghost
Copy link

ghost commented Nov 9, 2017

Rebase against master to fix test failures. (#21384)

@sandersn sandersn merged commit e043e9b into master Nov 9, 2017
@sandersn sandersn deleted the fix-incorrect-tuples branch November 9, 2017 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants