-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[v3] Binding generator tests for Go 1.24 features #4068
base: v3-alpha
Are you sure you want to change the base?
Conversation
This reverts commit e38684e7885c9c7b5ad3f704ad500c39bbce7715.
This reverts commit 7ed397dc452f420551dfdd05dfe0c6a7646b3ba4.
WalkthroughThe changes update the changelog to note new binding generator tests for Go 1.24. Several Go source files have been modified to uncomment and enable new type aliases, struct definitions, and methods—improving the clarity of type declarations. In addition, multiple JavaScript and TypeScript files have received new exports and functions, particularly a new service method “GetButGenericAliases” which processes alias groups with updated type creation logic. Similar updates extend to map key test cases with expanded generic type constraints and JSDoc/TS type definitions. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant GreetService
participant Call
participant AliasGroupFactory
Client->>GreetService: GetButGenericAliases()
GreetService->>Call: $Call.ByID("914093800")
Call-->>GreetService: Raw Data
GreetService->>AliasGroupFactory: $$createType3(rawData)
AliasGroupFactory-->>GreetService: AliasGroup instance
GreetService-->>Client: Promise(AliasGroup) & { cancel() }
Suggested reviewers
Poem
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 (1.62.2)Error: can't load config: the Go language version (go1.23) used to build golangci-lint is lower than the targeted Go version (1.24.0) ✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/models.js (1)
1332-1824
: 🛠️ Refactor suggestionReduce complexity in createFrom method expansions.
The newly introduced lines within the
createFrom
method add another layer of repetitive mappings for each field (e.g.,$$createField89_0
,$$createField90_0
, etc.). If code generation cannot be avoided, consider introducing higher-level abstractions or references (like arrays or dictionaries of field handlers) to make this section more manageable and avoid potential copy-paste errors.v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/models.js (1)
1332-1824
: 🛠️ Refactor suggestionRefactor the extended createFrom logic.
The expansions in this method again introduce repetitive definitions (
$$createField89_0
,$$createField90_0
, etc.). Explore bundling the repeated logic to a data structure or generator function that can build these mappings automatically, minimizing the risk of manual errors.
🧹 Nitpick comments (16)
v3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/greetservice.ts (1)
50-53
: Add documentation for consistency.The implementation looks good, but consider adding JSDoc documentation to match the style of other functions in the file.
Add documentation block before the function:
+/** + * Get generic aliases. + */ export function GetButGenericAliases(): Promise<$models.AliasGroup> & { cancel(): void } {v3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/models.ts (2)
9-21
: Consider using more descriptive field names.While the interface is well-documented, the field names (GAi, GAP, GPAs, etc.) could be more descriptive to improve code readability and maintainability.
Consider renaming fields to be more self-documenting, for example:
export interface AliasGroup { - "GAi": GenericAlias<number>; - "GAP": GenericAlias<GenericPerson<boolean>>; - "GPAs": GenericPtrAlias<string[] | null>; + "genericAliasNumber": GenericAlias<number>; + "genericAliasPerson": GenericAlias<GenericPerson<boolean>>; + "genericPtrAliasStrings": GenericPtrAlias<string[] | null>; // ... similar changes for other fields }
67-70
: Consider using a more flexible key type for GenericMapAlias.The current implementation restricts map keys to string type. Consider using a generic parameter for the key type with appropriate constraints.
-export type GenericMapAlias<T, U> = { [_: string]: U } | null; +export type GenericMapAlias<K extends string | number | symbol, V> = { [P in K]: V } | null;v3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/models.ts (1)
16-54
: Consider marking optional fields as optional rather than defaulting to null.
Currently, fields like"GPAs"
,"GPAP"
are assignednull
if missing. Marking them with?
or using a more descriptive default could improve clarity and type safety.- "GPAs": GenericPtrAlias<string[]>; ... - this["GPAs"] = null; + "GPAs"?: GenericPtrAlias<string[]>; ... + this["GPAs"] = this["GPAs"] ?? null;v3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/models.ts (2)
16-54
: Avoid using quoted property names unless necessary.
While valid, string-literal keys like"GAi"
,"GAP"
are atypical in TypeScript code and can make property references more cumbersome for developers.-export class AliasGroup { - "GAi": GenericAlias<number>; - "GAP": GenericAlias<GenericPerson<boolean>>; +export class AliasGroup { + GAi: GenericAlias<number>; + GAP: GenericAlias<GenericPerson<boolean>>;
56-91
: EnsurecreateFrom
handles unexpected field types.
If the parsed JSON includes incorrect types for these properties, it might lead to runtime issues. Consider applying type guards.v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/models.js (1)
83-120
: Enhance createFrom with error handling.
Same feedback as the TypeScript files: wrap JSON parsing in a try/catch to prevent runtime crashes on malformed data.v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/models.js (3)
17-81
: Consider preservingundefined
values.
The constructor defaults missing properties to non-null defaults, which might mask intentionalundefined
from upstream code. If preservingundefined
is desired, consider a more nuanced default assignment strategy.
222-226
: Possible duplication forGenericPersonAlias
.
You're directly exportingGenericPersonAlias
asGenericPerson
here, but a subsequent block also re-declares it with a different doc comment. Consider unifying them into a single approach to avoid overshadowing or confusion.
325-335
: Potential for more specific type checks.
All private creation functions use$Create.Any
. If stronger type-safety is desired, consider specialized creation methods or explicit validations.v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/models.js (2)
9-12
: Consider consolidating repeated alias definitions.These template-based alias definitions (e.g.,
BasicCstrAlias
,ComparableCstrAlias
,GoodTildeCstrAlias
,InterfaceCstrAlias
) follow the same straightforward pattern. In a codebase with numerous similar alias definitions, a more automated or unified approach might reduce maintenance overhead and potential duplication.Also applies to: 14-17, 43-46, 48-51
774-1221
: Evaluate constructor size and repetitive property initialization.Many new properties (
GAR
,GARPtr
,GAS
,GASPtr
, etc.) are initialized repeatedly with nearly identical logic. This repetition may affect readability and maintainability. Consider grouping these checks or employing a more dynamic approach (e.g., a single map/list for property names and conditions) to reduce duplication and enhance clarity.v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/models.js (2)
9-12
: Unify similar alias definitions.As with the other file, these template aliases (
BasicCstrAlias
,ComparableCstrAlias
,GoodTildeCstrAlias
,InterfaceCstrAlias
) repeat a shared pattern. Consider a centralized approach to alias creation or a code generator that reduces duplication and risk of omission.Also applies to: 14-17, 43-46, 48-51
774-1221
: Streamline repetitive constructor checks.These newly added property checks (
GAR
,GARPtr
,GAS
,GASPtr
, etc.) follow the same pattern as in the previous file, potentially complicating readability. A configuration-driven approach (e.g., iterating over an array of property keys) can simplify code and reduce duplication.v3/internal/generator/testcases/aliases/main.go (1)
64-67
: Consider adding a custom constraint interface.The generic constraint combines
comparable
andencoding.TextMarshaler
. Consider extracting this into a named interface for better reusability and documentation.+// TextMarshalable represents a comparable type that can be marshaled to text +type TextMarshalable interface { + comparable + encoding.TextMarshaler +} -type GenericMapAlias[T interface { - comparable - encoding.TextMarshaler -}, U any] = map[T]U +type GenericMapAlias[T TextMarshalable, U any] = map[T]Uv3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/greetservice.ts (1)
66-73
: Add JSDoc comments for the new function.The new
GetButGenericAliases
function lacks documentation. Consider adding JSDoc comments to explain its purpose and usage.Add documentation above the function:
+/** + * Returns a group of generic aliases. + * @returns {Promise<$models.AliasGroup> & { cancel(): void }} + */ export function GetButGenericAliases(): Promise<$models.AliasGroup> & { cancel(): void } {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=false/warnings.log
is excluded by!**/*.log
v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=true/warnings.log
is excluded by!**/*.log
v3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=false/warnings.log
is excluded by!**/*.log
v3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=true/warnings.log
is excluded by!**/*.log
v3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=false/warnings.log
is excluded by!**/*.log
v3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=true/warnings.log
is excluded by!**/*.log
v3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=false/warnings.log
is excluded by!**/*.log
v3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=true/warnings.log
is excluded by!**/*.log
📒 Files selected for processing (46)
docs/src/content/docs/changelog.mdx
(1 hunks)v3/go.mod
(1 hunks)v3/internal/generator/testcases/aliases/main.go
(3 hunks)v3/internal/generator/testcases/complex_instantiations/bound_types.json
(1 hunks)v3/internal/generator/testcases/complex_instantiations/main.go
(2 hunks)v3/internal/generator/testcases/map_keys/main.go
(2 hunks)v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/greetservice.js
(3 hunks)v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/index.js
(2 hunks)v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/models.js
(4 hunks)v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/index.js
(2 hunks)v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/models.js
(6 hunks)v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/greetservice.js
(3 hunks)v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/index.js
(2 hunks)v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/models.js
(4 hunks)v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/index.js
(2 hunks)v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/models.js
(6 hunks)v3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/greetservice.js
(1 hunks)v3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/index.js
(3 hunks)v3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/models.js
(4 hunks)v3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/index.js
(2 hunks)v3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/models.js
(3 hunks)v3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/greetservice.js
(1 hunks)v3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/index.js
(3 hunks)v3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/models.js
(4 hunks)v3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/index.js
(2 hunks)v3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/models.js
(3 hunks)v3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/greetservice.ts
(2 hunks)v3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/index.ts
(1 hunks)v3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/models.ts
(4 hunks)v3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/index.ts
(1 hunks)v3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/models.ts
(7 hunks)v3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/greetservice.ts
(2 hunks)v3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/index.ts
(1 hunks)v3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/models.ts
(4 hunks)v3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/index.ts
(1 hunks)v3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/models.ts
(7 hunks)v3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/greetservice.ts
(1 hunks)v3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/index.ts
(1 hunks)v3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/models.ts
(4 hunks)v3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/index.ts
(1 hunks)v3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/models.ts
(3 hunks)v3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/greetservice.ts
(1 hunks)v3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/index.ts
(1 hunks)v3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/models.ts
(4 hunks)v3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/index.ts
(1 hunks)v3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/models.ts
(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- v3/go.mod
👮 Files not reviewed due to content moderation or server errors (4)
- v3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/models.js
- v3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/models.ts
- v3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/models.ts
- v3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/models.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run Go Tests (ubuntu-latest, 1.23)
- GitHub Check: Run Go Tests (windows-latest, 1.23)
🔇 Additional comments (72)
v3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/index.ts (2)
1-2
: LGTM! Auto-generated file warning is present.The file correctly includes a warning comment indicating it's auto-generated and should not be edited manually.
11-11
: LGTM! New type exports for Go 1.24 features.The new type exports align with the PR objectives of testing Go 1.24 features, particularly around generic type aliases. The additions include:
- Generic type aliases:
GenericAlias
,GenericMapAlias
,GenericPersonAlias
,GenericPtrAlias
- Indirect aliases:
IndirectPersonAlias
,TPIndirectPersonAlias
- Container type:
AliasGroup
Also applies to: 16-18, 19-21, 22-22, 24-25
v3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/models.ts (1)
90-94
: LGTM! Well-structured type definitions.The indirect type aliases are well-documented and demonstrate good use of TypeScript's type system features.
Also applies to: 122-125
v3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/index.ts (3)
1-2
: LGTM: Auto-generated file warning is present.The warning comments clearly indicate this is an auto-generated file that should not be manually edited.
9-19
: LGTM: Value exports are well-organized.The value exports are cleanly organized and include the new generic-related aliases (
AliasGroup
,GenericPersonAlias
,IndirectPersonAlias
,TPIndirectPersonAlias
) while maintaining existing exports.
21-29
:✅ Verification successful
LGTM: Type exports support Go 1.24 features.
The type exports appropriately include the new generic type aliases (
GenericAlias
,GenericMapAlias
,GenericPtrAlias
) required for Go 1.24 compatibility.Since this PR is specifically for Go 1.24 features, let's verify the Go version requirement:
🏁 Script executed:
#!/bin/bash # Check go.mod for Go version grep -r "go 1.24" .Length of output: 41
LGTM: Type exports correctly support Go 1.24 features.
- The new generic type aliases (GenericAlias, GenericMapAlias, GenericPtrAlias) are properly exported.
- The repository’s
go.mod
file confirms the required Go version is set to 1.24.No further changes are needed.
v3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/index.ts (2)
1-2
:✅ Verification successful
LGTM! Auto-generated file.
This is an auto-generated file, and the changes align with the PR objectives to support Go 1.24 features, particularly around generic type aliases.
Since this is an auto-generated file, let's verify the generator's output:
🏁 Script executed:
#!/bin/bash # Description: Verify that the file is auto-generated and matches the expected output # Check if the file is being generated by the correct tool rg -A 2 "This file is automatically generated" v3/internal/generator/ # Check for any inconsistencies in the generated exports ast-grep --pattern 'export { $$$, GenericAlias, $$$, GenericMapAlias, $$$, GenericPtrAlias, $$$ } from "./models.js"'Length of output: 70445
LGTM! Auto-generated file verified successfully.
- The file header correctly indicates it is auto-generated ("DO NOT EDIT") and matches the expected output.
- The verification scripts confirmed the presence of the auto-generation comment and validated that the expected export patterns (including the new generic alias exports) are in place.
- The changes align with the PR objectives to support Go 1.24 features regarding generic type aliases.
9-19
:✅ Verification successful
New exports align with Go 1.24 generic features.
The new exports (AliasGroup, GenericPersonAlias, etc.) and type exports (GenericAlias, GenericMapAlias, etc.) correctly reflect the binding generator's support for Go 1.24 generic features.
Let's verify the Go source for these generic types:
Also applies to: 21-29
🏁 Script executed:
#!/bin/bash # Description: Verify the Go source for generic types # Check for generic type declarations in the Go source ast-grep --pattern 'type $_ interface { $$$ }' # Check for generic type aliases ast-grep --pattern 'type $_ = $_[$_]'Length of output: 56986
Verification: Go 1.24 Generics Export Alignment Achieved
The Go source verification confirms that the expected generic type aliases—such as the ones declared in
v3/internal/generator/testcases/aliases/main.go
(e.g.,
IndirectPersonAlias = GenericPersonAlias[bool]
TPIndirectPersonAlias = GenericAlias[GenericPerson[bool]]
)are correctly present. The TypeScript export file (which re-exports models from
"./models.js"
) now aligns with and reflects these updates, confirming that the binding generator is properly supporting the new Go 1.24 generic features.
- Verified that generic type aliases are defined as expected in the Go source.
- Confirmed that the TS exports (AliasGroup, AliasedPerson, EmptyStruct, GenericPerson, GenericPersonAlias, IndirectPersonAlias, Person, StrangelyAliasedPerson, and TPIndirectPersonAlias) correctly reflect these generic features.
Approved.
v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/index.js (2)
10-20
: LGTM! New exports are properly structured.The new exports are well-organized and follow proper JavaScript module syntax. The trailing comma is a good practice for version control.
40-57
:✅ Verification successful
LGTM! Generic type definitions are well-documented.
The new generic type definitions are properly structured with clear JSDoc documentation. This aligns well with Go 1.24's enhanced generic type support.
🌐 Web query:
What are the new generic type features in Go 1.24?
💡 Result:
Go 1.24 introduces full support for generic type aliases, a significant enhancement to Go's generics system first introduced in Go 1.18. This update allows developers to parameterize type aliases with generic constraints, improving code flexibility and maintainability.
Key Features of Generic Type Aliases
Parameterized Type Aliases
You can now define type aliases with their own generic parameters:type MyAlias[T int | string] = T // Valid in Go 1.24This enables concise abstractions while preserving type safety[1][5].
Refactoring Support
Generic aliases simplify moving types between packages without breaking existing code:// oldpkg/container.go type Container[T any] = newpkg.Container[T]This backward compatibility reduces friction during large-scale codebase changes[5][9].
Interchangeability
Aliases remain fully interchangeable with their original types, even when generics are involved:var num MyAlias[int] = 10 // Works like 'int' var str MyAlias[string] = "Hello" // Works like 'string'Benefits
- Improved Readability: Simplify complex generic declarations (e.g.,
type StringSet = Set[string]
)[5][9].- Code Reusability: Create reusable abstractions for common patterns (e.g., sets or pairs)[3][5].
- Safer Refactoring: Migrate types across packages while maintaining compatibility[5][9].
Temporary Disable Option
Developers can disable this feature using
GOEXPERIMENT=noaliastypeparams
, though this flag will be removed in Go 1.25[1][8].Example: Implementing a Generic Set
type Set[T comparable] map[T]struct{} func NewSet[T comparable](items ...T) Set[T] { s := make(Set[T]) for _, item := range items { s[item] = struct{}{} } return s } // Alias for string sets type StringSet = Set[string]This pattern reduces boilerplate while maintaining type safety[3][5].
The addition of generic type aliases marks a maturation of Go's generics implementation, addressing long-standing developer requests for more flexible type abstraction tools[7][8].
Citations:
- 1: https://dev.to/leapcell/go-124-released-massive-optimizations-key-upgrades-1mbn
- 2: https://linuxiac.com/go-1-24-released-with-faster-performance-better-tooling/
- 3: https://www.youtube.com/watch?v=WJ_-z-mLsOE
- 4: https://www.youtube.com/watch?v=cmtFI9eZ_UE
- 5: https://www.bytesizego.com/blog/go-124-generic-type-alias
- 6: https://madappgang.com/blog/go-12-4-drops-a-quntum-bomb-your-code-will-never-be-the-same/
- 7: https://go.dev/blog/go1.24
- 8: https://www.infoworld.com/article/3627904/go-1-24-brings-full-support-for-generic-type-aliases.html
- 9: https://golangnugget.com/p/what-to-expect-from-go-1-24-part-1
- 10: https://mojoauth.com/blog/exploring-go-1-24-a-deep-dive-into-the-new-go-tool-and-its-impact-on-the-ecosystem/
LGTM! Generic type definitions are clear and correctly leverage Go 1.24's support for generic type aliases.
The new type definitions:
- GenericAlias
- GenericMapAlias<T, U>
- GenericPtrAlias
are well-documented using
@template
and@typedef
annotations and follow standard JSDoc conventions. This implementation aligns perfectly with Go 1.24’s enhancement for parameterized type aliases, ensuring both clarity and compatibility.v3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/index.js (1)
17-20
: Well-structured type definitions for Go 1.24 features!The new type definitions are clear, properly documented, and correctly implement Go 1.24's generic type features in JavaScript. The JSDoc comments and type references are consistent and follow best practices.
Also applies to: 43-47, 49-53, 61-65, 67-71, 73-76, 93-96
v3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/greetservice.js (1)
59-65
: Clean implementation of GetButGenericAliases!The new method is well-documented and follows the established pattern for service methods. The return type annotation correctly reflects the Go 1.24 generic type.
v3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/models.js (2)
10-21
: Excellent implementation of Go 1.24's generic type system!The type definitions effectively capture Go 1.24's generic features with proper JSDoc annotations, nested type handling, and null safety.
Also applies to: 50-54, 70-74, 76-80, 82-85, 105-108
56-60
:❓ Verification inconclusive
Verify GenericMapAlias type parameter usage.
The type parameter
T
inGenericMapAlias<T,U>
is defined but not used in the type definition. The map keys are alwaysstring
despite having a type parameterT
. Please verify if this is the intended behavior or if the type parameter should be used for the key type.
🏁 Script executed:
#!/bin/bash # Search for the Go definition of GenericMapAlias to verify the intended behavior ast-grep --pattern 'type GenericMapAlias[T, U any] map[T]U'Length of output: 61
Verify usage of type parameter
T
in GenericMapAlias
- The JSDoc typedef for
GenericMapAlias
defines two type parameters (T
andU
), yet onlyU
is used as the type for the map’s values while the keys are hardcoded asstring
.- A search using the provided pattern for a corresponding Go definition (e.g.
type GenericMapAlias[T, U any] map[T]U
) returned no results, so the intended design isn’t clear.- Please verify if the design should allow a generic key (i.e. use
T
to type the keys) or if the unusedT
should be removed from the typedef to avoid confusion.Location:
v3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/models.js
(Lines 56–60)v3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/models.ts (3)
13-15
: Doc comment updated successfully.
No functional or correctness issues identified.
56-91
: Add robust error handling for JSON parsing.
If$$source
is a malformed string,JSON.parse($$source)
will throw an exception. Consider wrapping withtry/catch
or verifying string structure before parsing.
184-193
: Name collision between the const and type alias forGenericPersonAlias
.
Declaringexport const GenericPersonAlias = GenericPerson;
andexport type GenericPersonAlias<T> = GenericPerson<GenericPtrAlias<T>[]>;
under the same name may cause confusion or overshadowing in certain TypeScript compilers or tooling.v3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/models.ts (2)
13-15
: Doc comment inclusion is clear.
No issues spotted with these lines.
184-193
: Double-check overshadowing ofGenericPersonAlias
.
Same concern as in the other file: using aconst
and atype
alias under the same identifier can lead to confusing, albeit possibly valid, code.v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/models.js (3)
14-16
: Class JSDoc comment looks good.
No immediate issues detected with the doc comment.
17-82
: Check usage ofnew
with type aliases.
Properties like"GAP"
or"IPA"
are instantiated vianew IndirectPersonAlias()
. IfIndirectPersonAlias
is purely a type alias, it may cause confusion. Ensure the underlying runtime object is indeed a class constructor.
314-335
: Private type creation functions appear consistent.
No syntactic anomalies observed. Good use of$Create
helpers.v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/models.js (10)
14-16
: Class documentation looks good.
No issues found with the doc comment describing the class intent.
83-120
: Factory method looks solid.
Implementation for parsing string-to-object or using an existing object is logically consistent.
162-167
: Typedef forGenericAlias
appears correct.
No concerns regarding the doc comment or the template parameter usage.
168-172
:GenericMapAlias
definition is fine.
Straightforward definition for a dictionary-like alias.
227-231
: Consistent doc definition.
The typedef references a parameterizedGenericPerson
, which aligns with the preceding alias usage.
233-237
:GenericPtrAlias
definition is minimal and correct.
No concerns here.
239-243
:IndirectPersonAlias
export is clear.
It cleanly aliasesGenericPersonAlias
to a new name.
244-248
: Comment consistency is good.
Documentation clarifies the alias semantics.
315-319
:TPIndirectPersonAlias
assignment is straightforward.
No issues with assigningGenericPerson
to another export.
320-323
: Accurate typedef forTPIndirectPersonAlias
.
Aligns with the preceding export statement.v3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/models.ts (6)
8-9
: Basic type aliases look fine.
They simply forward the type parameter as-is.
24-25
:GoodTildeCstrAlias
is straightforward.
No issues with this alias.
26-27
:InterfaceCstrAlias
is minimal but correct.
Implements the same forwarding logic as the other aliases.
474-753
: Large block of new map properties.
These properties have varied acceptance or rejection comments, likely correlating with project logic. Ensure the doc comments or classification ("Accept", "Reject", "Soft reject") align with any future merges or usage changes.
754-1789
: Constructor andcreateFrom
expansions appear consistent.
They systematically add default objects for new fields and handle them in the parse logic. This is standard for the generated code and matches the existing patterns.
1788-1791
: Additional type aliases.
The newMixedCstrAlias
,NonBasicCstrAlias
, andPointableCstrAlias
mirror the pattern of simple generic forwarding. No concerns here.v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/models.js (2)
1830-1844
: Evaluate the naming and usage of new CstrAlias types.The newly added
MixedCstrAlias
,NonBasicCstrAlias
, andPointableCstrAlias
definitions appear consistent with existing patterns but introduce more symbolic complexity. If extensively used, confirm that these aliases are clearly documented and tested to maintain clarity for future contributors.
1947-1949
: Confirm the new $$createType77, $$createType78, and $$createType79 additions.These newly introduced type-creation functions replicate the existing
$Create.Map($Create.Any, $Create.Any)
pattern. Verify that they serve a distinct purpose and are not redundant with previously generated functions.v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/models.js (2)
1830-1844
: Validate newly added alias definitions.The new
MixedCstrAlias
,NonBasicCstrAlias
, andPointableCstrAlias
definitions increase your type coverage, but ensure they are tested and documented so future maintainers quickly grasp their intended usage.
1947-1949
: Confirm the purpose of $$createType77, $$createType78, and $$createType79.These additions resemble prior type-creation functions, so verify they align with a new requirement or usage rather than duplicating existing logic.
v3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/index.ts (1)
11-11
: Acknowledge newly exported alias types.These additional exports (
AliasGroup
,GenericAlias
,GenericMapAlias
,GenericPersonAlias
,GenericPtrAlias
,IndirectPersonAlias
,TPIndirectPersonAlias
) broaden the type system and likely support new functionality. Ensure you have or add appropriate tests wherever these types are consumed, and confirm documentation clarity since they're newly introduced or updated.Also applies to: 16-17, 19-21, 24-25
v3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/index.ts (1)
10-11
:✅ Verification successful
LGTM! New type exports align with Go 1.24 map key constraints.
The added type exports (BasicCstrAlias, ComparableCstrAlias, etc.) correctly reflect Go 1.24's map key constraint features in the TypeScript bindings.
Let's verify these types match Go 1.24's map key constraint features:
Also applies to: 18-19, 21-23
🌐 Web query:
What are the new map key constraint features in Go 1.24?
💡 Result:
Go 1.24 introduced significant performance improvements for maps through its new Swiss Table-based implementation, but there are no new map key constraint features added in this release[1][2][8]. The changes focus entirely on optimizing map operations rather than altering key requirements or behaviors:
Key Map-Related Changes in Go 1.24
Swiss Table Implementation
- Uses metadata bits (H1/H2 hash splitting) for faster lookups[2]
- Stores entries in contiguous memory groups (16 entries per group)[2]
- Eliminates overflow chains through linear probing and tombstones[2]
Performance Improvements
|| Before (Go ≤1.23) || After (Go 1.24) ||
|--------------------------|--------------------------|
| Lookup Time | Slower due to cache misses | 6-26% faster[4] |
| Insertion Speed | Linear degradation | Maintains performance at high load[1] |
| Memory Usage | Higher overhead | Reduced by ~15% for large maps[8] |The existing map key constraints remain unchanged:
- Keys must be comparable (
==
operator)- Slice/function/map types still cannot be used as keys[7][9]
- No support for duplicate keys or multiple values per key[9]
For cryptographic applications, RSA key constraints were tightened (minimum 1024-bit keys), but this applies specifically to
crypto/rsa
rather than general maps[10].Citations:
- 1: https://www.bytesizego.com/blog/go-124-swiss-table-maps
- 2: https://dev.to/tuna99/go-124-and-the-swiss-table-revolution-a-new-era-for-maps-1pdj
- 3: https://go.dev/blog/go1.24
- 4: https://antonz.org/go-1-24/
- 5: https://blog.howardjohn.info/posts/go-tools-command/
- 6: https://golangnugget.com/p/what-to-expect-from-go-1-24-part-2-finializer-and-weak-pointers
- 7: https://go.dev/ref/spec
- 8: https://madappgang.com/blog/go-12-4-drops-a-quntum-bomb-your-code-will-never-be-the-same/
- 9: https://blog.stackademic.com/a-beginners-guide-to-mastering-maps-in-go-dab44bb9b83b
- 10: https://tip.golang.org/doc/go1.24
LGTM! New type exports correctly mirror Go’s map key comparability requirements.
The additions (BasicCstrAlias, ComparableCstrAlias, GoodTildeCstrAlias, InterfaceCstrAlias, MixedCstrAlias, NonBasicCstrAlias, and PointableCstrAlias) provide a clear TypeScript representation of Go’s map key constraints. Note that while Go 1.24 introduced performance improvements with its Swiss Table-based maps, there are no changes to the actual map key constraints. These exports continue to reflect the standard requirements for map keys, ensuring consistency in the generated bindings.
- Applies to file:
v3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/index.ts
(Lines 10–11, also affecting sections at lines 18–19 and 21–23).v3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/index.ts (1)
10-11
: Changes are consistent with the previous file.Also applies to: 18-19, 21-23
v3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/index.ts (1)
14-15
: Changes are consistent with the previous files.Also applies to: 22-26
v3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/index.ts (1)
14-15
: Changes are consistent with the previous files.Also applies to: 22-26
v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/index.js (2)
11-20
: LGTM! New exports enhance type safety.The new exports (
AliasGroup
,GenericPersonAlias
,IndirectPersonAlias
,TPIndirectPersonAlias
) are well-organized and properly integrated with existing exports.
40-56
: LGTM! Well-documented generic type definitions.The new generic type definitions (
GenericAlias
,GenericMapAlias
,GenericPtrAlias
) are properly documented with JSDoc comments and follow TypeScript's template syntax conventions.v3/internal/generator/testcases/complex_instantiations/main.go (2)
27-27
: LGTM! Type alias enhances code reusability.The
SimplifiedFactory
type alias with generic constraint[T any]
is well-defined and follows Go's type system conventions.
45-45
: LGTM! Service configuration is properly integrated.The
SimplifiedFactory[Service14]{}
instantiation and itsGet()
method usage are correctly integrated into the service configuration.v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/index.js (1)
16-24
: LGTM! Comprehensive generic type definitions.The new generic type definitions are well-documented and properly structured:
- Each type has clear JSDoc documentation
- Template parameters follow consistent naming conventions
- Type constraints are properly defined through the models import
Also applies to: 50-73
v3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/index.js (1)
12-74
: Well-structured generic type definitions!The new generic type aliases are properly documented with JSDoc comments and follow consistent patterns. The use of @template for generic type parameters is correct and the naming conventions are clear.
v3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/greetservice.ts (1)
50-53
: Clean implementation of GetButGenericAliases!The new service method follows the established patterns in the codebase, with proper type annotations and consistent error handling.
v3/internal/generator/testcases/aliases/main.go (1)
109-111
:❓ Verification inconclusive
Consider initializing AliasGroup fields.
The method returns an empty AliasGroup. Consider initializing fields with meaningful default values to avoid potential nil pointer dereferences when using GenericPtrAlias fields.
🏁 Script executed:
#!/bin/bash # Check if there are any tests that verify the behavior of zero values rg -l "GetButGenericAliases.*nil" -g "*.go"Length of output: 45
I'll run a few more searches to inspect the definition and usage of
AliasGroup
—in particular, to confirm whether it contains pointer fields (such as those related toGenericPtrAlias
) that might require explicit initialization. That way, we can verify if returning it uninitialized is truly a risk.Please run the following shell script to further gather the necessary information:
Once you review the output of these commands, please confirm whether the
AliasGroup
type indeed has pointer fields that are used without initialization. If so, the following rewritten review comment will apply. Otherwise, if the design intentionally relies on zero-values and nils are handled safely, then this concern may be less urgent.
Rewritten Review Comment
Review Note: Verify AliasGroup Field Initialization
The method
GetButGenericAliases
in v3/internal/generator/testcases/aliases/main.go (lines 109–111) returns an emptyAliasGroup
. IfAliasGroup
has pointer fields—for example, those related toGenericPtrAlias
—leaving them as nil could lead to runtime nil pointer dereferences when these fields are used. Please verify the definition ofAliasGroup
; if pointer fields are present and non-nil defaults are expected, consider initializing these fields with meaningful values to improve reliability.
🏁 Script executed:
[run_scripts] #!/bin/bash # Search for the definition of AliasGroup to inspect its fields, # especially checking for pointer fields like those for GenericPtrAlias. rg -n "type AliasGroup" -g "*.go" # Look for any explicit initializations of AliasGroup within the repository. rg -n "AliasGroup{" -g "*.go" # Search for keyword 'GenericPtrAlias' to see how it's used across the codebase. rg -n "GenericPtrAlias" -g "*.go"Length of output: 783
AliasGroup Field Initialization Verification
The
GetButGenericAliases
method currently returns an uninitializedAliasGroup
value. Our investigation confirms thatAliasGroup
is defined in v3/internal/generator/testcases/aliases/main.go (line 79) and includes pointer-typed fields (e.g., viaGenericPtrAlias
). This raises the potential for nil pointer dereferences if these fields are later dereferenced without proper initialization. Please verify if the design intends to handle nil values safely; otherwise, consider initializing these fields with appropriate default values.v3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/models.ts (1)
12-21
: LGTM! TypeScript type definitions accurately mirror Go types.The TypeScript interfaces and type aliases correctly represent their Go counterparts, including proper generic type constraints and nullable types.
Also applies to: 62-126
v3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/models.js (1)
10-108
: LGTM! JSDoc type definitions accurately mirror Go and TypeScript types.The JSDoc type definitions correctly represent their Go and TypeScript counterparts, including proper generic type constraints and nullable types.
v3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/greetservice.js (1)
59-65
: LGTM! Service method implementation is correct.The GetButGenericAliases method is properly implemented with correct type annotations and error handling.
v3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/greetservice.ts (1)
91-100
: LGTM! Type creation functions are well-structured.The type creation functions are properly defined to support Go 1.24's type system features. The structure is consistent with the auto-generated nature of the file.
v3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/greetservice.ts (1)
1-101
: LGTM! Changes are consistent with the UseNames=true configuration.The implementation correctly uses
$Call.ByName
instead of$Call.ByID
while maintaining the same functionality.v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/greetservice.js (1)
1-116
: LGTM! JavaScript implementation matches the TypeScript version.The JavaScript implementation correctly mirrors the TypeScript version while maintaining proper JSDoc type annotations.
v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/greetservice.js (1)
1-116
: LGTM! Changes are consistent across all configurations.The implementation correctly handles the UseNames=true configuration while maintaining consistency with other versions.
v3/internal/generator/testcases/map_keys/main.go (2)
109-126
: LGTM! Well-structured type alias definitions.The type aliases are well-organized and properly constrained, providing good type safety for the map keys implementation.
229-286
: LGTM! Comprehensive map type testing.The Maps struct provides extensive test coverage for various type constraints and edge cases, ensuring robust type safety in the binding generator.
v3/internal/generator/testcases/complex_instantiations/bound_types.json (1)
15-16
: LGTM!The new service entries follow the existing pattern and are properly formatted.
docs/src/content/docs/changelog.mdx (1)
55-55
: LGTM!The changelog entry is properly formatted, categorized under "Added", and includes appropriate attribution and PR reference.
v3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/index.js (1)
16-74
: LGTM! Well-structured generic type definitions.The new generic type aliases are well-defined with proper JSDoc syntax, template parameters, and clear references to the underlying model types.
v3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/index.js (1)
12-75
: Identical changes to the other index.js file.These changes mirror those in the UseInterfaces=false mode, maintaining consistency across different generator modes.
v3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/aliases/index.js (1)
17-97
: LGTM! Well-documented type aliases with clear descriptions.The new type aliases are properly documented with descriptive comments explaining their purpose and constraints. The generic type parameters are well-defined and follow consistent naming conventions.
v3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/models.js (3)
5-43
: LGTM! Well-defined generic type aliases.The generic type aliases are properly defined with clear template parameters.
199-212
: LGTM! Additional generic type aliases are well-defined.The remaining generic type aliases follow the same pattern and are properly defined.
141-197
:✅ Verification successful
Verify compatibility with Go 1.24 map key constraints.
The new Maps properties use complex type constraints that depend on Go 1.24 features. Let's verify the compatibility.
🌐 Web query:
What are the new map key type constraint features in Go 1.24?
💡 Result:
Go 1.24 introduces several enhancements related to maps and generic type constraints, though no fundamental changes to map key type requirements beyond existing Go rules[6][9]. The primary advancements focus on performance optimizations and expanded generic capabilities:
Map Key Constraints (Existing Rules)
- Comparable types requirement remains: Map keys must still be comparable types (e.g., basic types, pointers, structs/arrays of comparable types)[6][9]
- Interface keys require dynamic comparability: When using interface types as keys:
var m map[interface{}]int m["string-key"] = 5 // Valid (string is comparable) m[[]byte{1}] = 10 // Runtime panic (slice not comparable)New Related Features in Go 1.24
1. Enhanced Generic Constraints
- Generic type aliases: Enables more flexible parameterization of type definitions[7][8]
type ComparableMap[K comparable, V any] = map[K]V- Extended interface constraints: Builds on Go's existing interface-based generics system[4]
2. Swiss Table Implementation
While not a key constraint change, the new map implementation affects how keys are handled:
- Optimized hash probing: Uses metadata bits for faster key comparisons[1][5]
- Better collision resolution: Linear probing replaces chained overflow buckets[5]
- Memory efficiency: Reduces overhead through compact storage[1]
Key Comparison Improvements
The new Swiss Table implementation enhances key handling through:
- Tombstone markers: Better handles deleted keys during rehashing[5]
- SIMD optimizations: Faster bulk comparisons of metadata bits[5]
Comparison Table: Old vs New Map Implementation
Feature Pre-Go 1.24 Go 1.24+ Collision Resolution Chained overflow buckets Linear probing Metadata Storage Separate tophash array Integrated bitmask Key Comparison Sequential checks Parallel SIMD checks Memory Usage Higher overhead ~15% reduction These changes maintain backward compatibility while providing significant performance gains for maps with comparable keys[1][5][8]. Developers get faster lookups/insertions without modifying existing code that adheres to Go's established key comparability rules[6][9].
Citations:
- 1: https://www.bytesizego.com/blog/go-124-swiss-table-maps
- 2: https://www.youtube.com/watch?v=cmtFI9eZ_UE
- 3: https://dolthub.com/blog/2024-12-05-whats-missing-from-golang-generics/
- 4: https://go101.org/generics/555-type-constraints-and-parameters.html
- 5: https://dev.to/tuna99/go-124-and-the-swiss-table-revolution-a-new-era-for-maps-1pdj
- 6: https://tip.golang.org/ref/spec
- 7: https://tip.golang.org/doc/go1.24
- 8: https://madappgang.com/blog/go-12-4-drops-a-quntum-bomb-your-code-will-never-be-the-same/
- 9: https://go101.org/article/type-system-overview.html
- 10: https://dev.to/leapcell/go-124-released-massive-optimizations-key-upgrades-1mbn
Compatibility Verified: Map key constraints remain consistent with Go 1.24 standards
Based on the latest Go 1.24 updates, there are no fundamental changes to map key type constraints beyond the established comparable type rules. The enhancements around generic type aliases and performance optimizations do not impact the validity of our map properties. The annotated definitions in the file (Lines 141–197 in
v3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/map_keys/models.js
) continue to be compliant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/src/content/docs/changelog.mdx
(1 hunks)v3/internal/generator/testcases/complex_json/main.go
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/src/content/docs/changelog.mdx
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Run Go Tests (ubuntu-latest, 1.24)
- GitHub Check: Run Go Tests (macos-latest, 1.24)
- GitHub Check: Run Go Tests (windows-latest, 1.24)
🔇 Additional comments (1)
v3/internal/generator/testcases/complex_json/main.go (1)
55-55
: LGTM! The newomitzero
tag is a Go 1.24 feature.The change from
omitempty
toomitzero
is appropriate for testing Go 1.24 features. Whileomitempty
omits both nil and zero values,omitzero
only omits zero values, providing more precise control over JSON serialization.
@@ -52,7 +52,7 @@ type Embedded3 string | |||
// Person represents a person | |||
type Person struct { | |||
// Titles is optional in JSON | |||
Titles []Title `json:",omitempty"` | |||
Titles []Title `json:",omitzero"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify Go 1.24 compatibility in the build environment.
Since this change uses the new omitzero
tag introduced in Go 1.24, ensure the build environment is using Go 1.24 or later.
🏁 Script executed:
#!/bin/bash
# Description: Check Go version in the build environment
go version
Length of output: 44
Update the Go version in the build environment
The build environment is currently using Go version 1.23.3, but the new omitzero
tag (used in Titles []Title \
json:",omitzero"``) requires Go 1.24 or later. Please update the Go version to at least 1.24 to ensure compatibility and proper functionality.
Description
What it says on the tin.⚠️ MERGE ONLY AFTER SWITCHING TO GO 1.24
Type of change
How Has This Been Tested?
Ran binding generator tests.
Test Configuration
Checklist:
website/src/pages/changelog.mdx
with details of this PRI have made corresponding changes to the documentationSummary by CodeRabbit
GetButGenericAliases
introduced to facilitate advanced alias handling..Service14
and.Service15
added to the service list.Titles
field in thePerson
struct to useomitzero
for improved JSON output.