Skip to content

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

Closed
rusco opened this issue Dec 19, 2014 · 18 comments
Closed

Enhance js.Object with shortcut for Slices/Maps conversion #137

rusco opened this issue Dec 19, 2014 · 18 comments

Comments

@rusco
Copy link
Member

rusco commented Dec 19, 2014

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 ?

package main

import "github.com/gopherjs/gopherjs/js"

var console = js.Global.Get("console")

func log(val ...interface{}) {
    console.Call("log", val...)
}

func GetArray(o js.Object, id string) []interface{} /* or js.S ? */ {
    return o.Get(id).Interface().([]interface{})
}

func GetMap(o js.Object, id string) map[string]interface{} /* or js.M ? */ {
    return o.Get(id).Interface().(map[string]interface{})
}

func main() {
    g := js.Global

    //sample1:
    g.Set("S", js.S{"a", "b", "c"})
    s := GetArray(g, "S")
    for idx, val := range s {
        log(idx, val)
    }

    //sample2:
    g.Set("i", []interface{}{"d", "e", "f"})
    i := GetArray(g, "i")
    for idx, val := range i {
        log(idx, val)
    }

    //sample 3:
    g.Set("M1", map[string]interface{}{"g": 1, "h": "hi1", "i": true})
    m1 := GetMap(g, "M1")
    for key, val := range m1 {
        log(key, val)
    }

    //sample 4:
    g.Set("M2", js.M{"g": 1, "h": "hi2", "i": true})
    m2 := GetMap(g, "M2")
    for key, val := range m2 {
        log(key, val)
    }

}
@dmitshur
Copy link
Member

Knowing that the underlying Javascript Object is an Array or a Map

What kind of JavaScript objects do you often use that are arrays/maps?

@rusco
Copy link
Member Author

rusco commented Dec 20, 2014

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
https://github.com/rusco/vue/blob/master/js/app.go#L116
https://github.com/rusco/vue/blob/master/js/app.go#L130
https://github.com/rusco/vue/blob/master/js/app.go#L175

Have a look at it and give me your opinion !

@neelance
Copy link
Member

neelance commented Jan 3, 2015

What do you think of having Interface() return js.S and js.M instead of []interface{} and map[string]interface{}. Then you could write obj.Interface().(js.M). It makes sense to me. This would break existing code, but better improve that API now than later. What I would dislike about an even shorter version is that it would hide the type assertion which may panic at runtime.

@rusco
Copy link
Member Author

rusco commented Jan 3, 2015

I am not worried at all about breaking code, the compiler shows me how to refactor things.
Returning js.S/js.M would be a minor syntactical improvement which I would very welcome.

@rusco
Copy link
Member Author

rusco commented Jan 3, 2015

Yet another idea:
If you leave the current API unchanged and add two more functions, maybe .S() and .M(), then it would be in the responsibility of the API user to avoid runtime panics, right ? My experience is that when I am interfacing the brittle JS world I have to carry type information with me in my mind anyway, therefore I wrote "knowing that ...".
Would it be a high effort for you to implement this change in a dev-branch ? Being able to make some real-world experiments would help to understand eventual drawbacks.

@neelance
Copy link
Member

neelance commented Jan 3, 2015

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.

@neelance
Copy link
Member

neelance commented Jan 3, 2015

We should look more into using js tags on fields to make this easier, I feel like this would be the better option.

@rusco
Copy link
Member Author

rusco commented Jan 3, 2015

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.

@dominikh
Copy link
Member

dominikh commented Jan 3, 2015

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. text/template.M to avoid passing a map[string]interface{} to template.Execute.

If someone is dealing with a lot of js.Object to map[string]interface{} conversions, surely he could've just written a one-line function that does the type assertion for him, if he truly wanted to save some typing.

Adding js.M and related, however, just adds another level of "this is JS, this is special", while it really is just an ordinary map[string]interface{}, something that exists elsewhere in the Go world already.

@dmitshur
Copy link
Member

dmitshur commented Jan 4, 2015

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 map[string]interface{} and I have a very good intuition of what it means, while js.M is less clear.

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 js package in code I write, or use it as little as possible. Instead, I rely on wrappers, which I'm okay with being more verbose/painful to write, but the idea is once they're available, you get a nice clean Go API complete with useful godoc and autocompletion. So any syntactic improvements to dealing with js package are largely wasted on me. I mostly care about the boolean value of "can it be done".

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.

neelance added a commit that referenced this issue Jan 4, 2015
@neelance
Copy link
Member

neelance commented Jan 4, 2015

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:

  • I like having js.S and js.M for constructing simple slices and maps to pass to JS. The type information is nearly useless there, since it gets lost when externalizing the values to JS arrays and objects. The idea is from http://godoc.org/labix.org/v2/mgo/bson#M . I see that this might not be as useful when converting values from JS to Go.
  • I don't mind breaking the existing js API. I feel that the project still needs to figure out the best ways for interfacing with JS.
  • As mentioned above, maybe we should focus on improving the js field tags. I have the feeling that they can be much more powerful, since they allow more flexible types, e.g. other slice types than just []interface{}.

@dmitshur
Copy link
Member

dmitshur commented Jan 4, 2015

My thoughts/comments:

  • I like having js.S and js.M for constructing simple slices and maps to pass to JS.

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 don't mind breaking the existing js API. I feel that the project still needs to figure out the best ways for interfacing with JS.

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.

  • As mentioned above, maybe we should focus on improving the js field tags. I have the feeling that they can be much more powerful, since they allow more flexible types, e.g. other slice types than just []interface{}.

Can you elaborate on what js field tag usage you have in mind? I don't quite understand how it's relevant here. Also, do you consider issue #141 (which is related to the same js fields tags, or is that something else?) as something worth fixing, and what kind of timeframe/milestone would you aim for?

@emidoots
Copy link

emidoots commented Jan 4, 2015

I like having js.S and js.M for constructing simple slices and maps to pass to JS.

Maybe js.Slice and js.Map could be used instead? It's longer to write.. but I think it's far more clear what it is.

I don't mind breaking the existing js API. I feel that the project still needs to figure out the best ways for interfacing with JS.

Food for thought: An option for keeping backwards compatibility long-term (if you think it's necessary) is to use git tag/branches go1.4 etc as the Go tool recognizes them and checks out those tags.

@dominikh
Copy link
Member

dominikh commented Jan 4, 2015

I don't mind breaking the existing js API.

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.

I like having js.S and js.M for constructing simple slices and maps to pass to JS

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 gopherjs/js at all. Anyone can just write type S []interface{} if they so desire; although this isn't a point I'll further argue about much, having the type in the js package by itself doesn't add nearly as much complexity as actually returning values of those types does.

maybe we should focus on improving the js field tags

Do keep in mind that struct tags will not help with something like o.Call().Interface().([]interface{}) – I don't think anyone would want to wrap the result of o.Call() in a struct, just to benefit from struct tags.

Maybe js.Slice and js.Map could be used instead

If you mean for constructing values, sure. I think that'd be more in line with Go naming than S and M would be. If you imply that this makes returning values of those types more acceptable, I still have to object, though.

@neelance
Copy link
Member

neelance commented Jan 4, 2015

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.

Do keep in mind that struct tags will not help with something like o.Call().Interface().([]interface{}) – I don't think anyone would want to wrap the result of o.Call() in a struct, just to benefit from struct tags.

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)
  }
}

@rusco
Copy link
Member Author

rusco commented Jan 4, 2015

+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
+1 for exploring js Tags: Anything that shortens the semantic gap to the JS world is welcome

@myitcv
Copy link
Member

myitcv commented Apr 26, 2017

@shurcooL - this can probably be closed in light of #617 and #633? Unless there's more discussion required on js.S and js.M (but I fear the passage of time has probably put that conversation to bed)?

@nevkontakte
Copy link
Member

nevkontakte commented Oct 23, 2021

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 js.Object methods in the new light.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants