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

Add async iterable flavors #1403

Merged
merged 8 commits into from
Sep 21, 2022

Conversation

MattiasBuelens
Copy link
Contributor

@MattiasBuelens MattiasBuelens commented Sep 18, 2022

Add support for async iterable declarations per Web IDL, and emit them as .asynciterable.d.ts flavors. This is a first step towards microsoft/TypeScript#29867.

At the time of writing, only FileSystemDirectoryHandle's async iterator is supported by two browser engines (Chrome and Safari). ReadableStream's async iterator is not implemented anywhere yet, and gets correctly filtered out by getRemovalData().

Fixes part of #1393.

@github-actions
Copy link
Contributor

Thanks for the PR!

This section of the codebase is owned by @saschanaz - if they write a comment saying "LGTM" then it will be merged.

Copy link
Contributor

@saschanaz saschanaz left a comment

Choose a reason for hiding this comment

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

That was quick, very cool! Thank you very much. Some nits:

src/build/bcd/mapper.ts Show resolved Hide resolved
"iterator": {
"type": {
"0": {
"overrideType": "R"
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems redundant here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough. I'll need to remember to add it back once browsers actually add support for this async iterator, and it actually ends up in the generated types. 😛

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmmmm. That said, this is the only needed manual data and we know it will eventually come, so maybe we can keep this 👀

const asyncIterators = emitWebIdl(exposed, options.global[0], "async");
await fs.writeFile(
new URL(
`${options.name}.asynciterable.generated.d.ts`,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not a fan of having extra files, but I guess we need it 🥲

src/build/emitter.ts Outdated Show resolved Hide resolved
@saschanaz
Copy link
Contributor

Thanks! LGTM.

@github-actions github-actions bot merged commit 5712b44 into microsoft:main Sep 21, 2022
@github-actions
Copy link
Contributor

Merging because @saschanaz is a code-owner of all the changes - thanks!

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.

None yet

2 participants