Skip to content

Editorial: Align with Web IDL specification #578

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

autokagami
Copy link

@autokagami autokagami commented Jul 24, 2025

🤖 This is an automated pull request to align the spec with the latest Web IDL specification. 🤖

The followings are the Web IDL validation messages, which may help understanding this PR:

  • Validation error at line 7 in wot-scripting-api,12:
            async iterable<ThingDescription>
            ^
    

    Error: async iterable is now changed to async_iterable.

Currently this autofix might introduce awkward code formatting, and feel free to manually fix it whenever it happens.

Please file an issue at https://github.com/saschanaz/webidl-updater/issues/new if you think this is invalid or should be enhanced.


Preview | Diff

@danielpeintner
Copy link
Contributor

I don't think the proposed change is correct/good, even though I found https://bugzilla.mozilla.org/show_bug.cgi?id=1978444

@saschanaz
Copy link
Member

Can you elaborate why you think it's incorrect?

@danielpeintner
Copy link
Contributor

I am not a WebIDL expert at all. So please excuse my ignorance...
I just think personally that it is not a good idea to combine variable/method identifiers into one...

async (at least for me) is a term on its own and should not be merged with iterator 🤷‍♂️

IF that's the new way in WebIDL we will do it, but I am definitely not a big fan of doing so 😏

@saschanaz
Copy link
Member

saschanaz commented Jul 25, 2025

Yeah, the reason we have done this way is to disambiguate what's expected after async in parser's perspective. (Previously we had only async iterable but as we are getting more asyncs we learned that the current grammar is not ideal.)

See also whatwg/webidl#1500 and whatwg/webidl#1504 for the discussion.

@JKRhb
Copy link
Member

JKRhb commented Jul 27, 2025

Thank you, @saschanaz, for the creation of this PR and for elaborating on the changes! As this PR takes over the latest state of the Web IDL spec, I think this PR should be fine to merge (although the prose probably also needs to be updated afterward).

Contemplating the change to the Web IDL spec a bit more, I think that is this making things actually more consistent and probably also easier to read, so thanks for the improvement here.

As a minor detail, I was wondering, though, whether the snake_casing is consistent with the rest of the Web IDL spec? Or was it a deliberate choice to deviate here?

@saschanaz
Copy link
Member

Admittedly this is the first occurrence of snake case, but I think that's the best choice as pascal case wouldn't make sense here (that's for types).

But async_sequence could be AsyncSequence... I filed whatwg/webidl#1510 for that. Thanks for your input!

@danielpeintner
Copy link
Contributor

BTW, in TypeScript we use or TS has AsyncIterable

export interface ThingDiscoveryProcess extends AsyncIterable<ThingDescription> {

@relu91
Copy link
Member

relu91 commented Jul 29, 2025

I don't have a strong opinion about how WebIDL should evolve (I just like the TS way because I'm more familiar with it). What I'm concerned about is how much this new choice is stable? Can we merge this PR without expecting any major refactoring soon? (e.g. whatwg/webidl#1510 is going to be fixed in this "release"? Or next? )

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.

5 participants