Skip to content

Unify enum variants into a single enumVariantTypeNode - #138

Merged
lorisleiva merged 2 commits into
mainfrom
single-enum-variant
Sep 1, 2026
Merged

Unify enum variants into a single enumVariantTypeNode#138
lorisleiva merged 2 commits into
mainfrom
single-enum-variant

Conversation

@lorisleiva

@lorisleiva lorisleiva commented Sep 1, 2026

Copy link
Copy Markdown
Member

This PR implements #127: the three enum variant flavours collapse into one enumVariantTypeNode whose payload is an optional data: TypeNode — absent for unit variants, a struct for named fields, a tuple for positional fields, and any other type node carried as-is (a variant holding a single type needs no tuple around it).

As scoped in the #124 resolution, the node gains an optional docs attribute matching structFieldTypeNode, so enum variants (doc-comment carriers in Rust) are documentable. Attribute order follows the convention: name, discriminator, docs, then the data/display children.

The enumVariantTypeNode union is deleted — the name now identifies the node — with enumTypeNode.variants and registeredTypeNode referencing the node directly. Examples rewritten; counts move to 93 nodes / 33 unions.

Upgrade lens note (for upgradeV1ToV2): mechanical — empty variant → no data; struct variant structdata; tuple variant tupledata, except that a tuple holding exactly one item unwraps to that single type, unless the item is a structTypeNode, tupleTypeNode or definedTypeLinkNode — those shapes determine the variant flavour in data position, so unwrapping would change the generated API (links are kept wrapped so the lens never needs resolution to be correct). The 1-tuple was the v1 workaround spelling of a single-type payload, and this unification is the proper solution. Renderers whose target requires a wrapper (e.g. the Rust renderer emitting a newtype-style variant) simply re-wrap on their side. name/discriminator/display copy through.

Closes #127 (manual close — default branch is 1.x). Part of #102.

The three enum variant flavours collapse into one enumVariantTypeNode whose payload is an optional data type node: absent for unit variants, a struct for named fields, a tuple for positional fields, and any other type carried as-is — a variant holding a single type needs no tuple around it. The node gains an optional docs attribute matching structFieldTypeNode, the enumVariantTypeNode union is deleted (the name now identifies the node), and enumTypeNode.variants plus registeredTypeNode reference the node directly.
@lorisleiva

Copy link
Copy Markdown
Member Author

@trevor-cortex

@trevor-cortex trevor-cortex left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Summary

Implements #127: collapses enumEmptyVariantTypeNode, enumStructVariantTypeNode, and enumTupleVariantTypeNode into a single enumVariantTypeNode with an optional data: TypeNode payload, deletes the now-redundant enumVariantTypeNode union, and adds an optional docs attribute per the #124 resolution. Docs, spec.json, examples, and tests are all updated in lockstep.

The execution is thorough and consistent:

  • The new node definition mirrors structFieldTypeNode conventions exactly — same docs() usage, same doc wording ("Markdown documentation for the…"), and the name → discriminator → docs → data → display ordering matches the established data-before-children pattern (cf. accountNode).
  • The union removal is applied everywhere it needs to be: TypeNode.ts, ALL_TYPE_NODE_UNIONS, registeredTypeNode members, the test's principal-unions list, and the docs README's Unions section.
  • The new test block covers the shape assertion, the data optionality/union type/child-attribute status, and explicitly asserts the three old nodes and the old union are gone — good regression coverage.
  • Changeset is correctly major per the repo's semver policy, with a clear migration diff.

Notes for subsequent reviewers

  • The generated artifacts (spec.json, docs/) look consistent with the source changes; CI verifies lockstep so no manual diffing needed there.
  • The one thing worth a second opinion is the upgrade guidance's blanket 1-tuple unwrap rule — see my inline comment on the changeset. When the single tuple item is itself a tupleTypeNode or structTypeNode, unwrapping is wire-compatible but changes the variant flavour a renderer would emit (e.g. V(SomeInlineStruct) becomes V { … }). Whether that's acceptable or should be carved out is a judgement call for the upgradeV1ToV2 implementation, but the changeset text is where consumers will look for the rule, so it's worth being precise there.
  • Minor: the migration example in the changeset (and the EnumTypeNode example) still wraps a single u32 in a 1-tuple for the rotate variant, right after the prose recommends unwrapping exactly that shape. Both spellings are valid v2, so this is cosmetic — inline comments below.

Comment thread .changeset/fresh-nights-fail.md Outdated
Comment thread .changeset/fresh-nights-fail.md Outdated
Comment thread src/spec/nodes/typeNodes/EnumTypeNode.examples.ts Outdated
Carves struct, tuple and defined-type-link items out of the single-item tuple unwrap rule — those shapes determine the variant flavour in data position, so unwrapping them would change the generated API despite identical wire format. The migration diff and the flagship enum example now model the recommended unwrapped output for a plain single-type payload.
@lorisleiva
lorisleiva merged commit 996c20a into main Sep 1, 2026
3 checks passed
@lorisleiva
lorisleiva deleted the single-enum-variant branch September 1, 2026 08:20
@lorisleiva lorisleiva linked an issue Sep 1, 2026 that may be closed by this pull request
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.

[v2] Unify enum variants into a single enumVariantTypeNode

2 participants