microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.19k stars 12.38k forks source link

Array of tuple literal not assignable to iterable of tuple with --lib es2015 -t es5 #32761

Open laughinghan opened 5 years ago

laughinghan commented 5 years ago

TypeScript Versions: 3.5.1, 3.5.3 (currently typescript@latest), 3.6.0-beta (currently typescript@beta), 3.6.0-dev.20190808 (currently typescript@next)

Search Terms:

Code

// Works:
declare function foo<T>(i: Iterable<T>): T
const x = foo([1]); // infers a: number

// Fails:
declare function bar<T, U>(j: Iterable<[T, U]>): [T, U]
const y = bar([[1, 'a']]);
//             ^^^^^^^^
//             Argument of type '(string | number)[][]' is not assignable to
//             parameter of type 'Iterable<[string | number, string | number]>'.

// workaround:
declare function qux<T, U>(j: Array<[T, U]> | Iterable<[T, U]>): [T, U]
const z = qux([[1, 'a']]); // infers z: [number, string]

Expected behavior: variable y is inferred to be of type [number, string] and parameter j is inferred to be of type Array<[number, string]>

Actual behavior: error TS2345: Argument of type '(string | number)[][]' is not assignable to parameter of type 'Iterable<[string | number, string | number]>'.

Playground Link

Related Issues: maybe #29311 ?

dragomirtitian commented 5 years ago

Cloned repo, ran tsc .\hello.ts -target es2015, no error.

Are you sure you don't have another tsc installed on your system? Run a tsc- -v

laughinghan commented 5 years ago

Are you able to reproduce the error with tsc --lib esnext hello.ts, as suggested in the README?

I am finding that -target es2015 makes the error go away. Thanks, that explains the difference from Playground; if -target is made to match the Playground "Target" option, then the behavior of tsc matches Playground exactly.

However, I'm targeting ES5 (and polyfilling ES2015 features). And the library I'm using, Facebook's ImmutableJS, presumably has to target a range of ES versions, so they want to reference Iterable in their type declaration: https://github.com/immutable-js/immutable-js/blob/master/type-definitions/Immutable.d.ts#L1420

Is this expected behavior with --lib?

RyanCavanaugh commented 5 years ago

If target is ES5 and you're polyfilling Symbol.iterator, then you should turn on downlevelIteration to cause arrays to appear to have the iterator symbol

laughinghan commented 5 years ago

But that makes for...of loops much slower at runtime? This is a typings issue, I'm not going to sacrifice runtime performance to fix it.

It looks like adding Array<[K, V]> as an additional type overload besides Iterable<[K, V]> fixes the issue. I'll submit a PR to ImmutableJS. Thanks for all your help and the ultra-fast response time!

laughinghan commented 5 years ago

@RyanCavanaugh Actually, I'm confused why --downlevelIteration matters. lib.es2015.iterable.d.ts declares a Symbol.iterator on Array: https://github.com/microsoft/TypeScript/blob/master/src/lib/es2015.iterable.d.ts#L40

So why isn't --lib es2015 sufficient to cause arrays to appear to have the iterator symbol?

The docs for --downlevelIteration only mention for..of, spread and destructuring, they don't mention anything about the iterator symbol either (Compiler Options).

laughinghan commented 5 years ago

Actually no, this seems like an issue with type inference.

In most cases (not for..of, obviously), --lib es2015 causes Array<T> to be assignable to Iterable<T>, but not if a tuple type is involved; in some cases, the error message doesn't even mention incompatibility between Array and Iterable, it says:

Type '(string | number)[]' is not assignable to type '[string | number, string | number]'.

In this Playground Link, the first 4 examples here all show that Array<T> is normally assignable to Iterable<T>, the next 4 demonstrate the issue when a tuple type is involved.

Both --downlevelIteration and using Array<[T, U]> as an alternative to Iterable<[T, U]> do workaround the issue, though.

laughinghan commented 5 years ago

@RyanCavanaugh just updated title, code sample, and Playground link to succinctly illustrate that downlevelIteration should be unnecessary, since array literals are already assignable to iterables if the contents aren't tuples.

typescript-bot commented 5 years ago

This issue has been marked as 'Question' and has seen no recent activity. It has been automatically closed for house-keeping purposes. If you're still waiting on a response, questions are usually better suited to stackoverflow.

laughinghan commented 5 years ago

@RyanCavanaugh Can you unmark this as a Question? I think the updated code sample succinctly illustrates that it's a bug

RyanCavanaugh commented 5 years ago

Looking

RyanCavanaugh commented 5 years ago

@rbuckton can you explain what's happening? Bug?