-
Notifications
You must be signed in to change notification settings - Fork 570
Enhance js.Object with shortcut for Slices/Maps conversion #137
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
Comments
What kind of JavaScript objects do you often use that are arrays/maps? |
Well actually all Model Data like for example the Todo-List of the TodoMVC Examples. What is the background of your question, don't you you a lot of Arrays/Maps in your daily JS Development ? Since we probably can not come up with hundreds of "DefinitelyTyped.go" bindings within a short time frame I asked myself what would be the result if I make the Interfacing to the existing js-world with just the plain js.Object. It has some disadvantages like for example being "stringly typed", but beside that It turned out that it works quite well. The resulting JS size is "relatively" small and for implementing your business logic you can use the usual go idioms. Here is an example of this experiment, still with a lot of native js Idioms like "forEach", "filter", "push" which could be avoided only with a lot of .(interface[]) conversions: https://github.com/rusco/vue/blob/master/js/app.go#L107 Have a look at it and give me your opinion ! |
What do you think of having |
I am not worried at all about breaking code, the compiler shows me how to refactor things. |
Yet another idea: |
I implemented the change suggested above. Other methods like .Str() and .Int() do not panic, they just force whatever type conversion JavaScript uses to get that type. On the other hand, .S() and .M() would need to panic, since there in no such forced type conversion in JS. I don't like mixing those two cases, I would like to keep it obvious to someone else who reads your code. |
We should look more into using |
Thanks for now, please give me some days to look into this before giving more feedback. In the meantime more gopherjs users might give their opinion. |
I'm not really sure why we're adding new types and breaking an existing API just to save some insignificant amount of typing. You don't see e.g. If someone is dealing with a lot of Adding |
FWIW, I agree with @dominikh's assessment here. I would prefer there to be as few JavaScript-specific things be added as possible. The criteria I would use is: can something be done without having to manually hand-edit the generated .js output? If not, it probably needs to be added. If yes but it requires being a little more verbose, that's probably normal (and if a certain packages wants, it can define local helpers to reduce verbosity). I have no problem with seeing One final point, I think we should be open to having multiple Go to JavaScript compilers in the future, and hopefully the Go code that needs to be written would be as similar as possible. It's possible another Go compiler will support native Go constructs, but they may or may not support all the same JavaScript inter-op types the same way as GopherJS does specifically. These are just my thoughts about future directions. I haven't had a chance to really try out these changes and form an opinion yet. Edit: It's worth noting that my personal preference is to avoid using Edit 2: Another point, I welcome experimentation and trying out new things in pursuit of a better/simpler solution. I don't mind breaking changes if it eventually leads to an improved experience for all, so don't hesitate to try things. |
Okay, I have reverted the change for now so you don't have to change your code yet. You have brought up some good points. Let's talk about it some more. My opinion atm:
|
My thoughts/comments:
I have no opinion on this. I don't actually do that in my Go/GopherJS code very often, if at all. So I defer to opinion of others.
I agree with that; sounds good to me. But it would be good to give 1-2 days of notice to allow for some discussion before bigger changes.
Can you elaborate on what |
Maybe
Food for thought: An option for keeping backwards compatibility long-term (if you think it's necessary) is to use git tag/branches |
Neither do I, on a general level. However, GopherJS has reached a point at which API breakage should be discussed in greater length, and shouldn't just serve as experimentation, as there's quite a bit of code that needs to be fixed whenever something breaks.
Sure, I can see the point of that and I've used similar in other projects, too. However, it's not precisely something that needs to be part of
Do keep in mind that struct tags will not help with something like
If you mean for constructing values, sure. I think that'd be more in line with Go naming than |
Yes yes, I get it. ;-) Next time we'll have more discussion beforehand. I didn't expect such a lively discussion, but I like it.
The struct could have the function itself: type Global struct {
js.Object
SomeFunc func() []int `js:"someFunc"`
}
func test() {
global := &Global{Object: js.Global}
for _, v := range global.SomeFunc() {
println(v)
}
} |
+1 for breaking the API in the future, maybe in a dev branch: even if the change reveals some drawbacks and has to be reverted you gain always more insight and experience |
I agree with #137 (comment), it seems like there isn't that much appetite for this issue at the moment. I would note, however, that once generics land in Go 1.18 and are supported by GopherJS we could re-evaluate I'm going to close this issue now, but if you think this is a mistake — please let me know, we can always reopen it. |
Knowing that the underlying Javascript Object is an Array or a Map I see myself repeatedly using the verbose
SomeJsObject.Interface().([]interface{}) and SomeJsObject.Interface().(map[string]interface{})
conversion to be able to iterate with the range statement.
Wouldn't it be useful to have these conversions implemented directly in js.Object ?
The text was updated successfully, but these errors were encountered: