Repository navigation
Conversation
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The change is narrowly scoped to a TS-only type assertion in an unreachable arm and is backed by a dedicated new snapshot fixture.
Review effort: Lite
Findings: None
What changed in this PR
This PR improves the TypeScript-native bindings generator so that conversions for single-tag variants are type-safe under TypeScript’s control-flow narrowing rules, eliminating an unreachable fallthrough that previously retained the input type.
Changes:
- Update the TS-native variant-to-candid conversion generator to emit
value as neveras the final fallthrough when the variant has exactly one tag. - Add a focused
single_tag_varianttest asset plus generated snapshots to lock in the expected output. - Register the new fixture in the generator snapshot test suite.
| File | Description |
|---|---|
| tests/snapshots/generate/single_tag_variant/single_tag_variant.ts.snapshot | New snapshot covering the generated TS wrapper and conversions for a single-tag variant case. |
| tests/snapshots/generate/single_tag_variant/declarations/single_tag_variant.did.js.snapshot | New snapshot for generated candid JS declarations for the fixture. |
| tests/snapshots/generate/single_tag_variant/declarations/single_tag_variant.did.d.ts.snapshot | New snapshot for generated TS type declarations for the fixture. |
| tests/generate.test.ts | Includes single_tag_variant in the list of snapshot fixtures to run. |
| tests/assets/single_tag_variant.did | Adds the new candid fixture exercising the single-tag variant behavior. |
| src/core/generate/rs/src/bindings/typescript_native/conversion_functions_generator.rs | Implements the as never fallthrough for single-tag __kind__-checked variant conversions. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
808e0ad to
7201f90
Compare
|
Force-pushed as part of a stack-wide rebase. This PR's own diff is unchanged — nothing to re-review here. Only #175, #187 and #188 changed content in this round: a nested option reached through a type alias took the single-option path, so Comparing this PR's commit patch before and after the rebase, every added and removed line is identical. The only difference is one line of surrounding context in the fixture list — |
…ersion
A variant conversion tests each tag in turn and returns the input from the arm that no
tag matched. With two or more tags TypeScript has narrowed the input to `never` by
then, so the arm typechecks against any return type. A variant with one tag is not a
union, so nothing narrows: the arm keeps the input type, and the conversion of
`variant { p : opt nat }` fails to typecheck where the header is stripped, on the arm a
well-formed value never reaches.
The unreachable arm already asserts `never` where narrowing cannot reach it — a tag
named after an inherited member — and now does so for a single-tag variant as well.
The arm is erased at runtime and was unreachable before, so nothing on the wire moves.
7201f90 to
17916d4
Compare
|
Dropped: we're fixing only what realistic interfaces hit silently. This one either fails loudly with a rename as the workaround, or needs names that don't occur in practice. The error only shows with |
Stacked on #188.
Behaviour
variant { p : opt nat }as an argumentvalue.__kind__ === "p" ? { p: … } : value— the fallthrough returns the input where the candid shape is expected, TS2322 under the stripped header… : value as nevervariant { p : opt nat; z }neverFour real interfaces carry a single-tag variant in argument position and typecheck clean now: the LLM canister, SNS-W and the ckETH minter among them. No existing snapshot moves.
Why
The fallthrough arm relies on TypeScript narrowing the discriminated union to
neveronce every tag has been tested. That narrowing only happens on a union; a variant with one tag lowers to a single object type, so the arm keeps the input type.unreachable_valuealready assertsneverin the one other case where narrowing cannot reach it — a tag named after an inherited member, where the own-property conjunction breaks it — and a single-tag variant is the same situation for a different reason.