Skip to content

[FEATURE REQUEST] Assignments in struct literal #236

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

Open
Archs opened this issue Jun 3, 2015 · 15 comments
Open

[FEATURE REQUEST] Assignments in struct literal #236

Archs opened this issue Jun 3, 2015 · 15 comments

Comments

@Archs
Copy link
Contributor

Archs commented Jun 3, 2015

I know now the js tag works fine, but the struct field has to be accessed directly to be effective, and thus the struct literal #114 (comment) would not work declaratively, for example:

a := &A{
        Object: js.Global.Get("Object").New(),
        Method: func() {
            println("hello")
        },
    }
a.Method()  // This gives: Uncaught TypeError: a.Object.Method is not a function.

but the following code is ok:

a := &A{
        Object: js.Global.Get("Object").New(),
    }
a.Method = func() {
    println("hello")
}
a.Method() // this is ok

This troubles me for a quite long time, could assignments in struct literal works directly?

I think this feature is useful and would be friendly to the beginners of gopherjs, for:

  1. it's more compatible with Golang itself
  2. when using the native js libraries, it's easier to supply constructor options as a struct literal
    than to declare a struct variable first and then to set each field separately
  3. you can be more declarative 😄
@neelance
Copy link
Member

neelance commented Jun 3, 2015

Yeah, we should probably do that. 👍

@abrander
Copy link
Contributor

I have added a Playground example to highlight the issue here:

http://www.gopherjs.org/playground/#/Eu_7w28UnN

I think all three tests should print the same expected result..?

@dmitshur
Copy link
Member

I agree the first two cases should print the same {"something":"Hello"} result, but I'm unsure if the third case should. Its *js.Object value is nil, so null output appears correct. I'm not sure how to handle setting Something when *js.Object is nil, perhaps that should be like setting a field of a struct via a nil pointer, a nil pointer panic?

@abrander
Copy link
Contributor

shurcooL,
I'm not entirely sure what the third case should return - but as you're hinting current behavior fails silently, which is always confusing.

This gets interesting if we could marshal a struct without a js.Object - can we do that somehow? That would be awesome for initializing "options"-objects which will never be read in Go-land.

@theclapp
Copy link

I looked into this a bit and thought I'd record my thoughts.

First, the GopherJS code is pretty nice and I could do simple things in it without understanding all of it. Well done.

Second, part of the problem here is that if we try to initialize all the fields on Object, that can cause problems with objects that have read-only fields, like WebSockets. But if we don't initialize them, then we might have uninitialized struct values, which would break one of the core guarantees of Go, where uninitialized struct fields have their zero value. Not sure what to do about it yet.

I couldn't find any api that would tell me whether an arbitrary field was writable or not. I tried Object.getOwnPropertyDescriptor, but it didn't work for WebSocket.url or any of its other read-only fields, though it's possible I used it incorrectly.

I also considered adding a tag field to note a field was read-only (e.g. jsro:"yes"), but haven't tried it yet.

I'm unclear on why structs with *js.Object fields have their fields repeated, once at the struct level and once within the Object field. As near as I can tell, if you have a *js.Object field, any struct field references go into the *js.Object field and the struct-level field values are ignored. At least, as long as the field has a js:"fieldName" tag.

In any case, in general the expectation seems to be that if a struct has a *js.Object field, then it's from Javascript and comes as-is. On the Go side we shouldn't be creating structure literals that try to initialize their fields. That's just my impression.

Is there a wiki page somewhere that documents the interaction of using *js.Object or not, and using js tags or not, and internal vs. external values?

Edit, grammar.

@myitcv
Copy link
Member

myitcv commented Oct 6, 2016

👍 on this, but I wonder whether we can/should go further.

Considering the following

package mypackage

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

type S struct {
    o *js.Object
    Name string `js:"name"`
}

func NewS() S {
    return S{o: js.Global.Get("Object").New()}
}
package main

import (
    "github.com/somewhere/mypackage"
    "github.com/gopherjs/gopherjs/js"
)

func main() {
    s1 := mypackage.S{Name: "Banana"}
    s2 := mypackage.NewS()
    s2.Name = "Banana"
    console := js.Global.Get("console")
    console.Call("log", s1, s1.Name, s2, s2.Name)
}

The output is, as "expected" (given the current implementation):

Object {} "undefined" Object {name: "Banana"} "Banana"

Notice I've intentionally used an unexported name for the *js.Object field in mypackage.S

However, don't we "want" the following expression:

mypackage.S{}

to transpile as if we'd written (given S in this instance):

mypackage.S{o: js.Global.Get("Object").New()}

(which we can't because the field o is intentionally not exported)

Reason being:

  • otherwise we always need a New* functions all over the shop
  • it saves typing the tiresome js.Global.Get("Object").New()
  • but also that:
mypackage.S{Name: "Banana"}

will not, given the current implementation, work as we might expect from a Go (read gc) perspective.

I realise this would be a further special casing for structs that define a single *js.Object field... is it too special?

If we followed this additional suggestion then:

mypackage.S{Name: "Banana"}

would effectively work as if compiled from the conceptual (but impossible in language terms):

mypackage.S{o: js.Global.Get("Object").New(), Name: "Banana"}

Thoughts?

@theclapp
Copy link

theclapp commented Oct 6, 2016

otherwise we always need a New* functions all over the shop

That's what I've done in my code and I agree it's mildly annoying.

it saves typing the tiresome js.Global.Get("Object").New()

Agree, but you can write a utility to at least make it shorter, which again is what I've done.

[automatically initialize *js.Object struct members]

I agree that this would be great and more natively Go-like. It's harder than it sounds. Implicit in doing that is initializing any unmentioned fields with their zero values. As I touched on in my earlier comment, some native Javascript object slots are read only, and I could find no a priori way to figure out which, so trying to initialize them will (I think) throw exceptions. (I guess we could catch them, but that seems yucky.)

And of course there may be other obstacles. I'm far from an expert in GopherJS internals.

That said, it seems like initializing random JS struct literals should be easier (i.e. just like regular Go structs), and accounting for the odd corner case should be possible.

I propose a JSMaker interface that the compiler would be aware of. If a type does not implement this interface, and the compiler is creating a struct literal with a *js.Object slot as the first slot, then automatically allocate it, and process other slot initializations in the appropriate way, routing them to the *js.Object slot as appropriate. That would suffice for the common case. I think.

For the uncommon case, then a type must have a JSMake method, which takes the object in question and initializes the JS slots in whatever way is appropriate.

If anyone likes this idea (particularly @neelance and @shurcooL) then I'll try to come up with some sample code.

@myitcv
Copy link
Member

myitcv commented Oct 6, 2016

@theclapp

I agree that this would be great and more natively Go-like. It's harder than it sounds. Implicit in doing that is initializing any unmentioned fields with their zero values

I'm not sure I understand what you mean by "unmentioned"?

I'm simply talking here about creating a regular Javascript Object

All I'm proposing is that:

mypackage.S{}

effectively transpile as if we'd written:

mypackage.S{o: js.Global.Get("Object").New()}

If there is more complex object creation required for the *js.Object field (that needs to be aware of some of the other things you mention, but to be honest I don't understand given my lack of experience) then you probably need a New* function in any case, and your API documentation should state this is how instances of S are created.

@theclapp
Copy link

theclapp commented Oct 7, 2016

type T struct {
    o *js.Object
    A int `js:"a"`
    B int `js:"b"`
}
t := T{o: js.Global.Get("Object").New()}

A and B are unmentioned. The Go spec says that they have to be initialized to their zero values, which is of course zero.

If all you want is for T{} to transpile as T{o: js.Global.Get("Object").New()}, and work even if T is in another package, that seems possible.

But if you also want this to work

t := T{A: 1, B: 2}

(which is the main thrust of this ticket), what I'm saying is that that's harder than it sounds.

Edit: Formatting.

@myitcv
Copy link
Member

myitcv commented Oct 9, 2016

@theclapp

A and B are unmentioned

Ah, I see now, thanks for clarifying. In spec terms they are omitted from the struct literal element list.

Just to clarify one point; consider the following:

package main

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

type S1 struct {
    A int    `js:"A"`
    B string `js:"B"`
    C []byte `js:"C"`
}

type S2 struct {
    *js.Object

    A int    `js:"A"`
    B string `js:"B"`
    C []byte `js:"C"`
}

func main() {
    s1_1 := S1{}
    s1_2 := S1{A: 5}
    s1_3 := S1{A: 2, B: "test"}

    s2_1 := S2{Object: js.Global.Get("Object").New()}
    s2_2 := S2{Object: js.Global.Get("Object").New()}
    s2_2.A = 5
    s2_3 := S2{Object: js.Global.Get("Object").New()}
    s2_3.A = 2
    s2_3.B = "test"

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

    console.Call("log", s1_1, s1_2, s1_3)
    console.Call("log", s2_1, s2_2, s2_3)
}

In the context of today's implementation, I'm advocating that:

x := S2{A: 5}

be transpiled as if we'd written:

x := S2{Object: js.Global.Get("Object").New()}
x.A = 5

In clarifying that, I think I also understand what you mean about this being "harder than it sounds" (correct me if I'm wrong). Consider the output from the example above:

s1_1 Object {A: 0, B: "", C: Array[0]} s1_2 Object {A: 5, B: "", C: Array[0]} s1_3 Object {A: 2, B: "test", C: Array[0]}
s2_1 Object {} s2_2 Object {A: 5} s2_3 Object {A: 2, B: "test"}

There is currently an asymmetry when it comes to structs that have a special *js.Object field vs those that do not. Further highlighted by this example:

package main

import (
    "fmt"

    "github.com/gopherjs/gopherjs/js"
)

type S struct {
    *js.Object

    A string `js:"A"`
}

func main() {
    o := js.Global.Get("Object").New()
    s := S{Object: o}

    console := js.Global.Get("console")
    console.Call("log", s.A)

    fmt.Println(s.A)
}

the output of which is:

undefined

in both the GopherJS playground console and browser console.

(Notice this is the mirror of the case you were providing, but I present it this way around to deal with the perhaps more common use-case of "consuming" external objects in GopherJS space

I actually think it's a problem(/bug?) that the output of fmt.Println(s.A) is undefined in the GopherJS console, and that it should instead be "". That is to say, in GopherJS-world (as opposed to the call to console.log) a read from a field of a *js.Object special struct should do something like (imagining the transpiled JS):

s.A === undefined ? "" : s.A

Perhaps @neelance and @shurcooL can comment at the point because I'm confident this must have been considered in the past

But it's important the struct initialisation does not do zero-value initialisation of omitted fields because GopherJS-external code may be sensitive to properties being present on an object or not. Hence:

x := S2{A: 5}

should not be transpiled as if we'd re-written:

x := S2{Object: js.Global.Get("Object").New()}
x.A = 0
x.B = ""
x.C = nil

@myitcv
Copy link
Member

myitcv commented Oct 11, 2016

Also consider http://www.gopherjs.org/playground/#/6UOPY4gswG

If instead we restrict the use of a *js.Object-special type (after its definition) to its pointer version, then bugs like this will hopefully never crop up.

e.g.

var x S      // ERROR
var x *S     // ok
var x **S    // ERROR
// ...

@theclapp
Copy link

In clarifying that, I think I also understand what you mean about this being "harder than it sounds" (correct me if I'm wrong).

You're not wrong. That's what I meant.

There is currently an asymmetry when it comes to structs that have a special *js.Object field vs those that do not.

Indeed. And understanding that, and reasoning about your code with that in mind, requires some adjustment. It's kind of magic, and not documented well enough. (Disclaimer: I haven't tracked GopherJS development closely since about July, so this may've improved.)

a read from a field of a *js.Object special struct should do something like (imagining the transpiled JS):

s.A === undefined ? "" : s.A

I think it should return its actual value. Doing anything else would make it harder to debug other code, especially native JS code that uses objects generated by Go code. And like so many cases, it's just not that simple. With a string, yeah, you can generate a "" on the fly. What do you do with structured types?

But it's important the struct initialisation does not do zero-value initialisation of omitted fields because GopherJS-external code may be sensitive to properties being present on an object or not. Hence:

x := S2{A: 5}

should not be transpiled as if we'd re-written:

x := S2{Object: js.Global.Get("Object").New()}
x.A = 0
x.B = ""
x.C = nil

This is the problem exactly. See above for my JSMaker interface suggestion on how to solve it. I guess it'd look something like this:

type JSMaker interface {
    JSMake(initializer interface{})
}

type regular struct {
    *js.Object
    A int `js:"a"`
}

type special struct {
    *js.Object
    A int `js:"a"`
    Readonly string `js:"readonly"`
}

func (s special) JSMake(rawInit interface{}) {
    init := rawInit.(special)
    s.A = init.A
    // s.Readonly not touched
}

So r := regular{A: 1} transpiles as

r := regular{Object: js.Global.Get("Object").New()}
r.A = 1

and s := special{A: 2} transpiles as

s := special{Object: js.Global.Get("Object").New()}
s.JSMake(special{A: 2})

and importantly, wrapping an existing JS object must still work:

o := getSomeRandomJSObject()
r := regular{Object: o}
s := special{Object: o}

both transpile exactly the same:

r := regular{Object: o}
s := special{Object: o}

I think if you specify the *js.Object slot and give initializers for slots that have js tags, the compiler'd have to throw an error. Which is weird, from the Go point of view, but makes sense, in the context of this discussion. The rule would be: If you mention the *js.Object, that's the only slot you can mention.

This really requires some experimentation with the GopherJS internals that I don't have time for right now, and don't really have the background for anyway, alas. FWIW, as I mentioned in January, I found the GopherJS code pretty readable, and you might be able to get a handle on this if you're willing to burn a Saturday on it.

@theclapp
Copy link

I said:

type special struct {
    *js.Object
    A int `js:"a"`
    Readonly string `js:"readonly"`
}

func (s special) JSMake(rawInit interface{}) {
    init := rawInit.(special)
    s.A = init.A
    // s.Readonly not touched
}

[...] and s := special{A: 2} transpiles as

s := special{Object: js.Global.Get("Object").New()}
s.JSMake(special{A: 2})

As usual, it's not that simple. Because in JSMake, init.A transpiles to init.Object.Get("a"). But init.Object is nil. Hmpf. Back to the drawing board.

It's entirely possible that the magic we'd have to embed in the compiler (and account for in our reasoning) would not outweigh the pain of just writing constructors.

@myitcv
Copy link
Member

myitcv commented Oct 12, 2016

@theclapp

With a string, yeah, you can generate a "" on the fly. What do you do with structured types?

The zero value for that field's type.

But my later comment is relevant at this point:

type A struct {
    *js.Object

    Name string `js:"name"`
    MyB  *B     `js:"myB"`
}

type B struct {
    *js.Object

    Age int `js:"age"`
}

Assuming the comment I referenced makes sense, then the type of field MyB has to be *B. Hence, the zero value in this situation would be nil

See above for my JSMaker interface suggestion on how to solve it

I'm not clear why we need another solution, other than the GopherJS compiler itself?

The semantics we're talking about here come entirely from having defined a struct type with a single field of type *js.Object (in addition to > 0 non*js.Object fields); I don't believe there's anything else we need in order to tell the GopherJS compiler to treat values of this struct differently?

As I said however, for these *js.Object-special types, I think the pointer restriction is necessary.

I think if you specify the *js.Object slot and give initializers for slots that have js tags, the compiler'd have to throw an error. Which is weird, from the Go point of view, but makes sense, in the context of this discussion.

This is an interesting point, but I would not err towards the constraint you describe because there's only one conceivable semantic if the *js.Object value is provided along with other field values. Considering:

var o *js.Object = ...

r := &A{Object: o, Name: "Paul"}

then it's as if we'd written:

r := &A{Object: o}
r.Name = "Paul"

which is effectively some sort of "merge" (I use that term carefully given it has some "well defined" meanings in Javascript-space)

@theclapp
Copy link

With a string, yeah, you can generate a "" on the fly. What do you do with structured types?

The zero value for that field's type.

Well yes, obviously. My point is, that can get expensive and non-trivial, and I'm not sure, but I suspect it'd be hard to do correctly in the face of nested JS types with read-only fields. (On the other hand, if we can get them right at all, then we can presumably get them right here.)

In any case, I stick by my earlier assertion: I'd rather it gave the actual value.

type A struct {
    *js.Object

    Name string `js:"name"`
    MyB  *B     `js:"myB"`
}

Assuming the comment I referenced makes sense, then the type of field MyB has to be *B

You can't actually put a Go pointer in a JS object. See http://www.gopherjs.org/playground/#/8zF9pawHx8.

I'm not clear why we need another solution, other than the GopherJS compiler itself?

You may be right. I'm not sure any more.

[...] there's only one conceivable semantic if the *js.Object value is provided along with other field values [...]

Fair enough.


We've gone back and forth on this a bit now. Basically I think the idea is to break the Go guarantee that all slots are initialized to their zero values for struct literals if the type has a *js.Object as its first slot. We'd initialize any mentioned fields, and ignore any unmentioned fields. On the surface, that seems like it'd work (for certain values of "work" :).

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

No branches or pull requests

7 participants