Support runtime union types via dynamic-union enums#4734
Conversation
3f4ca60 to
383f625
Compare
383f625 to
7e992cf
Compare
|
Just to reflect some private concerns here: this seems like an extension of new syntax sugar that I know @daxpedda wanted to try and avoid in the core, so I'm going to defer to him for review. E.g. once we land this, what if users might want all other Serde enum representations built into wasm-bindgen as well? It seems we might want to answer a question of where we draw a line between built-in serialization and things covered by serde-wasm-bindgen integration before adding features from said subset. There are definitely valid reasons to add some integrations to the core, at least for optimisations alone as serde-wasm-bindgen doesn't have as much access to type information at compile-time as wasm-bindgen itself does, but we need to make sure we don't paint ourselves in the corner and on the hook for maintenance of even larger API area with all the semver difficulties that wasm-bindgen already comes with. Maybe there are hooks we could expose instead to make tools like serde-wasm-bindgen more efficient or support more types, so that the core remains lean? I don't know the answer, but something worth exploring. Offtop: what you're adding here is rather the untagged enums not tagged ones. |
7e992cf to
c0c6a7a
Compare
c0c6a7a to
4845e0c
Compare
There was a problem hiding this comment.
See #2088 and #2407 for previous discussions.
I definitely have the concerns pointed out by RReverser, these are also laid out more in #2088.
However, I don't think its realistic that we are getting any sort of better output customization in wasm-bindgen in this version, because its just going be too difficult to design. Maybe we can live with an ad-hoc implementation just for this version, but I'm apprehensive about it.
@guybedford did you ever take a look at tsify? Again, I don't think wasm-bindgen is offering something here, in the end its all a JsValue and we aren't providing an optimized conversion. The TS part can be handled with typescript_type.
I think it would be good to discuss this in the upcoming meeting and how we want to proceed in general.
f0adc28 to
771867a
Compare
17c5a0a to
b114197
Compare
CodSpeed Performance ReportMerging #4734 will not alter performanceComparing Summary
|
b114197 to
0f9ca45
Compare
f0a1e6b to
2995e17
Compare
|
@RReverser @daxpedda I've updated this PR to support TypeScript generation and deeper integration into the type system, as well as renaming to |
daxpedda
left a comment
There was a problem hiding this comment.
I believe my review from last time still stands. As long as you are doing dynamic conversion, ergo find the right type in Wasm, this whole implementation can be done externally.
The part that can't be done externally is exporting, but I believe this isn't part of what you need?
(however, my conclusion below is that we should instead merge it, so I'm not advocating for doing this externally, just pointing it out)
So about the design space:
- Regardless of what kind of enum we have/introduce, the user should be able to control what to import and what to export. In practice this means being in control if JS/TS is generated for an enum.
- Right now we only support string and integer enums. In Rust they are represented as data-free enums (only unit variants).
- The second type of enum we want to support is one where variants are actual JS object with fields. In Rust these would be represented as enum with struct or tuple variants.
- Your implementation does something a bit different: it uses imported or exported types in tuple variants instead of inlining the properties. But these two are not in conflict, so we can proceed with this design.
In conclusion: I think your design works just fine and I do not believe that it will create conflicts in the future. I will defer to you on how the JS/TS output looks like. My concerns about import vs export still stand but the defaults are already messed up and the added design doesn't make it better or worse, but is instead consistent with whats already in place.
Just approving design, not code!
|
Also for context on where we stood on this in the past: I was generally hoping that we could avoid having everything in |
a600f54 to
1e3e38a
Compare
This adds support for dynamic unions: a
#[wasm_bindgen]enum that mixesstring-literal variants with single-field tuple variants is exported as an
untagged TypeScript union and dispatched dynamically at the JS↔Rust boundary.
The motivation is to express common JavaScript shapes —
"loading" | string | SomeClass | OtherClass— as a single Rust enum that can bematch-edexhaustively. Discrimination is dynamic: each variant is tested in
declaration order via
as_stringfor string literals andTryFromJsValuefor typed payloads.
produces the TypeScript
Resolves #2088, resolves #2153.
What's implemented
DYNAMIC_UNIONand per-variant descriptor exports(
__wbg_dynamic_union_<name>_<idx>) so cli-support can resolve variantTypeScript names through the normal descriptor pipeline.
ast::DynamicUnionparser path triggered by any tuple-style variant on a#[wasm_bindgen]enum. The previous error "enum variants with associateddata are not supported" is removed; mixed unit + tuple variants are now
parsed as a dynamic union.
IntoWasmAbi/FromWasmAbi/TryFromJsValue/OptionIntoWasmAbi/OptionFromWasmAbi/From<T> for JsValueimpls,plus
WasmDescribecarrying the variant type list.as_stringcheckwith an inner
match, so worst-case dispatch is one string readregardless of how many literal variants exist. Literal variants always
win against a generic
Stringarm regardless of declaration order.cli-supportplumbing:Descriptor::DynamicUnion,AdapterType::DynamicUnion, incoming/outgoing externref lowering, andoption-position handling so
Option<DynamicUnion>works.AuxDynamicUnionVariant::{Literal, Type}defersvariant TS rendering to JS-generation time, where renamed types resolve
through
qualified_to_identifier. A reference closure(
expand_typescript_refs) walks transitively-referenced types so nestedunions and string enums used as variants emit their full type aliases.
export type(was baretype) so the alias is a named module export.privateflag honoured by string enums and dynamic unions to suppressthe
exportkeyword.#[wasm_bindgen(fallback)]The new enum-level
fallbackattribute makes the last tuple variant anunconditional catch-all. This is the supported pattern for unions whose
trailing variant has no meaningful runtime check (e.g., interface-only
imports — types declared via
extern \"C\" { type Foo; }withtypescript_typebut no JS-side runtime construct, whoseinstanceofcheck always returns
false).renders as
\"loading\" | \"empty\" | Shape. Strings\"loading\"and\"empty\"route to their named variants; anything else is unconditionallyaccepted as
Anything(_).Nesting and
OptionA variant payload may itself be another dynamic union or a string enum,
and the union may appear inside
Option:generates:
Option<DynamicUnion>works in both argument and return positions andrenders as the standard
T | null | undefined/T | undefinedpair.C-style enum
TryFromJsValuefixThis PR fixes a separate bug uncovered by the new tests: c-style enum
TryFromJsValuewas applying JS unary+coercion viaf64::try_from,silently coercing strings, booleans, null, and arrays into numbers. As a
result
dyn_into::<MyEnum>()on a string would produce a valid enumvalue (typically the variant with discriminant
0, sinceNaN as u32 = 0),violating the
TryFromJsValuecontract.The conversion now uses
value.as_f64(), which only succeeds on actualJS numbers. This affects two surfaces:
dyn_into::<MyEnum>()for c-style enums correctly returnsErrfornon-numeric input.
Vec<MyEnum>from JS rejects elements that aren't numbers (theexisting
tests/wasm/enum_vecs.rs::test_invalidexpected this but waspreviously a no-op because the bypass coerced through).
Round-trip benchmark
benches/enum_roundtrip.rsmeasures string-enum vs dynamic-union round-tripcost through a JS shim. On Node 25 in release mode:
Within ~1%. The runtime cost difference between the two encodings is
effectively noise — a developer-facing comment on
ToTokens for ast::StringEnumnotes this and outlines the path to subsume string enums into dynamic
unions in a future PR.
Test coverage
tests/wasm/enums.rs— round-trips now go through JS shims declared intests/wasm/enums.jsto actually exercise the dispatcher (previouslythese called exported Rust functions Rust-to-Rust, bypassing
into_abi/from_abientirely). Tests cover string-literal variants,primitive payloads, exported and imported types, nesting,
Option, andthe new
fallbackattribute.crates/cli/tests/reference/dynamic-union.{rs,d.ts,bg.js,js,wat}—full reference fixture covering literals, fallback, exported struct,
imported type (annotated with
typescript_type), nesting,Option<Wrapper>, and aprivatevariant.crates/typescript-tests/src/dynamic_union.{rs,ts}— TS-side jesttests asserting the generated
.d.tsexposes union types as namedaliases, that variants typecheck both as literals and as object payloads,
that nested literals dispatch through the outer→inner chain, and that
Option<Wrapped>typechecks asT | null | undefined.crates/macro/ui-tests/invalid-enums.{rs,stderr}— new error coveragefor variants with named fields, variants with multiple unnamed fields,
and unit variants without a string discriminant in a tuple-bearing enum.
tests/wasm/enum_vecs.js— updated to assert the correct errormessage ("array contains a value of the wrong type") that the strict
TryFromJsValuenow produces.Naming
The feature is named dynamic union throughout — "dynamic" describes the
discrimination mechanism (runtime inspection per variant), distinguishing it
from compile-time TS unions and from Rust's own tagged enums. The original
draft used "discriminated union" which is technically the opposite of
what this is (these are untagged at the JS boundary).
Schema
Schema hash bumped to
9718506702724923631.