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

Allow specifying TReturn and TNext for AsyncIterableIterator and AsyncIterable #36723

Closed
wants to merge 2 commits into from

Conversation

bterlson
Copy link
Member

@bterlson bterlson commented Feb 10, 2020

Partially fixes #33932 by giving a way to provide a better type than any for TReturn. Implementation is simply adding and plumbing the TReturn and TNext parameters for both AsyncIterableIterator and AsyncIterable, and updating baselines.

@DanielRosenwasser
Copy link
Member

@typescript-bot test this
@typescript-bot user test this
@typescript-bot run dt

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 13, 2020

Heya @DanielRosenwasser, I've started to run the extended test suite on this PR at 3b69bf2. You can monitor the build here. It should now contribute to this PR's status checks.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 13, 2020

Heya @DanielRosenwasser, I've started to run the parallelized Definitely Typed test suite on this PR at 3b69bf2. You can monitor the build here. It should now contribute to this PR's status checks.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 13, 2020

Heya @DanielRosenwasser, I've started to run the parallelized community code test suite on this PR at 3b69bf2. You can monitor the build here. It should now contribute to this PR's status checks.

@typescript-bot
Copy link
Collaborator

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

@bterlson
Copy link
Member Author

@typescript-bot test this
@typescript-bot user test this
@typescript-bot run dt

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 13, 2020

Heya @bterlson, I've started to run the parallelized Definitely Typed test suite on this PR at aa0f332. You can monitor the build here. It should now contribute to this PR's status checks.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 13, 2020

Heya @bterlson, I've started to run the parallelized community code test suite on this PR at aa0f332. You can monitor the build here. It should now contribute to this PR's status checks.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 13, 2020

Heya @bterlson, I've started to run the extended test suite on this PR at aa0f332. You can monitor the build here. It should now contribute to this PR's status checks.

@sandersn sandersn added the For Milestone Bug PRs that fix a bug with a specific milestone label Feb 13, 2020
@ExE-Boss
Copy link
Contributor

ExE-Boss commented Feb 15, 2020

What about Iterable and IterableIterator?

Also, do note that this was originally rejected in #30790 (comment) by @rbuckton.

@bterlson
Copy link
Member Author

@ExE-Boss I would argue for similar treatment, but given @rbuckton's trepidation I think we should wait and see what he thinks. If adding Iterable and IterableIterator to this PR would help, I will do it!

@sandersn sandersn removed their assignment Mar 12, 2020
@rbuckton
Copy link
Member

rbuckton commented Jul 30, 2020

At one point in #30790 I was using unknown instead of any. The problem with undefined as a default is that not every Iterator can be assumed to return undefined, and making the type anything other than any or unknown caused a significant number of breaks in assignability where people were rolling their own custom iterators.

We also had issues with the fact that we were conflating T in Iterator and Iterable to include both the yielded and the returned type, and the fact that Iterator<T> was originally typed with return?(value?: any):

image

I am considering my concerns about adding TReturn and TNext to Iterable. My main concern is that it would dramatically increase .d.ts file output if suddenly every inferred Iterable<number> in the output became Iterable<number, void, undefined> because we fill in the default types at the time of compilation.

@sandersn
Copy link
Member

Unfortunately, we never finished reviewing this PR. It is pretty old now, so I'm going to close it to reduce the number of open PRs.

@sandersn sandersn closed this May 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Milestone Bug PRs that fix a bug with a specific milestone
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Iterator result of Iterable or AsyncIterable inferred to any
6 participants