fix(v2): sort generated enums in enum-only packages#5786
Conversation
Walkthrough
ChangesEnum generation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain modules listed in go.work or their selected dependencies" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
v2/internal/binding/binding.go (1)
90-107: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix variable name typo and consider updating the
seenslice.There is a minor typo in the variable name
fqemumname(should befqenumname). Additionally, while the code checks if the enum is in theseenslice, it never adds the enum toseenafter adding it to the generator. If the intention is to prevent duplicate processing of the same enum name across different iterations, you should add it to theseenslice.♻️ Proposed refactor
- fqemumname := packageName + "." + enumName - if seen.Contains(fqemumname) { + fqenumname := packageName + "." + enumName + if seen.Contains(fqenumname) { continue } w.AddEnum(enum) + seen.Add(fqenumname)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@v2/internal/binding/binding.go` around lines 90 - 107, In addEnumsToGenerator, rename the fqemumname variable to fqenumname and, after w.AddEnum(enum), record fqenumname in the seen StringSlicer so subsequent iterations skip already-added enums.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@v2/internal/binding/binding.go`:
- Around line 90-107: In addEnumsToGenerator, rename the fqemumname variable to
fqenumname and, after w.AddEnum(enum), record fqenumname in the seen
StringSlicer so subsequent iterations skip already-added enums.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0429ebf6-9171-44bf-922a-54225b54d1d7
📒 Files selected for processing (4)
v2/internal/binding/binding.gov2/internal/binding/binding_test/binding_enum_ordering_test.gov2/internal/binding/binding_test/binding_test.gowebsite/src/pages/changelog.mdx
Description
Wails v2.13.0 sorts generated enums when their Go package also contains structs that need TypeScript model generation. However, enum-only packages are handled by a separate "outstanding enums" path that still iterates over a Go map directly.
Because Go map iteration order is unspecified, running binding generation multiple times can reorder otherwise unchanged enum declarations in
models.ts, producing noisy source control diffs. Such as modals.tsThis change:
addEnumsToGenerator, so packages with structs and enum-only packages use the same deterministic path.This is a follow-up to #4664, which fixed enum ordering for packages that also contain generated structs.
Type of change
Please select the option that is relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration using
wails doctor.Test Configuration
Checklist:
website/src/pages/changelog.mdxwith details of this PR (v3 changelog entries are added automatically)Summary by CodeRabbit
Bug Fixes
Tests
Documentation