Skip to content

Enum features - #80

Open
hgiesel wants to merge 7 commits into
madonoharu:mainfrom
hgiesel:feat/enum-galore
Open

hgiesel wants to merge 7 commits into
madonoharu:mainfrom
hgiesel:feat/enum-galore

Conversation

@hgiesel

@hgiesel hgiesel commented Feb 26, 2026 •

Copy link
Copy Markdown
Contributor

A super-PR with all the changes from #77 #78 #79 merged into one big PR. See the individual PRs for more details.

Altogether it allows you to define much more powerful sum types like this, while keeping a very clean API on the Typescript side. Additionaly, you can generate TypeScript enums from Rust which you couldn't generate before at all (only types)

Closes #77
Closes #78
Closes #79

commit 9a6e94a
Author: Henrik Giesel <hengiesel@gmail.com>
Date:   Thu Feb 26 04:32:48 2026 +0100

    fix: rename_variants in absence of discriminants

commit 74ef0c9
Author: Henrik Giesel <hengiesel@gmail.com>
Date:   Thu Feb 26 04:13:58 2026 +0100

    fix: formatting js-idents as enum member names

commit cc6f1e2
Author: Henrik Giesel <hengiesel@gmail.com>
Date:   Thu Feb 26 04:11:01 2026 +0100

    feat: better infer discriminant tag

commit 08ee926
Author: Henrik Giesel <hengiesel@gmail.com>
Date:   Thu Feb 26 03:06:37 2026 +0100

    feat: implement value enum parsing function

commit 82c75b9
Author: Henrik Giesel <hengiesel@gmail.com>
Date:   Thu Feb 26 02:52:03 2026 +0100

    feat: more checks on value enums

commit 4c293b2
Author: Henrik Giesel <hengiesel@gmail.com>
Date:   Thu Feb 26 01:13:36 2026 +0100

    refactor

commit 1daf55b
Author: Henrik Giesel <hengiesel@gmail.com>
Date:   Wed Feb 25 19:59:57 2026 +0100

    refactor: introduce helper DiscriminantsConfig

commit 98e2da4
Author: Henrik Giesel <hengiesel@gmail.com>
Date:   Wed Feb 25 19:09:04 2026 +0100

    feat: pass key through to type alias generation

commit 6e30b0c
Author: Henrik Giesel <hengiesel@gmail.com>
Date:   Wed Feb 25 17:11:08 2026 +0100

    feat: modify TsTypeElement to allow for computed keys

commit 0c51f1a
Author: Henrik Giesel <hengiesel@gmail.com>
Date:   Wed Feb 25 01:53:48 2026 +0100

    feat: rename variant identifier to discriminants

commit 3052661
Author: Henrik Giesel <hengiesel@gmail.com>
Date:   Wed Feb 25 01:24:46 2026 +0100

    feat: add rename_variant option

commit 8dd8dae
Author: Henrik Giesel <hengiesel@gmail.com>
Date:   Wed Feb 25 00:20:58 2026 +0100

    feat: create variant enum

commit 67a6d42
Author: Henrik Giesel <hengiesel@gmail.com>
Date:   Tue Feb 24 21:35:30 2026 +0100

    feat: add TsValueEnum
commit 2048fcd
Author: Henrik Giesel <hengiesel@gmail.com>
Date:   Tue Feb 24 02:22:37 2026 +0100

    test: indent Type aliases

commit 9487104
Author: Henrik Giesel <hengiesel@gmail.com>
Date:   Tue Feb 24 01:54:31 2026 +0100

    test: reformat test with newlines in type aliases

commit acaa76a
Author: Henrik Giesel <hengiesel@gmail.com>
Date:   Tue Feb 24 01:25:07 2026 +0100

    feat: share body formatting code between type aliases and interfaces

commit 1b965f6
Author: Henrik Giesel <hengiesel@gmail.com>
Date:   Tue Feb 24 00:47:22 2026 +0100

    feat: add type_alias argument
commit 20d8c5e
Author: Henrik Giesel <hengiesel@gmail.com>
Date:   Tue Feb 24 19:11:25 2026 +0100

    chore: format

commit c533c47
Author: Henrik Giesel <hengiesel@gmail.com>
Date:   Tue Feb 24 18:37:21 2026 +0100

    feat: refer to types in namespaces directly

commit 7b41820
Author: Henrik Giesel <hengiesel@gmail.com>
Date:   Tue Feb 24 18:25:18 2026 +0100

    test: fix tests

commit 25286f2
Author: Henrik Giesel <hengiesel@gmail.com>
Date:   Tue Feb 24 06:07:54 2026 +0100

    feat: no need to delete type comments on enum variants anymore

commit ade3767
Author: Henrik Giesel <hengiesel@gmail.com>
Date:   Tue Feb 24 05:53:26 2026 +0100

    feat: type refer to namespace via its members
@siefkenj

Copy link
Copy Markdown
Collaborator

Thanks for this!

This super PR is too big to review, so I will look at the individual PRs when I am able.

Do they depend on each other?

@hgiesel

hgiesel commented Feb 26, 2026

Copy link
Copy Markdown
Contributor Author

@siefkenj
bbc2483 is the only commit that depends on the combined functionality

@madonoharu

Copy link
Copy Markdown
Owner

@hgiesel, @siefkenj — closing the loop on your February exchange, six months late.

I posted a first review on #79: #79 (comment). Six blockers, four of them needing a design call rather than a patch — details and repros are there. Those six are in code that #80 shares, so they apply here too.

The first review landed on #79, but #80 is the branch that is further along — the reverse of what the PR numbering suggests. On current main:

cargo test --test discriminants
#79 on main 4 passed, 5 failed
#80 on main 9 passed, 0 failed

All five failures are the namespace + discriminants combinations, and the reason is structural rather than cosmetic. Since #78 merged, main has referenced namespace members by name in the top-level union — GenericEnum.Unit | GenericEnum.NewType<T>. #79 re-expands each variant's full structural type there instead: { [ExternalType.Struct]: { x: string; y: number } } | ... for externally-tagged enums, { t: InternalT.Struct; x: string; y: number } | ... for internally-tagged ones. #80 reconciles the two — TsEnumDecl::Display there grows an if self.namespace branch producing the dot reference — and #79's history has no equivalent. That reconciliation is inside 77a55e9, one of the squashed commits, so it cannot be lifted out as-is.

The three features are not equally far along

Going through the tracker to see what each one answers:

Feature PR What asks for it Where it stands
type_alias #77 #61 — open, two 👍, linked with Closes Smallest diff, clearest demand. as_type_alias is the only thing outstanding
value_enum #79 #25 — four people outside the maintainers. I closed it in April as covered by namespaces; @ignatevdev commented on 2025-10-29, after that close, to say namespaces do not cover it Wanted, but may not reach the numeric case #25 is actually asking for
discriminants #79 No issue I could find Largest piece, blockers 1, 3 and 4 of the six — and the piece that already carries the #78 reconciliation above

@hgiesel — two things from that table, worth knowing whichever way each feature goes:

value_enum may not reach the case that is asking for it. #25's remaining ask is #[repr(u8)] enum PoolToken { A = 0, B = 1 } producing export enum PoolToken { A, B }. I tried it on your branch, with and without serde_repr:

export enum PoolToken { A = "A", B = "B" }

parse_value_enum always emits TsValueEnumLit::StringLit, nothing constructs NumberLit, and serde_repr is not recognized anywhere in the macro crate. If numeric is in scope for you the IR is already there; if it is not, saying so on #25 would stop people from waiting.

I could not find the issue discriminants was built for — if there is one, point me at it. That gap is in what I know, not an argument against the feature: it is the part that already solved the hardest problem here, reconciling with #78. What is missing is the use case on record, so it gets judged on what it is for rather than only on its share of the blockers.

How I intend to land this

Your February point stands — this is too big to review in one piece, and six months of nothing is the proof. So rather than ask anyone to take another run at 1,900 lines, I am going to cut it into a stack. From #80 rather than from #77 and #79, since #80 is the only branch where the enum work and #78 already agree:

Contents Needs
1 type_alias main
2 TsValueEnum + discriminants + the #78 reconciliation main
3 value_enum 2, for the shared IR
4 bbc2483 — keep comments on type aliases inside enums 1 and 2

Row 4 needs 1 for the multi-line body formatting and 2 for ToStringWithIndent — I checked by applying it without row 1, and test_enum_with_type_override fails with doc comments embedded in single-line output.

This is not a re-slicing of existing commits. value_enum and discriminants share the same IR — TsValueEnumDecl, TsValueEnumLit, TsValueEnumMember — so the two interleave across #79's fifteen commits rather than separating cleanly, and row 2 has to carry that 77a55e9 work out of the squash. It is closer to re-authoring than to git rebase -i.

Row 1 goes first and does not wait on the rest, once as_type_alias is settled.

#77, #79 and #80 stay open as the source of record until the stack is up, and I would close them in favor of it rather than the other way around. Nothing you have filed disappears mid-transition.

@hgiesel — I would rather do that cutting than hand it to you. Same offer as on #79: happy to cut the stack from your commits, with authorship kept, if you would rather not carry it after six months of us sitting on the review. If you would rather carry it yourself, say so and it is yours — the six blockers on #79 are the work either way.

@siefkenj — one thing that is yours rather than mine: rename_variants reads as "rename the variants" but does the opposite — it makes TypeScript ignore serde's rename and use the Rust ident (point 11 of the #79 review). Same kind of call you made for type_alias → as_type_alias, and it is a public name, so better to settle now than rename after release. If you have no preference I will pick one before opening row 2.

@madonoharu madonoharu mentioned this pull request Aug 29, 2026
@madonoharu madonoharu added this to the 0.6.0 milestone Aug 29, 2026
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.

3 participants