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

Editorial: Update use of WebIDL "invoke a callback function" #102

Merged
merged 2 commits into from
Aug 12, 2024

Conversation

jeremyroman
Copy link
Contributor

@jeremyroman jeremyroman commented Aug 9, 2024

@jeremyroman
Copy link
Contributor Author

@shaseley ptal?

In principle I think the callback type could be changed to return Promise<any>, because that causes WebIDL to automatically wrap non-promises and thrown exceptions, in which case the explicit handling here could be omitted entirely.

But that is fairly minor and might be annoying to propagate to browser vendors, could in theory have slight visible effects because the [[Resolve]] steps would run twice, etc., so I've used "rethrow" instead and continued to explicitly catch the exception here.

/cc @domenic

@shaseley
Copy link
Collaborator

shaseley commented Aug 9, 2024

LGTM, thanks. You might need to rebase to get past the build error. There was a stale reference that somehow wasn't previously caught, which I've fixed on main. But rerunning all jobs on this didn't fix it.

@jeremyroman
Copy link
Contributor Author

Rebased; I don't have permissions to either re-run the workflow or merge.

@shaseley shaseley merged commit 714aeb3 into WICG:main Aug 12, 2024
2 checks passed
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.

2 participants