-
Notifications
You must be signed in to change notification settings - Fork 570
Proposal: implicit Object instantiation for *js.Obect-special struct types #634
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
Can you clarify what you meant by "mixing of the two forms"? |
Thanks for the prompt - I should have included such an example... modified the headline description accordingly. |
Ah perfect this is what I was looking for. Big +1 from me. My previous gopherjs tests had been primarily JS -> Go for bindings, however now that I'm writing some that are primarily Go -> JS with several dozens of structs, it's extremely painful to lose the ability to use the struct literal and assign each field on its own. I think this is especially true since it ruins what would otherwise be a normal public API, now you have to interact with it differently just because of gopherjs. Unless there's a trick everyone is using? I could see having two versions of the same struct and assigning them manually etc, or converting to a Happy to help implement this if it seems like something a gopherjs internals noob could dive into. |
Edit: Updated New() after some real usage. Here's a sketch of an implementation of this idea in current GopherJS, if you don't mind the use of js.InternalObject() to get at the "outer" versions of a js-special field. Given a type T struct {
*js.Object
i int `js:"i"`
j int
} we're trying to automate something like this: func NewT(t *T) *T {
t.Object = js.Global.Get("Object").New()
io := js.InternalObject(t)
t.Set("i", io.Get("i"))
return t
} Example: t := NewT(&T{i: 1, j: 2})
println("t:", t, "t.i:", t.i, "t.j:", t.j)
// => t: Object {$val: Object, Object: Object, i: 1, j: 2} t.i: 1 t.j: 2
t.i = 3
println("t:", t, "t.i:", t.i, "t.j:", t.j)
// => t: Object {$val: Object, Object: Object, i: 1, j: 2} t.i: 3 t.j: 2
// Note that the "outer" i hasn't changed ^^^^ This function assumes that #633 has not been implemented, so there are the usual "inner" and "outer" parts of a js-special struct, and a field you want to go into the "inner" Object has to have a // Edit: Warning: New() doesn't work with more complex types, e.g. []string fields.
// It's a neat toy, so far, but not production ready. Test carefully.
func New(t interface{}) interface{} {
io := js.InternalObject(t)
valueOfT := reflect.ValueOf(t).Elem()
// If the first field (assumed to be the *js.Object field) is set, just
// return t unchanged. Does no other error checking.
f0Name := valueOfT.Type().Field(0).Name
if io.Get(f0Name) != nil {
return t
}
if !valueOfT.Field(0).CanSet() {
// reflect's Set method won't set unexported fields
panic("The *js.Object field must be exported")
}
typ := valueOfT.Type()
obj := js.Global.Get("Object").New()
for field := 1; field < typ.NumField(); field++ {
if jsName, ok := typ.Field(field).Tag.Lookup("js"); ok {
goName := typ.Field(field).Name
obj.Set(jsName, io.Get(goName))
}
}
valueOfT.Field(0).Set(reflect.ValueOf(obj))
return t
} Example: t2 := New(&T{i: 10, j: 11}).(*T)
println("t2:", t2, "t2.i:", t2.i, "t2.j:", t2.j)
// => t2: Object {$val: Object, Object: Object, i: 10, j: 11} t2.i: 10 t2.j: 11
t2.i = 13
println("t2:", t2, "t2.i:", t2.i, "t2.j:", t2.j)
// => t2: Object {$val: Object, Object: Object, i: 10, j: 11} t2.i: 13 t2.j: 11
// "outer" i still hasn't changed. ^^^^^ Playground link. Edit: This link does not have the updated New() code listed above, though it is similar. I've only just written this today and haven't played with it much beyond the examples given, but it seems promising. This function doesn't implement some of the error checking specified. In particular it ignores this part "Mixing of the two forms would result in a compiler error". Also, since it's written using the stock |
This was exactly my motivation behind raising this for discussion so 👍 to your comments.
I'm working around this for now in type _PProps struct {
o *js.Object
ClassName string `js:"className"`
}
type PProps struct {
ClassName string
}
func (p *PProps) assign(v *_PProps) {
v.ClassName = p.ClassName
}
func P(props *PProps, children ...Element) *PDef {
rProps := &_PProps{
BasicHTMLElement: newBasicHTMLElement(),
}
if props != nil {
props.assign(rProps)
}
// ...
} So as a user of the However in my case this proxying is code generated so I can sympathise with a situation where this isn't possible/practical... Side noteTo build on your point about a "normal public API" there is another thing to note with the props example. Only making mention of this to clarify what's going on in the example above. I'm effectively using the type _PProps struct {
*BasicHTMLElement
} gets "exploded" into type PProps struct {
ClassName string
DangerouslySetInnerHTML *DangerousInnerHTMLDef
ID string
Key string
OnChange
OnClick
Role string
Style *CSS
} in order that the caller can use composite literals without needing to explicitly reference embedded struct types. With the explosion we can do: p := &PProps{
ClassName: "example",
} without it we'd be left to do: p := &PProps{
BasicHTMLElement: &r.BasicHTMLElement{
ClassName: "example",
},
} |
@theclapp - not quite sure I follow. Are you suggesting we should retain the inner and outer parts of |
@myitcv No, it was mostly because I think this proposal would be great, and have wanted it or something like it myself for a long time, so I implemented it and posted it here. If this proposal is never implemented, or takes a long time, in the meantime we have some code that works now that gets us 80% of the way there. It might also conceivably provide a guide to whoever eventually implements this proposal. |
@theclapp great, sounds like we're all arriving at the same solution in some way shape or form!
#617, #633 and #644 are the next things I'm going to work on, in that order. #633 will require coordination with authors of bindings (because it's a somewhat of a breaking change). But that said, all three issues are effectively on my critical path so selfishly at least I want to see them solved 👍 |
Under this proposal, you could still say |
@theclapp - (good question because I think this particular case could do with an explicit mention) The underlying v := &T{}
// would be equivalent to
v := &T{o: js.Global.Get("Object").New()} Can you see any issues with this? |
I don't think so. I thought I would when I asked the question in the first place, but on further consideration I think not. It helps me to imagine that we've redefined the zero-value for a js-special struct. In the same way that you can immediately assign to fields in a regular Go struct after creating it, you can also under this proposal immediately assign to fields in a js-special struct after you've created it. type T1_Go struct {
i int
}
type T2_JS struct {
o *js.Object `jsspecial:"true"`
i int
}
t1 := &T1_Go{}
t1.i = 1
t2 := &T2_JS{}
t2.i = 1 // no muss, no fuss, same as a Go struct;
// e.g. no need to initialize t2.o first. |
I can't quite tell from this thread, but does the above assume a restriction (compilation error) is in place that prevents |
@shurcooL I don't think we assume that, though as I understand it, and from what I've seen, non-pointer
(Of course, pretty sure I wrote that part of the wiki!) ... And yet playing around with it briefly in the playground seems to work. Has that always worked and I was just mistaken, or has something changed? |
As @theclapp says, it's certainly not set in stone according to how things are currently written:
Theoretically (subject somewhat to the current discussion in #633) there is no reason why non-pointer values would not work. But to my mind by enforcing the use of pointer types (and values) we better reflect the fact that the underlying So to agree with a point that @flimzy made earlier (where he even suggested we implement this now, before these 3 proposals) we should make it a compile time error when the non-pointer type is used. |
Ok. Because, yeah, if values are allowed, it would mean the zero values would not be equal, which would be weird: var t1, t2 T1_Go
fmt.Println(v1 == t2) // true
var t3, t4 T2_JS
fmt.Println(t3 == t4) // false (because t3.Object != t4.Object) |
Uh oh!
There was an error while loading. Please reload this page.
Following discussion with @neelance and @shurcooL
This proposal is a follow on from #633, that is to say, please read this proposal as if #633 has been approved, implemented and merged.
Consider the following
*js.Object
-special struct type declaration:At present, as described in the wiki, when creating a value of type
*T
via a struct literal, you have to initialise the fields separately:We propose that values of type
*T
can be created in either one of two ways:&T{o: someO}
&T{Name: "Peter"}
The first case represents no change over the current situation.
In the second case, an
Object
is implicitly instantiated for us, so it's as if we'd written:Note, by preventing a mixing of the two forms we remove any questions/ambiguity around order of evaluation etc. Mixing of the two forms would result in a compiler error:
This proposal leans heavily on the discussion in #236 (thanks for raising @Archs and to @theclapp and others for the ensuing discussion)
The clearer semantics introduced in #633 make this proposal itself much clearer, but also allow for fluent, unambiguous use of composite literals for
*js.Object
-special struct types.The text was updated successfully, but these errors were encountered: