Skip to content
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

can't bind golang generic type #3715

Open
lysShub opened this issue Aug 29, 2024 · 11 comments
Open

can't bind golang generic type #3715

lysShub opened this issue Aug 29, 2024 · 11 comments
Labels
Bug Something isn't working

Comments

@lysShub
Copy link

lysShub commented Aug 29, 2024

Description

golang type:

image

bind ts type:
image

To Reproduce

none

Expected behaviour

bind correct

Screenshots

No response

Attempted Fixes

No response

System Details

# Wails
Version | v2.9.1

# System
┌─────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
| OS           | Windows 10 Pro                                                                                   |
| Version      | 2009 (Build: 19045)                                                                              |
| ID           | 22H2                                                                                             |
| Go Version   | go1.22.0                                                                                         |
| Platform     | windows                                                                                          |
| Architecture | amd64                                                                                            |
| CPU          | Intel(R) Xeon(R) CPU E5-1650 v4 @ 3.60GHz                                                        |
| GPU 1        | NVIDIA GeForce GTX 1060 3GB (NVIDIA) - Driver: 31.0.15.4629                                      |
| Memory       | 32GB                                                                                             |
└─────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘

# Dependencies
┌───────────────────────────────────────────────────────┐
| Dependency | Package Name | Status    | Version       |
| WebView2   | N/A          | Installed | 128.0.2739.42 |
| Nodejs     | N/A          | Installed | 20.16.0       |
| npm        | N/A          | Installed | 10.8.1        |
| *upx       | N/A          | Available |               |
| *nsis      | N/A          | Available |               |
└─────────────── * - Optional Dependency ───────────────┘

Additional context

No response

@lysShub lysShub added the Bug Something isn't working label Aug 29, 2024
@BigBallard
Copy link

I bump this heavily. I attempted to do the same thing with attempting to create a generic result object to bypass the lack of good error handling or multiple return type support in wails.

type Error struct {
	Message string `json:"message"`
}

type GenericResult[T any] struct {
	Data  *T     `json:"data,omitempty"`
	Error *Error `json:"error,omitempty"`
}
func (s *StudentController) GetStudent(id string) GenericResult[Student] {
        ...
}

Results in:

export function GetStudent(arg1:string):Promise<domain.GenericResult[freegrad/domain>;

Note that freegrad is the go module of my application and domain is the submodule to Student.

This kind of thing aligns with the enhancement request for error handling #3301. While Wails does not allow for returning multiple return types, we are forced into this single object result type to make things clean, or at least attempt to without having to create a unique return type for each function. However, without properly handling generics this way, we are forced to do just that instead of a single generic return type that can be used across the entire application.

Is this something that can be address in the v3 release? I would be willing to take a stab at it in that case but would like more feedback and discussion first.

@BigBallard
Copy link

It also seems that there is a dangling PR related to this issue that may be a fix #3626 ?

@leaanthony
Copy link
Member

There are 2 distinct things you are bringing up here:

  1. Lack of error returns in the API.

I personally believe that if you are trying to call methods on nil pointers it SHOULD crash. This is standard for Go. If that's the only thing you're guarding against then it creates a wildly cluttered API whose only usefulness is to guard you against yourself in whichever is the first method you call in the application. If it can error, and we don't bubble that back up, then sure. In general though, UI errors are catastrophic system errors and there's nothing you can do about them.

  1. Binding generic methods

We are totally open to supporting this and welcome anyone with the time and knowledge to do it. V2 uses a third party library which now supports this. It just needs someone to look at porting the new one over and adding in the Wails custom code, or refactoring to use it in a different way so that the module can be used directly (if possible). V3 uses a static analyser to understand your code and generate bindings. It also does not support generics at this point, but there is an if statement waiting to be filled in with the appropriate code. Perhaps @fbbdev can advise.

@BigBallard
Copy link

BigBallard commented Dec 3, 2024

On point one, I am not talking about calling method's on nil pointers or anything. Indeed this would be a crash and not something that should casually bubble up the frontend. I'm really just talking about exposing a go type error to the frontend, which really has to do more with returning more than one type from any backend API call. The ability to use generics with this case would help alleviate the overhead of creating a bunch of wrapper return types to expose an error on the backend. I can see this being an implementation decision for each project but the reality is that being limited to one return type is a bit of bottleneck.

I'll take a look at the code for v3 and see if I can figure out how generics can work in the stub you are mentioning.

@leaanthony
Copy link
Member

leaanthony commented Dec 3, 2024

When you say "only limited to one return type" what do you mean exactly? You have 2: a result and an error. Perhaps I'm misunderstanding what you're saying. The error is returned to the frontend via a promise rejection.

@BigBallard
Copy link

Well in that case it might be my misunderstanding. From what I gather, a golang error translates to an error that would be caught in the catch function of a promise?

@fbbdev
Copy link

fbbdev commented Dec 3, 2024

As regards v3, multiple return values and generics are already fully supported up to Go 1.23.

Some minor changes will be required to support generic aliases that should be coming with Go 1.24.

Return values of type error are elided from the returned tuple; if any one of these happens to be non-nil, the promise rejects. In case async/await is used, this results in an exception being thrown.

A bridge between JS and Go must necessarily be unidiomatic to some extent, as the two languages adopt incompatible idioms: the current approach favours JS-style error handling (hence either normal return values or exceptions, but not both).

Error values are marshaled somehow and passed on as the rejection value/exception. I don't recall the exact details and can't check the code right now. Anyways, all this will have to be detailed in v3 docs as we enter the beta phase.

@BigBallard
Copy link

BigBallard commented Dec 3, 2024

So regarding the error comment, this is in place now I presume? If so we should make sure to include these details in the documentation for at least v3 if not v2 as well because this detail is not currently present from what I could find.

Also what is the type of error supposed to be on the js side of things?

@fbbdev
Copy link

fbbdev commented Dec 3, 2024

I've had a look at the code, here's the details. I hope this clears things up a bit. Better documentation is definitely needed.

Wails v2

The binding engine supports either exactly one return value or exactly two return values where the 2nd one implements the error interface.

If an error formatting function is provided among application options (field options.App.ErrorFormatter), it is invoked to convert non-nil errors to arbitrary values which are then marshaled to JSON and used as promise rejection reasons.

If no error formatting function is provided, then non-nil errors are converted to strings by calling the standard Error() method, and the resulting string is used as promise rejection reason.

Wails v3

The binding engine supports arbitrarily many return values. Return values whose exact type is error are elided and treated as potential errors. The remaining ones, including those that implement the error interface but are of any other type, are used to form the promise resolution value. If there is just one, it is marhsaled to JSON directly. Otherwise, an array of return values is marshaled, in order of declaration.

If any one of the potential error values is non-nil, all of them are joined in order of declaration using errors.Join(...). The promise then rejects; the reason is a string of the form "Error calling method: <error>" where the placeholder <error> is replaced by the string obtained by calling the standard Error() method on the result of errors.Join(...).

Re: v3. We definitely can, and probably should, do better here @leaanthony.

When errors are returned from user code, as opposed to being raised internally by wails, we could try and marshal them to JSON directly instead of formatting them with the aforementioned template. If marshaling fails, we could then reject with the standard error string, without any additional decorations.

We should use errors.Join(...) only when there is actually more than one error.

Also, we could introduce a special exception class and name for internal errors, to make them easily distinguishable from user-defined API errors.

Thoughts?

@leaanthony
Copy link
Member

We had a v2 PR for a custom error handler for bindings which is an option.

@leaanthony
Copy link
Member

So regarding the error comment, this is in place now I presume? If so we should make sure to include these details in the documentation for at least v3 if not v2 as well because this detail is not currently present from what I could find.

It's covered here: https://wails.io/docs/howdoesitwork#calling-bound-go-methods

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants