Collapse Record<K, Union> per-arm explosion to a single binding#13
Merged
Conversation
`Record<K, U>` where `U` is a union (flattens to >1 alternative) now emits one binding carrying the original union, instead of one binding per arm. The arms each lower to a distinct `Object<JsString>` / `Object<Number>` / `Object<Boolean>` whose phantom is a compile-time tag the runtime never sees, so the cartesian-expanded variants were pure API-surface noise — every caller had to pick one of N siblings backed by the same JS object. Folding the union back at `flatten_type` time leaves the existing JsValue-erasure path in `to_syn_type` to lower `Record<K, Union>` to plain `&Object`. Single-typed Records (`Record<string, string>`) still emit `&Object<JsString>` — the typed phantom is meaningful when the value side is monomorphic. Snapshot impact: workers-types' `WorkerLoaderWorkerCode.modules` (2 arms → 1), `GatewayOptions.metadata` (5 → 1), `VectorizeVector.metadata` (7 → 1) collapse to single bindings. Auto-gen suffix logic also picks the param-name-derived suffix (`_with_context`) instead of the type-derived one (`_with_record`), since there's no longer ambiguity to disambiguate against.
guybedford
reviewed
Apr 29, 2026
Replace the Record-only carve-out with a uniform LUB-based collapse applied at every `flatten_type` recursion into a generic container's element type. Reuses the existing LUB infrastructure (`subtyping::lub_types`) that already drives `TypeRef::Union` lowering and @throws aggregation, so there's one rule for "what does a union of types mean at the binding layer" rather than two. The `Record`-only `vs.len() > 1` special case is replaced by a `lub_collapse` helper applied uniformly to `Array`, `Set`, `Promise`, `Record`, and `Map` element types. Implementation: * `typemap.rs`: rename private `union_lub` → `pub(crate) lub_named`, the existing helper that already implemented exactly this LUB resolution. Doc-comment updated to describe both call sites. * `signatures.rs`: new `lub_collapse` 4-line wrapper that calls `lub_named` and falls back to the original unflattened TypeRef when no meaningful LUB exists. Each generic-container arm of `flatten_type` calls it on the inner alternatives. Snapshot impact (workers-types.rs only): 352 lines of phantom-sibling explosions removed. `Record<K, Union>` collapses (already in wasm-bindgen#13) preserved; new collapses across `Array<Union>`, `Promise<Union>`, `Set<Union>` apply the same rule. Where the union members share a named ancestor (e.g. `TypeError | RangeError` → `Error`), the phantom narrows to that ancestor instead of erasing. CONVENTIONS: * Drop the Record-only carve-out subsection. * Extend `## Subtyping LUB across unions` to mention that the same rule runs inside generic containers at flatten-time. Tests: 8 new unit tests in `codegen::signatures::tests` covering `Record`, `Array`, `Map`, `Promise` collapse plus direct `lub_collapse` happy path / fallback / pass-through. 127 unit + 1 snapshot pass, clippy + fmt clean.
Contributor
|
I'm still not sure how I feel about this - this might be a tipping argument for wasm-bindgen/wasm-bindgen#4734 actually as well... |
Contributor
|
I'll actually land this for now as it's cleaner, then we can always iterate on this further later on. |
guybedford
approved these changes
Apr 30, 2026
Contributor
|
Basically pending union wrappers, this is the starting point and custom bindgen can be added until we have a solution for upcastable union types such as along the direction of wasm-bindgen/wasm-bindgen#4734. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Record<K, U>whereUis a union (flattens to >1 alternative) now emits one binding carrying the original union, instead of one binding per arm. The arms each lower to a distinctObject<JsString>/Object<Number>/Object<Boolean>whose phantom is a compile-time tag the runtime never sees, so the cartesian-expanded variants were pure API-surface noise — every caller had to pick one of N siblings backed by the same JS object.Folding the union back at
flatten_typetime leaves the existing JsValue-erasure path into_syn_typeto lowerRecord<K, Union>to plain&Object. Single-typed Records (Record<string, string>) still emit&Object<JsString>— the typed phantom is meaningful when the value side is monomorphic.Snapshot impact: workers-types'
WorkerLoaderWorkerCode.modules(2 arms → 1),GatewayOptions.metadata(5 → 1),VectorizeVector.metadata(7 → 1) collapse to single bindings. Auto-gen suffix logic also picks the param-name-derived suffix (_with_context) instead of the type-derived one (_with_record), since there's no longer ambiguity to disambiguate against.