[v3] Fix binding generator bugs and prepare for Go 1.24 #4045
Conversation
|
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 127 files out of 234 files are above the max files limit of 100. Please upgrade to Pro plan to get higher limits. You can disable this status message by setting the WalkthroughThis pull request updates the project documentation and codebase across multiple modules. It includes a comprehensive update to the changelog, dependency additions, and refactoring changes in the internal generator, binding, and rendering logic. Several methods and fields have been renamed or removed, and new functionality has been introduced in type handling, model predicate evaluation, and file generation. Additionally, test cases have been updated with new service types and alias modifications. Changes
Sequence Diagram(s)sequenceDiagram
participant G as Generator
participant C as Collector (PackageInfo)
participant R as Renderer
participant FS as File System
G->>C: Collect models & predicates data
C-->>G: Return PackageInfo and models list
G->>R: Invoke generateModels(info, models)
R->>FS: Write rendered models file
FS-->>R: Confirm file saved
R-->>G: Return success status
Possibly related PRs
Suggested reviewers
Poem
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (9)
v3/internal/generator/generate_test.go (1)
108-122: Improve warning logging implementation.Consider the following improvements:
- Handle potential errors from
log.Close()- Use a mutex to prevent race conditions in log file creation
- Write warnings directly to avoid memory overhead of sorting
Here's a suggested implementation:
if log, err := creator.Create("warnings.log"); err != nil { t.Error(err) } else { - func() { - defer log.Close() + var closeErr error + func() { + defer func() { + if err := log.Close(); err != nil { + closeErr = err + } + }() warnings := report.Warnings() - slices.Sort(warnings) + // Write warnings directly to avoid memory overhead + for _, msg := range warnings { + if _, err := fmt.Fprintln(log, msg); err != nil { + t.Error(err) + return + } + } - for _, msg := range warnings { - fmt.Fprintln(log, msg) - } }() + if closeErr != nil { + t.Error(closeErr) + } }v3/internal/generator/models.go (2)
24-24: Favor including root cause in error message.
Consider wrapping the original error (e.g.,err) to provide additional context, making debugging easier.
32-32: Duplicate error paths.
Line 32 logs a similar failure message as line 24. For clarity, consider distinguishing them or combining to avoid confusion if subsequent steps fail.v3/internal/generator/render/create.go (1)
176-179: Code duplication for skipping marshaler structs
Lines 176–179 repeat the same checks as lines 64–67. Consider extracting a helper function to avoid duplication and improve maintainability.v3/internal/generator/collect/predicates.go (1)
70-70: Minor doc comment typo.
The comment contains “un uninstantiated” which could be clarified as “an uninstantiated.”Apply the following diff to improve grammar:
-// instantiate instantiates typ if it is un uninstantiated generic type +// instantiate instantiates typ if it is an uninstantiated generic typev3/internal/generator/render/info.go (2)
9-26: Add field documentation to the modelInfo struct.While the struct has a high-level comment, adding documentation for individual fields would improve maintainability and make the code more self-documenting.
Add field documentation like this:
// modelInfo gathers useful information about a model. type modelInfo struct { + // IsEnum indicates whether the model represents an enumeration type IsEnum bool + // IsAlias indicates whether the model is a type alias IsAlias bool + // IsClassAlias indicates whether the model is a class-based alias IsClassAlias bool + // IsTypeAlias indicates whether the model is a pure type alias IsTypeAlias bool + // IsClassOrInterface indicates whether the model represents a class or interface type IsClassOrInterface bool + // IsInterface indicates whether the model represents an interface type IsInterface bool + // IsClass indicates whether the model represents a class type IsClass bool + // Template contains string representations for type parameters Template struct { + // Params contains the raw parameter names Params string + // ParamList contains the formatted parameter list with angle brackets ParamList string + // CreateList contains the parameter list for creation functions CreateList string } }
40-65: Consider using strings.Join for better performance.The current string building logic could be simplified and made more efficient using strings.Join for the parameter lists.
Consider refactoring to use strings.Join:
if len(model.TypeParams) > 0 { - var params, paramList, createList strings.Builder - - paramList.WriteRune('<') - createList.WriteRune('(') - - for i, param := range model.TypeParams { - if i > 0 { - params.WriteRune(',') - paramList.WriteString(", ") - createList.WriteString(", ") - } - params.WriteString(param) - paramList.WriteString(param) - - createList.WriteString("$$createParam") - createList.WriteString(param) - } - - paramList.WriteRune('>') - createList.WriteRune(')') - - info.Template.Params = params.String() - info.Template.ParamList = paramList.String() - info.Template.CreateList = createList.String() + params := model.TypeParams + createParams := make([]string, len(model.TypeParams)) + for i, param := range model.TypeParams { + createParams[i] = "$$createParam" + param + } + + info.Template.Params = strings.Join(params, ",") + info.Template.ParamList = "<" + strings.Join(params, ", ") + ">" + info.Template.CreateList = "(" + strings.Join(createParams, ", ") + ")" }v3/internal/generator/collect/type.go (1)
89-91: Improve error handling in Collect method.The current implementation provides a dummy group without proper error context. Consider enhancing error handling to provide more context about why the declaration couldn't be found.
Add more context to error handling:
// Provide dummy group. + // TODO: Consider adding more context about why the declaration couldn't be found + // and potential remediation steps. info.Decl = newGroupInfo(nil).Collect() returnv3/internal/generator/analyse.go (1)
72-76: Clean refactor of type parameter handling!The type assertion using interface is a more elegant solution than the previous type switch.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
v3/go.sumis excluded by!**/*.sumv3/internal/generator/testdata/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (43)
docs/src/content/docs/changelog.mdx(3 hunks)v3/go.mod(1 hunks)v3/internal/flags/bindings.go(0 hunks)v3/internal/generator/.gitignore(1 hunks)v3/internal/generator/Taskfile.yaml(1 hunks)v3/internal/generator/analyse.go(2 hunks)v3/internal/generator/collect/imports.go(5 hunks)v3/internal/generator/collect/index.go(1 hunks)v3/internal/generator/collect/model.go(5 hunks)v3/internal/generator/collect/predicates.go(1 hunks)v3/internal/generator/collect/properties.go(0 hunks)v3/internal/generator/collect/struct.go(2 hunks)v3/internal/generator/collect/type.go(1 hunks)v3/internal/generator/generate.go(1 hunks)v3/internal/generator/generate_test.go(4 hunks)v3/internal/generator/includes.go(0 hunks)v3/internal/generator/models.go(1 hunks)v3/internal/generator/render/create.go(6 hunks)v3/internal/generator/render/default.go(3 hunks)v3/internal/generator/render/functions.go(3 hunks)v3/internal/generator/render/info.go(1 hunks)v3/internal/generator/render/renderer.go(3 hunks)v3/internal/generator/render/templates.go(1 hunks)v3/internal/generator/render/templates/index.tmpl(2 hunks)v3/internal/generator/render/templates/models.js.tmpl(7 hunks)v3/internal/generator/render/templates/models.tmpl(0 hunks)v3/internal/generator/render/templates/models.ts.tmpl(7 hunks)v3/internal/generator/render/templates/service.js.tmpl(1 hunks)v3/internal/generator/render/templates/service.ts.tmpl(1 hunks)v3/internal/generator/render/type.go(5 hunks)v3/internal/generator/service.go(0 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/complex_instantiations/other/local.go(1 hunks)v3/internal/generator/testcases/map_keys/bound_types.json(1 hunks)v3/internal/generator/testcases/map_keys/main.go(1 hunks)v3/internal/generator/testcases/marshalers/bound_types.json(1 hunks)v3/internal/generator/testcases/marshalers/main.go(1 hunks)v3/internal/generator/testcases/no_bindings_here/person.go(1 hunks)v3/internal/generator/testcases/struct_literal_non_pointer_single/main.go(1 hunks)v3/internal/generator/testcases/struct_literal_single/main.go(1 hunks)v3/internal/generator/typedefs.go(0 hunks)
💤 Files with no reviewable changes (6)
- v3/internal/generator/service.go
- v3/internal/flags/bindings.go
- v3/internal/generator/includes.go
- v3/internal/generator/render/templates/models.tmpl
- v3/internal/generator/typedefs.go
- v3/internal/generator/collect/properties.go
✅ Files skipped from review due to trivial changes (3)
- v3/internal/generator/.gitignore
- v3/internal/generator/testcases/map_keys/bound_types.json
- v3/internal/generator/testcases/marshalers/bound_types.json
🧰 Additional context used
📓 Learnings (1)
v3/internal/generator/analyse.go (1)
Learnt from: fbbdev
PR: wailsapp/wails#4001
File: v3/internal/generator/analyse.go:140-206
Timestamp: 2025-01-15T22:32:41.113Z
Learning: The `FindServices` function in `analyse.go` correctly uses `iter.Seq[*types.TypeName]` as its return type to implement an iterator pattern. The sequence stops when either all items are processed or when the yield function returns false, allowing for early termination of the iteration.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run Go Tests (macos-latest, 1.23)
- GitHub Check: Run Go Tests (windows-latest, 1.23)
🔇 Additional comments (95)
v3/internal/generator/Taskfile.yaml (1)
10-10: Enhanced Cleanup Command:
The updated cleanup command now removes.got.logfiles in addition to.got.[jt]s, ensuring that all temporary log files generated during binding can be cleaned effectively. This change aligns perfectly with the PR objectives to improve artifact management prior to the Go 1.24 release.v3/internal/generator/testcases/complex_instantiations/other/local.go (1)
5-7: LGTM! Service type update is consistent.The change from Service14 to Service16 and corresponding LocalService update is clean and straightforward.
v3/internal/generator/testcases/complex_instantiations/main.go (2)
24-25: LGTM! New service types added.Addition of Service14 and Service15 types expands the test cases appropriately.
27-27: Verify if commented code should be enabled for Go 1.24.The commented out type alias and its usage appear to be related to Go 1.24 features. According to the PR objectives, these should remain commented out until the subsequent PR that transitions to Go 1.24.
Please confirm if this is intentional and will be uncommented in the follow-up PR for Go 1.24 transition.
Also applies to: 45-45
v3/internal/generator/testcases/complex_instantiations/bound_types.json (1)
15-15: LGTM! Service reference updated consistently.The update from Service14 to Service16 in bound_types.json matches the changes made in local.go.
v3/internal/generator/collect/index.go (1)
111-111: Verify the breaking change inIsEmptybehavior.The method now considers the index empty if there are no exported models, rather than checking for the absence of all models. This is a breaking change that could affect downstream code that relies on the previous behavior.
Let's verify the impact of this change:
Additionally, please verify:
- Are there any components that depend on detecting the presence of unexported models?
- Does this change align with the intended behavior for Go 1.24 support?
Consider documenting this breaking change in the changelog and updating relevant tests to ensure they account for the new behavior with unexported models.
v3/internal/generator/generate_test.go (3)
10-10: LGTM!The addition of the "slices" package is appropriate for the new warning logging feature.
92-98: LGTM! Good refactoring.Extracting the creator improves readability and makes it available for the new warning logging feature.
41-42: Add validation for filename configurations.Consider adding validation to ensure:
- Filenames are not empty
- File extensions don't conflict with the TS/JS options
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check if filename validation exists in the codebase # Search for filename validation in the generator package rg -A 5 'ModelsFilename|IndexFilename' --type goLength of output: 3811
Filename validation is already implemented in the generator.
The validations in
v3/internal/generator/generate.goalready ensure the following:
- Models and index filenames are not empty.
- Filenames do not include uppercase characters.
- The models and index filenames are not identical when an index file is expected.
Since these validations cover the configuration issues raised (and TS/JS filenames are expected to be provided without extensions as per the flag defaults), no additional validation is needed.
v3/internal/generator/models.go (3)
9-11: Well-documented function signature.
The added doc comments sufficiently clarify the prerequisites for callinggenerateModels(i.e.,info.Collectmust be invoked first).
12-19: Good approach to merging and pruning imports.
Merging import maps before clearing irrelevant imports is a clean way to ensure only the necessary imports are retained.
29-29: Renderer call aligns with the updated signature.
The direct pass ofimportsandmodelsis consistent withrenderer.Models. Looks good.v3/internal/generator/render/templates/index.tmpl (3)
2-2: Neat parameter extraction.
Storing.UseInterfacesin$useInterfacessimplifies references throughout the template.
32-52: Conditional export for objects is clear.
The new checks for$model.Object.Exportedand skipping non-exported objects help keep the generated index clean. However, note the immediatebreakat line 37 will stop processing subsequent models. Confirm that this is the intended behavior (rather than skipping only that model).
54-96: Logical separation of object exports and type exports.
Using two separate code paths for objects vs. type definitions is easy to follow. The toggling of$hasTypesand$hasObjectsensures that only the relevant declarations get exported. All changes look consistent with the reworked model generation process.v3/internal/generator/render/default.go (5)
49-52: Explicit handling of type parameters.
Panic for type parameters (i.e., unreachable scenario) makes the intention clear. This check helps surface unexpected usage early.
106-107: Refined marshaler checks.
ReplacingIsAny/IsStringAliaschecks withMaybeJSONMarshalerandMaybeTextMarshalerprovides more accurate detection of custom marshaler behavior.
123-125: Graceful fallback for text marshallers.
Returning""forMaybeTextMarshaleris sensible, as it aligns with typical textual encoding defaults.
133-141: Using$zerofield for model defaults.
This usage offers a clean approach to referencing a zero-value via each model’s$zeroproperty. Ensure that$zerois consistently generated in all relevant model declarations.
147-151: Clear distinction between JSON and text marshalers.
Returningnullfor JSON and""for text marshallers ensures each custom marshaling path is respected.v3/internal/generator/render/templates/models.ts.tmpl (12)
21-22: Initialize$infoand$templatefor model dataThese lines cleanly consolidate metadata into
$infoand$template, improving maintainability.
35-35: Enum detection logicThe use of
$info.IsEnumis straightforward and aligns with the updated enum handling approach.
60-65: Conditional block for class aliasesThis block neatly segregates handling for class aliases, ensuring clarity in the generated code.
78-81: Separate handling for alias, type alias, and class/interfaceThese conditions correctly differentiate various model types, enhancing clarity in the template.
93-93: Conditional check for class-based modelsThis logic properly distinguishes class-based models from interfaces or enums.
96-97: Constructor partial initializationAllowing a partial source object provides flexibility while maintaining a typed structure.
107-107: Doc generation constrained by template parametersGenerating documentation only when template parameters exist keeps the output concise and relevant.
115-115: RefinedcreateFromsignatureReferencing
$template.ParamListdirectly simplifies the creation function’s logic.
121-121: Optional closure for generic creationConditional closure usage nicely accommodates both generic and non-generic scenarios.
123-123: Leverage$module.JSCreateWithParamsfor field parsingCentralizing field creation logic ensures consistency and reduces code duplication.
129-130: Closure-based approach with typed parsingReturning an inner function when template parameters exist provides flexible instantiation patterns.
141-142: Final return expression for generic creationThis completes the typed instantiation process, returning a properly constructed model instance.
v3/internal/generator/render/templates/models.js.tmpl (14)
22-23: Introduce$infoand$templatefor model detail retrievalThis unification enhances readability and keeps all model-related checks centralized.
25-25: Extended condition for documentation or param listConsolidating these checks ensures doc comments are only generated when necessary.
35-35: Guard for template parameters on non-class aliasesThis condition helps accurately shape the JSDoc output and model references.
37-37: Enum detection blockThe
$info.IsEnumcheck smoothly handles enumeration-specific logic.
40-40: Type alias identificationClearly differentiates direct type aliases from other model types.
42-42: Interface definition checkCorrectly branches into the interface-specific rendering path.
60-60: Export of enumerated constantsEnumerating zero-value and named enum entries in one object is a concise approach.
86-90: Export class alias logicProperly references underlying type details to handle class alias generation seamlessly.
100-101: JSDoc for template parametersClearly documents template usage, aiding consumers of these generated definitions.
109-109: Constructor documentation for partial initializationInforms the user about partial construction and associated property defaults.
153-153: GenericcreateFrommethod definitionAccommodating typed parsing or fallback to an untyped approach is a flexible design.
155-155: Parsing field creation referencesEach field can be uniquely parsed using
$module.JSCreateWithParams, preserving type fidelity.
161-162: Closure approach for type parametersAutomates dynamic creation for models with generic parameters.
173-173: Final instantiation returning the constructed modelCompletes the typed creation pattern by building a new class instance with optional user data.
v3/internal/generator/render/type.go (4)
75-77: Pointer type handling for nullabilityPropagating the element type's nullability ensures correct representation for pointer types.
190-190: Unified marshaler checks for named typesCombining JSON and text marshaler checks prevents conflict and clarifies fallback logic.
192-192: Delegate torenderBasicTypefor non-marshaller basic aliasesDefers to a simpler path while skipping any specialized marshaling logic.
234-238: Return specialized types for custom marshaler structsReturning "any" or "string" based on marshaler capability properly represents the final serialized form.
v3/internal/generator/render/create.go (5)
10-10: Use of golang.org/x/tools/go/types/typeutil
No concerns: the new import is properly used for advanced type checks and analysis.
25-25: Enhanced cycle detection with typeutil.Map
Replacing the basic map withtypeutil.Mapcleanly handles visited logic and prevents recursion loops.
30-56: Implementation details for needsCreateImpl
The approach of setting the visited marker and unaliasing types is coherent, ensuring that cycles are handled correctly and special types (classes, string aliases, etc.) are detected.
64-67: Skipping code generation for empty or marshaler structs
These checks correctly short-circuit code generation when the struct is empty or is a possible marshaler, preventing unnecessary logic.
138-138: Early break when type isn’t flagged for creation
This immediate return helps avoid extra allocations for types that do not require creation.v3/internal/generator/collect/model.go (5)
42-48: Added Predicates field in ModelInfo
Caching type-check results in a dedicated field avoids repetitive predicate function calls and improves clarity.
60-71: New Predicates struct
Grouping marshaler-related flags and classification booleans in a single struct clarifies which properties are determined for each model.
136-166: Precomputing predicate data
Storing the precomputed marshaler checks and type properties is efficient, preventing repeated lookups during rendering.
168-222: Handling alias and named types
The revised handling of alias chains and named types, including checks against marshaler interfaces, is thorough and properly updates the underlying type.
323-324: Recording imports for struct fields
Including each field’s type in the import map ensures all necessary references are registered for generation.v3/internal/generator/testcases/map_keys/main.go (4)
1-108: Comprehensive definition of marshaler-related types and constraints
A solid demonstration of various marshaler interfaces and constraints being tested in the same package.
128-286: Rich Maps struct for verifying map key acceptance/rejection
The exhaustive map definitions effectively illustrate which constraint-based types and interfaces can serve as valid keys, aiding in thorough testing.
288-290: Service method returning a generic Maps instance
Returning an empty value is appropriate for demonstration. This confirms type resolution without extra complexity.
292-307: Wails application main entry
The application setup and error handling appear correct for a test scenario.v3/internal/generator/collect/predicates.go (2)
1-6: Initial package and comments look good.
No issues spotted with the package declaration and initial comments.
92-242: Overall logic withinisMarshaleris helpful, but verify heavy instantiation usage.
The function correctly addresses named vs. pointer receiver checks, generic types, and cycle detection. However, instantiating generic types on every call can be expensive. If performance becomes an issue, consider introducing a localized cache at call sites to avoid repeated instantiations.Would you like assistance to create a small caching mechanism or test for performance regressions?
v3/internal/generator/testcases/no_bindings_here/person.go (2)
13-13: Alias usage forImpersonatoris valid.
Confirm that any consumers expecting a distinct type (instead of an alias) are updated accordingly. Otherwise, calls or reflections on type identity may differ.
20-20: Alias usage forPrivatePersonis valid.
Ensure that references toPrivatePersonno longer rely on differences frompersonImpl.v3/internal/generator/render/templates.go (1)
21-23: Template mapping renamed totmplModels.
This is a straightforward rename fromtmplTypedefstotmplModels. Ensure all references to the old variable name are removed or updated.v3/internal/generator/render/functions.go (2)
16-31: Document the removal of isclass function.The removal of the isclass function might impact existing code. Consider adding a comment explaining why it was removed and how to handle its use cases.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for potential usages of the removed isclass function rg "isclass" --type goLength of output: 22
Removal of
isclassfunction verified and documentation recommended
- The repository search for “isclass” showed no remaining references, indicating that its removal did not leave any active usage.
- Nonetheless, it's best practice to add a comment (either alongside the function mapping or within the commit message) explaining why
isclasswas removed and advising on how to address any legacy use cases if needed.
99-107: Add test cases for istpalias function.The new istpalias function is crucial for type parameter handling but lacks test coverage. Consider adding comprehensive test cases to verify its behavior with different types.
v3/internal/generator/render/templates/service.ts.tmpl (1)
34-34: Verify the impact of models import path change.The change in import path from internal to models file needs verification to ensure it doesn't break existing functionality.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for other templates that might be affected by this change rg -l "import.*\$models" --type ts --type jsLength of output: 45
🏁 Script executed:
#!/bin/bash # Print a few lines around line 34 of the service.ts.tmpl file # to verify that the models import path is as expected. sed -n '30,38p' v3/internal/generator/render/templates/service.ts.tmplLength of output: 312
🏁 Script executed:
#!/bin/bash # Search for references to the templated models import in the repository rg "import \* as \$models" -nLength of output: 30
🏁 Script executed:
#!/bin/bash # Search for the template placeholder for models import in the repository to verify its usage. rg "{{js \$models}}" -nLength of output: 114
Impact of the models import path change appears minimal.
Our investigation shows that:
- The import statement
import * as $models from "./{{js $models}}";is conditionally included based on$imports.ImportModelsand is isolated to the template file.- Repository-wide searches for references using
$modelsor the template placeholder did not reveal any additional usage that could be impacted.No adverse effects have been detected based on our review of the template and repository references. It is recommended, however, to manually verify the integration of the generated files in the consuming parts of the project.
v3/internal/generator/render/templates/service.js.tmpl (1)
35-35: LGTM! Import statement updated correctly.The change from importing models from internal to importing directly from models aligns with the PR objective of removing internal.js model files.
v3/internal/generator/testcases/aliases/main.go (3)
7-7: LGTM! Import added for foreign private alias support.The import is correctly added to support the new GetButForeignPrivateAlias method.
104-106: LGTM! Method added for testing foreign private alias bindings.The method is correctly implemented to test binding generation for private types from external packages.
56-87: Verify Go 1.24 compatibility for generic alias definitions.The commented-out generic alias definitions are prepared for Go 1.24 support. These will need to be uncommented once Go 1.24 is released.
✅ Verification successful
🌐 Web query:
What is the current status of generic alias support in Go 1.24?Length of output: 5092
Generic alias definitions are now fully supported in Go 1.24.
The web query confirms that Go 1.24 brings full support for generic type aliases, so the commented-out code inv3/internal/generator/testcases/aliases/main.gois ready to be uncommented once the release is in effect.
- The generic alias definitions will work without requiring experimental flags.
- Full backward compatibility and cross-package support are ensured.
v3/internal/generator/render/renderer.go (2)
41-41: LGTM! Template reference updated to use models template.The change from tmplTypedefs to tmplModels aligns with the broader refactoring of model handling.
79-79: LGTM! Method renamed and updated to use new predicates system.The changes improve code organization by:
- Renaming Typedefs to Models for better clarity
- Using the new Predicates.IsClass property for type checking
Also applies to: 89-89
v3/internal/generator/testcases/struct_literal_non_pointer_single/main.go (1)
177-178: LGTM! Method updated to test pointer value maps instead of pointer key maps.The change from map[*int]int to map[int]*int improves test coverage for binding generation with pointer values in maps.
v3/internal/generator/testcases/struct_literal_single/main.go (1)
177-178: LGTM! The method signature change improves clarity.The change from
map[*int]inttomap[int]*intmakes more sense semantically, as maps typically use non-pointer types as keys. This aligns with the PR's objective to fix binding generator bugs.v3/internal/generator/testcases/marshalers/main.go (5)
12-72: Well-structured test cases for basic marshaler types!The code provides comprehensive coverage of marshaler implementations:
- Value vs pointer receivers
- JSON vs Text marshaling
- Combined JSON+Text marshaling
- Interface definitions for custom marshalers
74-87: Good coverage of alias type scenarios!The test cases for alias types ensure proper handling of:
- Non-marshaler aliases
- JSON marshaler aliases
- Text marshaler aliases
- Combined marshaler aliases
89-123: Thorough testing of implicit implementations!The test cases cover various implicit implementation scenarios:
- Basic implicit implementations
- Overridden implementations
- Mixed implementations (e.g., JSON but Text, Text but JSON)
124-188: Comprehensive Data struct combining all test cases!The
Datastruct effectively aggregates all test scenarios:
- Both value and pointer fields for each type
- Clear comments indicating expected TypeScript types
- Logical grouping of related fields
190-209: Simple and effective service implementation!The service implementation and main function provide a minimal but sufficient setup to test the marshaler bindings in a real application context.
v3/internal/generator/analyse.go (1)
92-93: Improved method processing logic!The simplified condition for processing named type methods is clearer and more direct.
v3/internal/generator/generate.go (1)
212-212: LGTM! Simplified model generation.The removal of the typedef generation step reduces complexity while maintaining core functionality.
v3/internal/generator/collect/imports.go (4)
6-7: Good improvement in type cycle detection!Using
typeutil.Mapinstead of a regular map for tracking visited types is a better approach as it's specifically designed for handling Go types and provides proper type identity comparison.Also applies to: 124-125
130-168: Well-structured improvements to type parameter handling!The changes enhance the handling of generic types and provide better error messaging. The use of type assertions with interfaces is clean and the error message is more precise.
191-198: Enhanced error reporting for unsupported types!The warning messages now include the package path and provide clearer information about unsupported types, which will help developers identify and fix issues more easily.
209-228: Improved map key type handling!The changes provide better support for string aliases and type parameters as map keys, with appropriate warnings for potentially invalid cases. The warning messages are clear and helpful.
v3/internal/generator/collect/struct.go (2)
196-202: Good handling of parametric fields!Marking fields with type parameters as optional in class mode is a safe approach since their default JS values cannot be determined in advance.
328-329: Good addition of 'omitzero' JSON flag support!The implementation cleanly adds support for the 'omitzero' flag alongside 'omitempty', maintaining consistency with JSON serialization practices.
v3/go.mod (1)
131-131: Verify the dependency version.The specified version of
golang.org/x/exp/typeparamsis from 2025. Please verify if this is the correct version to use.✅ Verification successful
🌐 Web query:
What is the latest version of golang.org/x/exp/typeparams?Length of output: 4903
Verify the experimental dependency version in go.mod
Thegolang.org/x/exp/typeparamspackage is experimental and is not released with a stable tag. Instead, it’s referenced via a pseudo-version derived from a specific commit. Although the web query indicates a placeholder pseudo-version (v0.0.0-19700101000000-a9213eeb770e) due to the absence of tagged releases, your go.mod file specifies v0.0.0-20250128182459-e0ece0dbea4c—which reflects a commit timestamp from January 28, 2025. This approach is expected with experimental modules. Just ensure that this commit is the one intended for your project.docs/src/content/docs/changelog.mdx (1)
48-50: Well-documented changes in changelog!The changelog clearly documents all the binding generator improvements, including:
- Support for generic aliases
- Addition of the omitzero JSON flag
- Fixes for type-related issues and map key handling
Also applies to: 65-70
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
v3/internal/generator/render/templates/index.tmpl (1)
75-88: Consider refactoring documentation generation to reduce duplication.The documentation generation logic could be made more maintainable by extracting it into a reusable template partial.
Consider creating a partial template for documentation:
{{- define "modelDocumentation" -}} /** {{- if hasdoc .Decl.Doc}} {{- jsdoc .Decl.Doc.Text ""}}{{if hasdoc .Doc}} *{{end}} {{- end}} {{- if hasdoc .Doc}} {{- jsdoc .Doc.Text ""}} {{- end}} {{- if .Template.ParamList}} * @template {{.Template.Params}} {{- end}} * @typedef {$models.{{jsid .Name}}{{.Template.ParamList -}} } {{jsid .Name}} */ {{- end -}}Then use it like:
-/** -{{- if hasdoc $model.Decl.Doc}} -{{- jsdoc $model.Decl.Doc.Text ""}}{{if hasdoc $model.Doc}} - *{{end}} -{{- end}} -{{- if hasdoc $model.Doc}} -{{- jsdoc $model.Doc.Text ""}} -{{- end}} -{{- if $template.ParamList}} - * @template {{$template.Params}} -{{- end}} - * @typedef {$models.{{jsid $model.Name}}{{$template.ParamList -}} } {{jsid $model.Name}} - */ +{{template "modelDocumentation" $model}}v3/internal/generator/testcases/directives/js/test_all.js (1)
1-3: LGTM! Consider documenting the test coverage pattern.The implementation is correct. Since this appears to be part of a comprehensive test suite where CustomMethod is called with different arguments across multiple files ("everywhere", "everywhere again", "Classes", etc.), consider adding a comment explaining the test coverage pattern and how this file fits into the overall testing strategy.
import { CustomMethod } from "./service.js"; +// Part of comprehensive test suite that verifies CustomMethod behavior +// across different contexts (JS/TS, Classes/Interfaces) CustomMethod("everywhere again");v3/internal/generator/testcases/directives/main.go (1)
36-51: Consider adding window options for better UX.The webview window is created without any options. Consider adding basic window options like title, size, etc., for a better user experience.
- app.NewWebviewWindow() + app.NewWebviewWindow(application.WebviewWindowOptions{ + Title: "Test Application", + Width: 800, + Height: 600, + })
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
v3/internal/generator/render/templates/index.tmpl(2 hunks)v3/internal/generator/testcases/directives/bound_types.json(1 hunks)v3/internal/generator/testcases/directives/includes.go(1 hunks)v3/internal/generator/testcases/directives/js/test.js(1 hunks)v3/internal/generator/testcases/directives/js/test_all.js(1 hunks)v3/internal/generator/testcases/directives/js/test_c.js(1 hunks)v3/internal/generator/testcases/directives/js/test_i.js(1 hunks)v3/internal/generator/testcases/directives/js/test_j.js(1 hunks)v3/internal/generator/testcases/directives/js/test_jc.js(1 hunks)v3/internal/generator/testcases/directives/js/test_ji.js(1 hunks)v3/internal/generator/testcases/directives/js/test_t.ts(1 hunks)v3/internal/generator/testcases/directives/js/test_tc.ts(1 hunks)v3/internal/generator/testcases/directives/js/test_ti.ts(1 hunks)v3/internal/generator/testcases/directives/main.go(1 hunks)v3/internal/generator/testcases/directives/otherpackage/dummy.go(1 hunks)v3/internal/generator/testcases/directives/otherpackage/js/test_j.js(1 hunks)v3/internal/generator/testcases/directives/otherpackage/js/test_jc.js(1 hunks)v3/internal/generator/testcases/directives/otherpackage/js/test_t.ts(1 hunks)v3/internal/generator/testcases/directives/otherpackage/js/test_tc.ts(1 hunks)v3/internal/generator/testdata/tsconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- v3/internal/generator/testcases/directives/js/test_jc.js
- v3/internal/generator/testcases/directives/bound_types.json
- v3/internal/generator/testcases/directives/otherpackage/dummy.go
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run Go Tests (macos-latest, 1.23)
- GitHub Check: Run Go Tests (windows-latest, 1.23)
🔇 Additional comments (20)
v3/internal/generator/testdata/tsconfig.json (1)
8-8: ExpandedlibConfiguration: Including "DOM" in the TypeScript Compilation.Adding
"DOM"alongside"ES6"extends the available type definitions to include DOM-related APIs. This is appropriate if the generated bindings or test cases involve browser or DOM-specific functionality. Please verify that this inclusion is intentional and aligned with the project’s target environments.v3/internal/generator/render/templates/index.tmpl (2)
2-2: LGTM! Clear and well-structured variable declarations.The new variables are well-named and properly initialized to support the enhanced export logic.
Also applies to: 33-34
39-39: Add error handling for modelinfo helper.The modelinfo helper is called without error handling. Consider adding error checking to handle potential failures gracefully.
Could you verify if the modelinfo helper has built-in error handling or if it needs to be added here?
v3/internal/generator/testcases/directives/otherpackage/js/test_jc.js (2)
3-3: LGTM! Test case for JavaScript classes.The invocation of
CustomMethodwith "JS Classes" is consistent with the test case pattern described in the summary, specifically targeting JavaScript class functionality.
1-1: Verify the imported module exists.The import statement looks correct syntactically. Let's verify that the referenced module exists and exports the
CustomMethod.v3/internal/generator/testcases/directives/js/test_j.js (1)
1-3: LGTM!The import path and usage of
CustomMethodare correct and consistent with the file's purpose as a JavaScript test case.v3/internal/generator/testcases/directives/js/test_t.ts (1)
1-3: LGTM!The import path and usage of
CustomMethodare correct. The JavaScript import in a TypeScript file is a valid pattern.v3/internal/generator/testcases/directives/otherpackage/js/test_j.js (1)
1-3: LGTM!The import path correctly references the parent directory's service.js, and the usage is consistent with other test files.
v3/internal/generator/testcases/directives/js/test_c.js (1)
1-3: LGTM! Clear and focused test case for class directives.The test appropriately imports and uses CustomMethod to verify binding generation for class-related scenarios.
v3/internal/generator/testcases/directives/js/test.js (1)
1-3: LGTM! Base test case for general binding behavior.The test serves as a baseline verification for binding generation with the "everywhere" directive.
v3/internal/generator/testcases/directives/js/test_i.js (1)
1-3: LGTM! Interface-specific test case.The test appropriately verifies binding generation for interface scenarios, which is particularly relevant for the Go 1.24 support mentioned in the PR objectives.
v3/internal/generator/testcases/directives/js/test_tc.ts (2)
1-3: LGTM! TypeScript class-specific test case.The test extends binding generation verification to TypeScript classes, ensuring comprehensive coverage across both JavaScript and TypeScript.
1-4: Verify test coverage completeness.The test suite demonstrates good coverage across different scenarios (Classes, Interfaces, TypeScript). Consider adding tests for:
- Error cases (invalid directives)
- Edge cases in type conversions
- New Go 1.24 features (when enabled)
v3/internal/generator/testcases/directives/otherpackage/js/test_tc.ts (1)
1-3: LGTM! Implementation follows TypeScript test case conventions.The import path and usage of CustomMethod are correctly implemented for testing TypeScript classes from a different package.
v3/internal/generator/testcases/directives/js/test_ti.ts (1)
1-3: LGTM! Implementation follows TypeScript test case conventions.The import path and usage of CustomMethod are correctly implemented for testing TypeScript interfaces.
v3/internal/generator/testcases/directives/js/test_ji.js (1)
1-3: LGTM! Implementation follows JavaScript test case conventions.The import path and usage of CustomMethod are correctly implemented for testing JavaScript interfaces.
v3/internal/generator/testcases/directives/main.go (4)
1-10: LGTM! Console logging directives are well-organized.The inject directives follow a consistent pattern and provide good coverage for both JavaScript and TypeScript environments.
21-28: LGTM! Well-documented and type-safe implementation.The exported function is properly documented with JSDoc for JavaScript and includes TypeScript type annotations. The implementation correctly handles async/await and string concatenation.
29-34: LGTM! Service implementation follows wails patterns.The Service struct correctly implements both visible and internal methods using the wails directive pattern.
13-19: Consider removing unused embed package import.The
embedpackage is imported but not used in the code. Consider removing it if it's no longer needed.Let's check if the embed package is used elsewhere in the codebase:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check for embed package usage in the codebase. # Search for embed package usage in Go files rg --type go "//go:embed|_ \"embed\""Length of output: 8724
Unused embed import in v3/internal/generator/testcases/directives/main.go should be removed.
- The file imports
_ "embed"but, as verified by the absence of any//go:embeddirectives in this file, it appears that the embed package is not actually used.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
v3/internal/generator/collect/index.go (1)
117-121: Consider renaming IsEmpty or updating its documentation.The method name "IsEmpty" suggests checking for the presence of any entities, but the implementation only checks for exported entities. This could be confusing for callers. Consider either:
- Renaming to
HasNoExportedEntities()to better reflect its behavior, or- Updating the documentation to clearly state that it only considers exported entities.
// IsEmpty returns true if the given index -// contains no data for the selected language. +// contains no exported services, no exported models, +// and no injections for the selected language. func (index *PackageIndex) IsEmpty() bool { return !index.HasExportedServices && !index.HasExportedModels && len(index.Package.Injections) == 0 }v3/internal/generator/render/templates/models.ts.tmpl (1)
108-174: Consider improving type safety in the constructor.While the implementation correctly handles both classes and interfaces with proper generic type support, the constructor's type safety could be improved.
Consider this improvement for the constructor:
- constructor($$source: Partial<{{jsid $model.Name}}{{$template.ParamList}}> = {}) { + constructor($$source: Readonly<Partial<{{jsid $model.Name}}{{$template.ParamList}}>> = {}) {This change:
- Prevents accidental mutation of the source object
- Better aligns with TypeScript best practices for immutable inputs
v3/internal/generator/testcases/enum/main.go (1)
25-33: LGTM! Well-documented constants with clear values.The age constants are well-organized and include descriptive comments where needed.
Consider adding a comment for
NewBorn,Teenager, andYoungAdultconstants for consistency with other constants.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
docs/src/content/docs/changelog.mdx(3 hunks)v3/internal/generator/collect/index.go(4 hunks)v3/internal/generator/collect/model.go(7 hunks)v3/internal/generator/render/info.go(1 hunks)v3/internal/generator/render/templates/index.tmpl(2 hunks)v3/internal/generator/render/templates/models.js.tmpl(7 hunks)v3/internal/generator/render/templates/models.ts.tmpl(7 hunks)v3/internal/generator/render/templates/service.js.tmpl(2 hunks)v3/internal/generator/render/templates/service.ts.tmpl(2 hunks)v3/internal/generator/testcases/directives/internal.go(1 hunks)v3/internal/generator/testcases/directives/main.go(1 hunks)v3/internal/generator/testcases/directives/unexported.go(1 hunks)v3/internal/generator/testcases/enum/main.go(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- v3/internal/generator/testcases/directives/unexported.go
🚧 Files skipped from review as they are similar to previous changes (4)
- v3/internal/generator/render/templates/service.ts.tmpl
- v3/internal/generator/render/info.go
- v3/internal/generator/render/templates/service.js.tmpl
- v3/internal/generator/testcases/directives/main.go
🧰 Additional context used
📓 Learnings (2)
v3/internal/generator/render/templates/index.tmpl (7)
Learnt from: fbbdev
PR: wailsapp/wails#4045
File: v3/internal/generator/render/templates/index.tmpl:37-37
Timestamp: 2025-02-05T11:53:01.219Z
Learning: The template `v3/internal/generator/render/templates/index.tmpl` uses `break` for optimization when encountering non-exported models, though the underlying sorting contract needs verification in the codebase.
Learnt from: fbbdev
PR: wailsapp/wails#4045
File: v3/internal/generator/render/templates/index.tmpl:64-64
Timestamp: 2025-02-05T11:53:10.068Z
Learning: In the collect.Index type, models are sorted with exported models first, followed by non-exported models. This allows for quick iteration termination using `break` when encountering the first non-exported model.
Learnt from: fbbdev
PR: wailsapp/wails#4045
File: v3/internal/generator/render/templates/index.tmpl:64-64
Timestamp: 2025-02-05T11:53:10.068Z
Learning: The PackageIndex type in v3/internal/generator/collect/index.go guarantees that "exported models precede all unexported ones and both ranges are sorted by name". This is enforced by the Index method's sorting implementation, making it optimal to use `break` instead of `continue` when encountering the first non-exported model in templates.
Learnt from: fbbdev
PR: wailsapp/wails#4045
File: v3/internal/generator/render/templates/index.tmpl:37-37
Timestamp: 2025-02-05T11:53:01.219Z
Learning: The template `v3/internal/generator/render/templates/index.tmpl` uses `break` when encountering the first non-exported model, which relies on a contract that models are sorted with exported ones first.
Learnt from: fbbdev
PR: wailsapp/wails#4045
File: v3/internal/generator/render/templates/index.tmpl:37-37
Timestamp: 2025-02-05T11:53:01.219Z
Learning: In the `collect.PackageIndex` type, models are sorted with exported models appearing first, followed by non-exported models. This contract allows for optimization in templates by breaking iteration early when the first non-exported model is encountered.
Learnt from: fbbdev
PR: wailsapp/wails#4045
File: v3/internal/generator/render/templates/index.tmpl:37-37
Timestamp: 2025-02-05T11:53:01.219Z
Learning: The `PackageIndex` type in `v3/internal/generator/collect/index.go` guarantees that exported models precede all unexported ones in the Models slice, as documented in the type's comment and enforced by the `Index` method's sorting function. This contract enables the optimization in `index.tmpl` where iteration can break early upon encountering the first non-exported model.
Learnt from: fbbdev
PR: wailsapp/wails#4045
File: v3/internal/generator/render/templates/index.tmpl:37-37
Timestamp: 2025-02-05T11:53:01.219Z
Learning: The template `v3/internal/generator/render/templates/index.tmpl` relies on models being sorted with exported ones first, which enables breaking early when encountering non-exported models. This contract needs to be verified and documented in the codebase.
v3/internal/generator/collect/index.go (1)
Learnt from: fbbdev
PR: wailsapp/wails#4045
File: v3/internal/generator/render/templates/index.tmpl:37-37
Timestamp: 2025-02-05T11:53:01.219Z
Learning: The `PackageIndex` type in `v3/internal/generator/collect/index.go` guarantees that exported models precede all unexported ones in the Models slice, as documented in the type's comment and enforced by the `Index` method's sorting function. This contract enables the optimization in `index.tmpl` where iteration can break early upon encountering the first non-exported model.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run Go Tests (macos-latest, 1.23)
- GitHub Check: Run Go Tests (windows-latest, 1.23)
🔇 Additional comments (26)
v3/internal/generator/collect/index.go (2)
13-14: LGTM! Well-documented struct changes.The new boolean fields and updated documentation clearly communicate the sorting behavior and presence of exported entities.
Also applies to: 19-19, 22-22
51-54: LGTM! Efficient sorting implementation.The sorting logic ensures exported entities precede internal ones, which enables template optimizations by allowing early breaks when processing exported entities. The implementation is consistent across services and models, with proper handling of edge cases.
Also applies to: 61-76, 94-109
v3/internal/generator/render/templates/models.ts.tmpl (4)
21-22: LGTM! Good refactoring of model information.The introduction of
$infoand$templatevariables consolidates model information and template parameters, making the code more maintainable and less prone to bugs.
35-60: LGTM! Clean enum type generation.The use of
$info.IsEnumsimplifies the condition check while maintaining proper TypeScript enum generation with zero value handling.
60-78: LGTM! Improved handling of class aliases and generics.The implementation correctly handles both type parameter aliases and regular aliases, with proper support for generic types through
$template.ParamList.
79-107: LGTM! Well-structured type alias generation with namespace support.The implementation properly handles type aliases with predefined values, generating a namespace object when needed. The use of
$template.ParamListensures proper generic type support.v3/internal/generator/testcases/directives/internal.go (3)
3-8: LGTM! Well-documented internal model.The internal model is clearly documented with the
//wails:internaldirective, making its purpose explicit.
10-13: LGTM! Well-documented internal service.The internal service is clearly documented with the
//wails:internaldirective, making its purpose explicit.
15-15: LGTM! Clean method implementation.The method signature is clear and follows Go conventions.
v3/internal/generator/testcases/enum/main.go (2)
22-24: LGTM! Well-defined type alias with clear documentation.The
Agetype alias enhances code readability and type safety.
46-46: LGTM! Clean field addition.The
Agefield is appropriately added to thePersonstruct.v3/internal/generator/render/templates/index.tmpl (4)
22-22: LGTM! Optimal handling of internal services.The use of
breakwhen encountering internal services is correct and efficient, as it stops processing once internal services are reached.Also applies to: 27-27
34-36: LGTM! Clear tracking variables.The introduction of
$hasObjectsand$hasTypesimproves code organization by clearly tracking the presence of different entity types.
37-51: LGTM! Well-structured model export logic.The model export logic is well-organized and handles different cases appropriately:
- Checks for internal models
- Processes model information
- Handles different model types (values, class aliases, classes)
64-92: LGTM! Comprehensive type export handling.The type export logic is thorough and handles:
- Internal model checks
- Template parameters
- Documentation
- TypeScript vs JavaScript differences
v3/internal/generator/render/templates/models.js.tmpl (3)
22-23: LGTM! Clean model information handling.The use of
modelinfofunction consolidates model-related information, improving code organization.
25-59: LGTM! Comprehensive documentation generation.The documentation generation is thorough and handles:
- Model documentation
- Template parameters
- Type-specific documentation (enum, type alias, interface)
162-186: LGTM! Well-structured creation function.The
createFromstatic method is well-implemented with:
- Proper type parameter handling
- Source parsing
- Field creation
v3/internal/generator/collect/model.go (6)
21-53: LGTM! Good performance optimization and export control.The changes to
ModelInfostruct are well thought out:
- The
Internalfield provides explicit control over model export behavior- The
Predicatesfield caches type predicate results, avoiding repeated evaluations
65-76: LGTM! Well-structured predicate caching.The
Predicatesstruct efficiently caches commonly checked type properties, reducing the need for repeated evaluations.
141-154: LGTM! Good export control implementation.The implementation correctly handles model export status:
- Initializes
Internalbased on the object's export status- Parses directives to allow manual control via the "internal" directive
156-168: LGTM! Improved generic type handling.The type parameter handling is now more robust:
- Properly checks for generic types
- Correctly records type parameter names
170-184: LGTM! Excellent performance optimization.The predicate precomputation is a significant performance improvement:
- Uses instantiated type to avoid repeated instantiations
- Caches all commonly used predicates
188-260: LGTM! Comprehensive type handling.The type handling is now more robust:
- Properly handles alias types without skipping chains
- Correctly handles named types and their underlying types
- Properly detects and handles marshaler interfaces
docs/src/content/docs/changelog.mdx (2)
48-53: LGTM! Well-documented changes.The changelog entries for the binding generator improvements are clear and comprehensive:
- Added package path reporting in warnings
- Added support for generic aliases and omitzero JSON flag
- Added new directives for controlling binding generation
68-76: LGTM! Clear documentation of fixes.The changelog properly documents all the fixes made to the binding generator:
- Fixed undefined behavior with generic types
- Fixed output for models with mismatched properties
- Fixed map key types and marshaler interface handling
- Fixed type cycle detection and invalid references
|
|
||
| // Oh no, some grey hair! | ||
| MiddleAged Age = 50 | ||
| Mathusalem Age = 1000 // Unbelievable! |
|
@fbbdev - Looks like the generator tests are failing |
|
@leaanthony expected, I did not push testdata yet, it would make the PR impossible to review After you approve code changes I'll push testdata and take care of PR checks |
|
I missed that bit heh |
…rep' into v3-alpha-bugfix/bindgen-go1.24-prep
|
Merged testdata |
|
There are still errors, fixing |
|
Pushed a fix, hopefully will be enough |
# Conflicts: # docs/src/content/docs/changelog.mdx
Description
[NOTE: to make the patch more accessible, example and test data updates have been omitted. The full patch is available at
fbbdev/wails:v3-alpha-bugfix/bindgen-go1.24-prep-testdataand will be pushed right before merging. Until then, automated tests cannot pass.]This PR provides extensive bug fixes for the binding generator, mostly related to the handling of maps, aliases and generic types. Apart from ensuring correct code generation in certain corner cases, these fixes are required in order to support generic aliases, a new language feature that's coming with Go 1.24 (scheduled for release in February 2025).
The PR comprises four distinct sections, for easier reviewing:
47c22d8..dff23437d21033..c39d9907c1d8ed..b11c4f9b5f7396..b448e02(this one's a bit of a mixed bag, I extended the scope of a couple directives and added weakly typed enums)The actual specific changes that enable Go 1.24 support are minimal and version-agnostic, i.e. they can be merged right now with no negative consequence.
Tests for Go 1.24 features have been commented out. As soon as this PR is merged, I'll open a new one that re-enables them, to be merged as soon as Wails moves to Go 1.24. A preview of the final patch for Go 1.24 is available at
fbbdev/wails:v3-alpha-bugfix/bindgen-go1.24.Changelog
🚨 Breaking changes 🚨
internal.jsmodel files have been removed, all models can be found inmodels.jsAdded
omitzeroJSON flagFixed
Type of change
Please select the option that is relevant.
How Has This Been Tested?
Thorough regression tests have been added for all fixed issues. The test suite passes consistently under Go 1.23.5 and 1.24rc2. Binding and service examples have been updated and tested manually.
Test Configuration
Checklist:
docs/src/content/docs/changelog.mdxwith details of this PRSummary by CodeRabbit
Summary by CodeRabbit
-internalflag has been removed from thegenerate bindingscommand.