Skip to content

Conversation

MattiasBuelens
Copy link
Contributor

@MattiasBuelens MattiasBuelens commented Sep 25, 2025

@MattiasBuelens
Copy link
Contributor Author

@domenic Could you assign some reviewer(s) to this PR? I can't seem to do it myself.

@domenic
Copy link
Member

domenic commented Sep 27, 2025

I'll still work on jsdom stuff, don't worry. I plan to review this tomorrow :)

@domenic
Copy link
Member

domenic commented Sep 28, 2025

I appreciate the separate commits; it made reviewing this easier. If you're up for some rebase-fu, here are some minor notes:

  • "Rename operations" -> "Rename test operations for clarity"
  • Merged "Fix object type handling" and "Test object inside a union", and expand on the fix (maybe with reference to Avoid generating broken code when the object type is in a union #231).
  • Merge "Add AllowSharedBufferSource" into "Support [AllowShared] on all ArrayBufferView types"
  • Edit the commit message for "Generate conversions for ArrayBufferView types" to explain why we're doing this now when we didn't before.

Or, if you prefer, we can just do a big squash-and-merge with a sufficiently-detailed commit message.

This also fixes the crash reported in jsdom#231.
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

Awesome stuff, thank you!

The repeated conversions for unions with primitives are a bit strange, but I see the follow the spec exactly. (I.e. https://webidl.spec.whatwg.org/#js-union steps 13.1 + 17.)

Probably some optimizations are possible for simple unions. But I think the right tactic there would be to work on the more general project of slimming down the generated code, driven by analyzing all of jsdom to see what are the worst offenders weighted by web platform prevalence.

@domenic domenic merged commit 170f8a3 into jsdom:main Sep 29, 2025
3 checks passed
@MattiasBuelens MattiasBuelens deleted the allow-resizable-shared branch September 29, 2025 04:38
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