Skip to content
This repository has been archived by the owner on May 8, 2024. It is now read-only.

Tests should reject unqualified use of undefined interfaces #34

Open
BurtHarris opened this issue Sep 1, 2016 · 4 comments
Open

Tests should reject unqualified use of undefined interfaces #34

BurtHarris opened this issue Sep 1, 2016 · 4 comments

Comments

@BurtHarris
Copy link

The test tool seems to allow definitions to use undefined and unqualified interfaces like IterableIterater<T>, Symbol, and even ReadableStream.

Should it?! This seems to lead to issues like DefinitelyTyped/DefinitelyTyped#10919.

@mhegazy mhegazy added the bug label Sep 22, 2016
@mhegazy mhegazy assigned ghost Sep 22, 2016
@mhegazy mhegazy removed the bug label Sep 22, 2016
@mhegazy mhegazy unassigned ghost Sep 22, 2016
@mhegazy
Copy link

mhegazy commented Sep 22, 2016

totally misread it. before we do that we need to make sure all libraries can abide by it. not sure this would be the case, at least for some of the polyfills like core-js that really need to define Symbol.

@BurtHarris
Copy link
Author

BurtHarris commented Sep 22, 2016

As I remember the problems revolved around dependency on these symbols, not definition of them. That said, how polyfills should work for TypeScript is still very unclear to me, but I'm not saying that the .d.ts for a polyfill shouldn't be able to define Symbol, rather that unqualified (and undefined) references to new types like Symbol (and the like) seem like to leave the question of how/when to introduce the polyfill definition unanswered.

Perhaps a .d.ts file should declare what minimum version of ECMAScript (or polyfills) it requires.

@mhegazy
Copy link

mhegazy commented Sep 26, 2016

As I remember the problems revolved around dependency on these symbols, not definition of them.

Can you elaborate?

@BurtHarris
Copy link
Author

BurtHarris commented Sep 26, 2016

Sure @mhegazy, I'll try again. Lets take issue DefinitelyTyped/DefinitelyTyped#10919 for example. It appears that a change to the node package's definition that introduced use of IterableIterator. This commit passed the integration tests, presumably because they were processing it using ECMAScript 2015 mode. But this change causes surprising errors for typescript users still emitting ES5 code because IterableIterator is not declared anywhere they can see.

I'm suggesting that perhaps something should be done to prevent the hard-to-interpret error that results when an existing definitions file is upgraded to take a new dependency on ES2015. I.e. maybe there should be some qualifier in the package that explicitly says if it requires ES2015, (or a polyfill.)

But no matter how that's resolved, I wasn't proposing generating errors for a polyfill(s) that define IterableIterator. On the contrary, I'd like to see some sort of conditional import feature that includes the polyfill only if we are not emitting ES2015. Does that make sense? Please feel free to retitle the issue if that's called for...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants