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

A lot of different bugs while generating bindings #2303

Open
FrancescoMerenda opened this issue Jan 15, 2023 · 23 comments
Open

A lot of different bugs while generating bindings #2303

FrancescoMerenda opened this issue Jan 15, 2023 · 23 comments
Labels
Bug Something isn't working

Comments

@FrancescoMerenda
Copy link

Description

The binding generator does not properly handle maps, type aliases and other packages types:

In my codebase I have a file where I save all the types used in the cross comunications so my binded methods often uses external packages types.
This are some of the types inside intypes.go

type Library struct {
	Comics  ComicsMap `json:"Comics"`
	Version string `json:"Version"`
}
type ComicsMap map[string]Comic
type Comic struct{
	Title string `json:"Title"`
	Slug string `json:"Slug"`
}

And this is the method I want to bind inside app.go

func (a *App) GetLibrary() map[string]intypes.Comics {
	\\ my code
}

If I try to generate the file I get a non existent and wrong type:

export function GetLibrary():Promise<map[string]intypes.Comic>;

To Reproduce

  1. Define a struct in a separated package
  2. Bind a method that returns a map which the value is a type of another package
  3. Generate bindings with either wails dev or wails generate module

Expected behaviour

Generate the bound methods with the proper return type

export function GetLibrary():Promise<{[key: string]: intypes.Comic}>;

Screenshots

No response

Attempted Fixes

While trying to solve this problem I discovered even more problems.

If I try to return the type alias ComicsMap Like this:

func (a *App) GetLibrary() intypes.ComicsMap {
	\\ my code
}

It generate it likes this:

export function GetLibrary():Promise<intypes.ComicsMap>;

Which is correct but it break the import like this:

import {map[string]intypes} from '../models';

and the generated models.ts doesn't have the ComicsMap type inside the intypes namespace.

After this I then tried to create a similar type inside the app.go file (so it simply was map[string]Comic) and did everything as expected confirming that the problem was the different package.
This of course did not solve the problem so I tried returning the Library struct above directly and it worked as expected.

So basically no type aliases, no external packages maps. Bound methods can't return an external package struct if not inside a given struct

System Details

# System

OS           | Windows 10 Pro
Version      | 2009 (Build: 19044)
ID           | 21H2
Go Version   | go1.19.4
Platform     | windows
Architecture | amd64

# Wails

Version | v2.3.1

# Dependencies

Dependency | Package Name | Status    | Version
WebView2   | N/A          | Installed | 108.0.1462.76
npm        | N/A          | Installed | 8.15.0
*upx       | N/A          | Available |
*nsis      | N/A          | Available |
* - Optional Dependency

# Diagnosis

Your system is ready for Wails development!
Optional package(s) installation details:
  - upx : Available at https://upx.github.io/
  - nsis : More info at https://wails.io/docs/guides/windows-installer/

Additional context

Aside from the bug I would also advise the use of js Maps:

export function GetLibrary():Promise<Map<string, intypes.Comic>>;

and the creation of types aliases inside the models.ts (if not generating it was intended).

I tried giving a fast look at "internal/binding/generate.go" but I would need more time to figure it out

@FrancescoMerenda FrancescoMerenda added the Bug Something isn't working label Jan 15, 2023
@ATenderholt
Copy link
Contributor

I've seen something similar myself. I'll try to see if I can come up with a fix.

@ATenderholt
Copy link
Contributor

I found a couple more cases that weren't fixed by #2326:

type Key struct {
	Description string
	Label       string
}

type Value struct {
	Description string
	Label       string
}

func (h *AliasTest) LocalMapTypes() map[Key]Value                     { return nil }
func (h *AliasTest) LocalMapsTypesWithPointer() map[*Key]Value        { return nil }

@leaanthony How should pointers be handled? In general, seems like *type should become type? but that could be problematic for map keys.

Also, I saw tygo mentioned in #2323. Are there plans to switch to that and start using ast (or related) instead of reflect? If so, has that work been started somewhere?

@FrancescoMerenda
Copy link
Author

While trying to investigate this issue I managed to make a system that uses reflect that is capable of handling all of this different cases correctly but with some different things like using maps instead of generic objects, typed arrays, and .ts (instead of js+d.ts). I have no idea on how I could add it to wails and it probably needs some fixes.

Regarding pointers I think the actual binding is only made to transfer data and should keep the *type to type stuff

@leaanthony
Copy link
Member

leaanthony commented Feb 4, 2023

Also, I saw tygo mentioned in #2323. Are there plans to switch to that and start using ast (or related) instead of reflect? If so, has that work been started somewhere?

Yeah, the work has started! The parser is in v3/internal/parser, there is an example I'm using for testing in v3/examples/binding. If you install the v3 cli, then go to v3/examples/binding then run wails generate bindings, you can see the current progress. It's very basic and very incomplete (static analysis is hard!) but yes, I'm looking to somehow integrate tygo if possible, but I've been too busy getting a multi-window runtime done to look any further in it 😅 Could totally do with some help on that! You can ping me on discord for more info/help 👍

https://discord.com/channels/1042734330029547630/1071249333929922640

@FrancescoMerenda
Copy link
Author

type Key struct {
Description string
Label string
}

type Value struct {
Description string
Label string
}

func (h *AliasTest) LocalMapTypes() map[Key]Value { return nil }
func (h *AliasTest) LocalMapsTypesWithPointer() map[*Key]Value { return nil }

Correct me if I'm wrong, but this specific case is not standard json as only string keys are allowed. So either you decide a standard to pass it between frontend and backend (for example with two arrays of objects) or you simply can't bind that specific method.

P.S. to use it in js you will also need to use Maps instead of Objects and think about nested maps cases

@ATenderholt
Copy link
Contributor

type Key struct {
Description string
Label string
}
type Value struct {
Description string
Label string
}
func (h *AliasTest) LocalMapTypes() map[Key]Value { return nil }
func (h *AliasTest) LocalMapsTypesWithPointer() map[*Key]Value { return nil }

Correct me if I'm wrong, but this specific case is not standard json as only string keys are allowed. So either you decide a standard to pass it between frontend and backend (for example with two arrays of objects) or you simply can't bind that specific method.

P.S. to use it in js you will also need to use Maps instead of Objects and think about nested maps cases

Ah, of course. This is a case that can't be supported. Also dealing with Maps isn't trivial, so these cases would just need another approach like the standard you suggest.

@FrancescoMerenda
Copy link
Author

I was thinking that to handle maps this is what it's needed in the backend:

type GenericStruct struct {
	K []any
	V []any
}

func MapFromArrays(raw_data *GenericStruct) map[any]any {
	result := make(map[any]any, len(raw_data.K))
	for i, k := range raw_data.K {
		result[k] = raw_data.V[i]
	}
	return result
}

func ArraysFromMap(raw_map map[any]any) *GenericStruct {
	result := &GenericStruct{K: make([]any,len(raw_map)),V: make([]any,len(raw_map))}
	for k, v := range raw_map {
		result.K = append(result.K, k)
		result.V = append(result.V, v)
	}
	return result
}

This two functions are both needed for either/both args and results of the function.

In the exported ts this is what it's needed:

export function MapFromArrays<K, V>(raw: {"k":Array<K>, "v":Array<V>}):Map<K, V>{
	const map = new Map<K, V>();
	const [keys, values] = [raw["k"], raw["v"]];
	keys.forEach((k, i) => {
		map.set(k, values[i]);
	})
	return map;
}

export function ArraysFromMap<K, V>(map:Map<K, V>):{"k":Array<K>, "v":Array<V>} {
	const keys:Array<K> = [];
	const values:Array<V> = [];
	for (const [key, value] of map.entries()) {
		keys.push(key);
		values.push(value);
    }
    return {"k":keys, "v":values}
}

export async function LocalMapTypes(arg1:Map<app.Key, app.Value>): Promise<Map<app.Key, app.Value>>{
	const raw1 = window['go']['main']['App']['LocalMapTypes'](ArraysFromMap(arg1)) as {"k": Array<app.Key>, "v": Array<app.Value>}
	return MapFromArrays(raw1)
}

The ts/js functions must be added in every function export body (if they uses maps) as in the example.
If you want to keep up with the separated js/d.ts files standard the code is a just a bit different.

I would personally use ts instead of 2 separated files and I would add this two methods either to the runtime package or some WailsTools package since they are needed everytime you want to bind a function that needs maps.

I would like to know your ideas and maybe give me some guidance on how to add this stuff to wails if you want me to.

@ATenderholt
Copy link
Contributor

Those two map conversion functions make sense to me, not sure if this is something @leaanthony would want as part of v2 or should just go in v3.

@FrancescoMerenda
Copy link
Author

Those two map conversion functions make sense to me, not sure if this is something @leaanthony would want as part of v2 or should just go in v3.

Well I don't know how the wails back end works but this simple function doesn't handle nested maps, arrays of maps and objects with a field of type map. To handle those specific cases you would need a reviver for the json parser (in the frontend) and something similar for the backend (but I haven't seen how you can do it, maybe with a custom serializer/deserializer and some runtime reflections). Lastly, it should be specified in the docs which structure is reserved and can't be used ( so a struct with "k" and "v" as keys that could be "|k|", "|v|" to make it less common) to avoid bad serialization.

I'm not sure this is something that can be done easily and in a clean way. Maybe what should be done is simply use maps instead of objects when possible (so only with string type keys maps).

@leaanthony
Copy link
Member

What does tygo generate for these scenarios?

@FrancescoMerenda
Copy link
Author

What does tygo generate for these scenarios?

Apparently they don't handle those scenarios in a specific way maybe because they simply "translate" without worrying about how the json serializer would behave.

In js,ts if you try to json parse an invalid type you either get a string version of the type (if it's a valid object key) or a syntax error (for example if the key is an object).
In go you get a type error

@leaanthony
Copy link
Member

So then the question is, does tygo do most of what we already need (static analysis) and we extend it to handle these cases? TBH I'd like to extend it to do bindings generation too as there's no need to reinvent the wheel and the current chatGPT created static analyser appears to be slow in certain circumstances (yay recursion).

@FrancescoMerenda
Copy link
Author

So then the question is, does tygo do most of what we already need (static analysis) and we extend it to handle these cases? TBH I'd like to extend it to do bindings generation too as there's no need to reinvent the wheel and the current chatGPT created static analyser appears to be slow in certain circumstances (yay recursion).

The thing is that binding generation is strictly related to the desired output files structure. I'm talking about imports and external type reference and this is something you will never be able to achieve with tygo. Speed is not that important when it's only limited to compile time and reflect is not that slow anyway. I personally think that tygo simply has a different use case than this. Tygo translate while wails need to generate binds to specific structures, generate utility methods, is related to json serialization and deserialization and more. I think it simply need a revamp and some reworks to improve its capabilities.

As I said I can help you with this, I simply need some guidance on how to integrate it with wails source code

@FrancescoMerenda
Copy link
Author

Those two map conversion functions make sense to me, not sure if this is something @leaanthony would want as part of v2 or should just go in v3.

Well I don't know how the wails back end works but this simple function doesn't handle nested maps, arrays of maps and objects with a field of type map. To handle those specific cases you would need a reviver for the json parser (in the frontend) and something similar for the backend (but I haven't seen how you can do it, maybe with a custom serializer/deserializer and some runtime reflections). Lastly, it should be specified in the docs which structure is reserved and can't be used ( so a struct with "k" and "v" as keys that could be "|k|", "|v|" to make it less common) to avoid bad serialization.

I'm not sure this is something that can be done easily and in a clean way. Maybe what should be done is simply use maps instead of objects when possible (so only with string type keys maps).

I think I found how to do this:

JS/TS code:

export namespace Wails {
    export type WailsMap<K,V> = {"|K|": K[], "|V|": V[]}

    export function ArraysFromMap<K, V>(map:Map<K, V>): WailsMap<K, V> {
        const keys:Array<K> = [];
        const values:Array<V> = [];
        for (const [key, value] of map.entries()) {
            keys.push(key);
            values.push(value);
        }
        return {"|K|":keys, "|V|":values}
    }
    
    export function MapFromArrays<K, V>(raw: WailsMap<K, V>):Map<K, V>{
        const map = new Map<K, V>();
        const [keys, values] = [raw["|K|"], raw["|V|"]];
        keys.forEach((k, i) => {
            map.set(k, values[i]);
        })
        return map;
    }

    
    export function Reviver(key:string, value:any) {
      if (typeof value != "object") return value;
      switch (value.hasOwnProperty("|K|") && value.hasOwnProperty("|V|")) {
          case true: {
              return MapFromArrays(value);
          }
          default:
              return value;
      }
    }
    export function Replacer(key:string, value:any) {
        if (value instanceof Map) {
          return ArraysFromMap(value);
        }
        return value;
      }
}

TS in wails runtime:

const res = JSON.parse(data, Wails.Reviver)
//or
const str = JSON.stringify(res, Wails.Replacer);

GO:

type WailsMap[K comparable, V any] map[K]V

type JsonMap[K comparable, V any] struct {
	Keys   []K `json:"|K|"`
	Values []V `json:"|V|"`
}

func (m WailsMap[K, V]) UnmarshalJSON(data []byte) error {
	arrays := JsonMap[K, V]{}
	if err := json.Unmarshal(data, &arrays); err != nil {
		return err
	}
	for i, k := range arrays.Keys {
		m[k] = arrays.Values[i]
	}
	return nil
}

func (m WailsMap[K, V]) MarshalJSON() ([]byte, error) {
	result := &JsonMap[K, V]{Keys: make([]K, 0, len(m)), Values: make([]V, 0, len(m))}
	for k, v := range m {
		result.Keys = append(result.Keys, k)
		result.Values = append(result.Values, v)
	}
	return json.Marshal(result)
}

Go usage:

type MyStruct struct {
	Name string `json:"Name"`
	Age  int    `json:"Age"`
}
type MyKey struct {
	Title string `json:"Title"`
	Year  int    `json:"Year"`
}

func TestFromMap() {
	input := make(WailsMap[*MyKey, *MyStruct])
	input[&MyKey{"Avatar", 2009}] = &MyStruct{"John", 24}
	input[&MyKey{"Something", 2023}] = &MyStruct{"Jack", 22}
	data, _ := json.Marshal(&input)
	output := make(WailsMap[*MyKey, *MyStruct])
	json.Unmarshal(data, &output)
	fmt.Printf("%+v\n", string(data))
	for k,v :=range output{
		println(k.Title,k.Year,v.Name,v.Age)
	}
}

So basically this just needs to be added in the frontend and backend runtime and the only thing that can break is if someone uses that two thing as keys. Maybe a different method should be used in the js code to understand when he is in a wails map

@ATenderholt
Copy link
Contributor

@leaanthony I tried to ping you on Discord, but it's not working for me. The first time I click the link, there was a popup that sounded like I didn't have permissions (as I recall, this was awhile ago). Now it just takes me to an empty channel. I'm atenderholt#4872 if you need to send an invite.

@leaanthony
Copy link
Member

https://discord.gg/JDdSxwjhGf should work

@wsy998
Copy link

wsy998 commented Sep 25, 2023

I've identified a new issue. When my return value is a two-dimensional array, the frontend generates an 'any[]'.

@leaanthony
Copy link
Member

Feel free to add a test and open a PR 👍

@wsy998
Copy link

wsy998 commented Sep 25, 2023

Feel free to add a test and open a PR 👍

I can't fix this bug.😂But I can offer an idea: Can we parse the Go files using AST and then generate TypeScript files?

@manterfield
Copy link

Apologies if this is already covered above, but an issue I just found:

Given a method that returns a type

SomeLogs struct {
  Logs map[string][]LogEntry `json:"logs""
}

Types will be generated for app.SomeLogs correctly referencing the type app.LogEntry, but that app.LogEntry will not actually be generated.

If I change that to:

SomeLogs struct {
  Logs map[string]LogEntry `json:"logs""
}

or

SomeLogs struct {
  Logs []LogEntry `json:"logs""
}

then the type app.LogEntry is also correctly generated.

As a workaround to get the type for app.LogEntry in my frontend I added an extra field to my struct, but in truth that only exists to force generation of the bindings.

@leaanthony
Copy link
Member

I can't fix this bug.😂But I can offer an idea: Can we parse the Go files using AST and then generate TypeScript files?

We do this in v3

@leaanthony
Copy link
Member

Apologies if this is already covered above, but an issue I just found:

Basically, if it's not used it can't be picked up. It's annoying. For v3 we have implemented parsing the project directly.

@manterfield
Copy link

Basically, if it's not used it can't be picked up. It's annoying. For v3 we have implemented parsing the project directly.

Sorry, but I'm not sure I understand what "not used" means in this context.

The only thing that changed between the working (type generated) and not working (type not generated) versions was the depth of the type in the field.

The map of slices of LogEntry didn't generate, but map of LogEntry did. Both referred to the type in the typescript output regardless (but in the non-working one it was referring to a type that didn't exist).

(Restating in case I've explained badly in the first attempt)

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

5 participants