Proposal: explicit *js.Object-special struct types #633
Comments
|
👍 My contribution to the bikeshedding: What about |
|
Under the new proposal would the following be compliant: |
|
I think I like this proposal. My specific comments below.
This sounds like a big improvement, that wouldn't even need to wait for this proposal. Is there a reason (other than being a breaking change) this behavior couldn't be implemented immediately?
I wonder if this tag is really even necessary. Go already prohibits duplicate fields, so there will only ever be one embedded Maybe the thought is that someone might want to disable this functionality by not specifying the tag? Is that a meaningful use-case? If the tag is deemed necessary, then my bikeshedding contribution would be to use the existing |
|
Another point for consideration: It might be desirable to disable this functionality on a per-field basis. Supporting The use case would be when wrapping a JS object that you don't want to pollute with Go-specific fields: Of course a work-around would be two structs in Go, effectively managing the "external" object explicitly. |
|
Edit: strikethrough question to @neelance and @shurcooL @4ydx I notice you updated your example to make the slice of type @flimzy - thanks for the feedback.
Potentially, yes. I'm going to be focussing on this proposal in the first instance so if you or indeed anyone else would like to take up the mantle on making a more immediate change...
The type T struct {
o *js.Object `jsspecial:"true"`
Name string `js:"name"`
Age int
location string
}
Not that I can think of, but again I might be missing something.
I'll add that to the list (in the headline description)
Related to my point above, I don't think it makes sense to disable this functionality. This is essentially where the analogy with
👍 |
Hmm. I guess I missed that part of the proposal. I was about to ask whether this was really appropriate, but I think in writing my question, I've convinced myself that it is. :) (Which I consider slightly unfortunate... I was hoping for a cleaner spec, but I digress...)
You're talking about Go methods, right (not JS functions as object members). I'm not seeing immediately why this is problematic. Maybe you can elaborate a bit? |
You didn't at all because it's possible today! https://gopherjs.github.io/playground/#/25H-0WPIk7
Not sure I follow (the bit about a "cleaner spec")?
Thanks for prompting me to actually think about this 👍 I've arrived at the same conclusion: it's not a problem. |
Just saying that not requiring an extra tag would be a bit cleaner. But that's irrelevant, since it's not a good option :) |
|
[First paragraph moved from my comment on #634 and edited slightly.] I kind of like the inner and outer parts of js-special structs -- I kind of like the ability to have js-data and a channel or mutex in the same struct -- but I'm happy with the workaround of using a regular Go struct for the Go-bits and containing a js-special struct for the JS bits. I like Would there still be outer and inner objects? That is, would the js fields be implemented as foo.Object.field_name? Or would there be only one object? E.g. if I have type T struct {
*js.Object `jsspecial:"true"`
Name string `js:"name"`
Age int
location string
}
t := &T{Name: "Larry"}
println(t)what would I see in the console? Would I see (something similar to) If there are still outer and inner objects, I like @flimzy's idea of |
|
Thanks for your feedback @theclapp
I think this is the direction we've pretty much headed in so good to hear that you can't think of any critical reasons against it.
I'll answer from the perspective of the Javascript VM (which is ultimately what you see in the debugger and all that matters at the end of the day) Let's take your example (assuming we haven't implemented #634 for now): t := &T{Object: js.Global.Get("Object").New()}This will transpile to: t = new T.ptr(new ($global.Object)(), "");So in Javascript VM terms there are two objects that have been instantiated here. But under this proposal, setting any fields on t.Name = "Paul"
t.Age = 99will transpile to: t1.Object.name = $externalize("Larry", $String);
t1.Object.Age = 99That is, under this proposal it will be impossible to declare and therefore set any fields on the "outer" object. Hence the So to answer your question precisely there are two objects at play. But from a Go perspective it's as if you're setting fields on just one (hopefully that makes sense?)
That's a slightly different question because t := &T{Object: js.Global.Get("Object").New()}
t.Name = "Larry"
print(t)will transpile to: t = new T.ptr(new ($global.Object)(), "");
t.Object.name = $externalize("Larry", $String);
console.log(t);Because we can now accurately identify If you then want to bring a var v *js.Object
// v gets set somehow
t := &T{Object: v} |
|
Just as an aside, before any breaking changes are introduced, it might be nice to get vendored GopherJS working so people who can't immediately update their code have some recourse. I think #415 is the issue tracking that. |
In #415 it's unclear (to me at least) whether they (the maintainers) want vendoring to be a supported option, and it seems like 415 could be harder and/or more intricate than this issue and its relatives (#634). So maybe just make a branch or tag or release or fork, so it's easy to get the "before #633" code, and leave it at that. (Just my two cents. I don't have any GopherJS in production, so it's easy for me to say "go ahead and break the world, I won't mind". :) |
We'll definitely need to figure out a viable transition plan. I think that should be described as part of the proposal.
I've applied labels to make it more clear. It's a bug, it requires figuring out what's wrong and coming up with a fix. I'm not aware of anyone working on it right now. |
Good point and noted |
|
I'd like to point out that where both BasicNode and URLUtils contain the same JavaScript object in their |
|
@dominikh thanks for the feedback; yours was feedback I was keen to solicit, particular when it comes to migration (but that can wait for later). Rewriting part of your example in terms of this proposal: type BasicNode struct {
*js.Object `jsspecial:"true"`
}
type BasicElement struct {
*BasicNode
}
type BasicHTMLElement struct {
*BasicElement
}
type URLUtils struct {
*js.Object `jsspecial:"true"`
Href string `js:"href"`
// ...
}the embedding of a struct with a But just to reiterate an earlier point, we lose the Go "side"... every field in a struct is the Javascript object. ...which leads nicely onto Currently (i.e. pre this proposal): var o = js.Global.Get("Object").New()
a := &HTMLAnchorElement{
URLUtils: &URLUtils{
Object: o,
},
BasicHTMLElement: &BasicHTMLElement{
BasicElement: &BasicElement{
BasicNode: &BasicNode{
Object: o,
},
},
},
}
a.HrefLang = "HrefLang"
a.Href = "href"transpiles to: o = new ($global.Object)();
a = new HTMLAnchorElement.ptr(new BasicHTMLElement.ptr(new BasicElement.ptr(new BasicNode.ptr(o))), new URLUtils.ptr(o, ""), "");
a.BasicHTMLElement.BasicElement.BasicNode.Object.hreflang = $externalize("HrefLang", $String);
a.URLUtils.Object.href = $externalize("href", $String);which ends up modifying the same underlying object, Under this proposal, the transpiled result looks quite different because the o = new ($global.Object)();
a = new HTMLAnchorElement.ptr(new BasicHTMLElement.ptr(new BasicElement.ptr(new BasicNode.ptr(o))), new URLUtils.ptr(o, ""), "");
a.BasicHTMLElement.BasicElement.BasicNode.Object.hreflang = $externalize("HrefLang", $String);
a.BasicHTMLElement.BasicElement.BasicNode.Object.URLUtils.href = $externalize("href", $String);The This is a breaking change therefore. And I think this snippet (taken from your func wrapHTMLElement(o *js.Object) HTMLElement {
if o == nil || o == js.Undefined {
return nil
}
el := &BasicHTMLElement{&BasicElement{&BasicNode{o}}}
c := elementConstructor(o)
switch c {
case js.Global.Get("HTMLAnchorElement"):
return &HTMLAnchorElement{BasicHTMLElement: el, URLUtils: &URLUtils{Object: o}}
// ...confirms as much (hope you agree?). However, I wonder whether there is an alternative, picking up on this comment... But before I go down that path too much, let me first clarify: is the reason you're embedding in this way to effectively "subclass"? i.e. re-use the definition of |
Do you have a working fork that generates that, or are you more or less forecasting? 'Cause that seems odd. It's kind of weird that If I understand correctly, this breaks the idea of embedding a JS-special struct in a Go struct with other regular Go fields. If JS-special-ness is "transitive upwards", so to speak, then any field in a struct with a js-special struct deeper in its definition becomes a js field. E.g., S3 won't work: type S1 struct {
*js.Object
I int `js:"i"`
}
type S2 struct {
ch chan int
}
type S3 struct {
*S1
S2
ch2 chan int
J int `js:"j"`
}
type S4 struct {
S2
*S1
}Again, IIUC, in S3 you'd have S2 (and thus S2.ch) and ch2 be fields in S1.Object, which won't work (channels aren't externalizable). And S2 isn't defined as a pointer, which in a regular Go struct would be fine, but in this case, I don't know. And then what if you change the order of S1 and S2 (like in S4), what (if anything) happens there? Is the "transitive-upwardness" of js-special-ness conditional on the js-special field being the first field? That starts to get weird. (Playing with these ideas here.) Sorry, I'm confused. Bottom line: how how would @dominikh achieve what he has now under this proposal? |
|
@myitcv In terms of the DOM API, both URLUtils and HTMLAnchorElement are interfaces, and HTMLAnchorElement uses URLUtils as a mixin. On the Go side, DOM interfaces translate to structs, and mixins are ordinary composition (or what you'd call "subclassing"). |
|
@dominikh thanks.
Indeed, in https://www.w3.org/TR/html5/ terms.
Yes I couldn't quite reconcile the use of "subclass" myself... mixin is better. But the key is that when this code gets transpiled there is a single underlying Javascript object, as you say. Either which way... What about if we code-generated the types in With code generation, consider the resulting type HTMLAnchorElement struct {
*js.Object `jsspecial:"true"`
// interface HTMLAnchorElement
//
Target string `js:"target"`
Download string `js:"download"`
HrefLang string `js:"hreflang"`
// ...
// interface HTMLElement
//
Title string `js:"title"`
Lang string `js:"lang"`
// ...
// interface URLUtils
//
Href string `js:"href"`
Protocol string `js:"protocol"`
// ...
}Then instantiation is easy: a := &HTMLAnchorElement{Object: js.Global.Get("Object").New()}
a.Lang = "Martian"(and indeed with #634 we could go one better to For edge cases we'd still have the option of writing methods etc. Just looking to leverage some code generation to avoid the repetitive parts where there is clear reuse. Does this also perhaps help to present a better "interface" to the user, per @tj's comment in the linked thread? I ask because composition doesn't map well to what these a := &HTMLAnchorElement{
URLUtils: &URLUtils{
Object: o,
},
BasicHTMLElement: &BasicHTMLElement{
BasicElement: &BasicElement{
BasicNode: &BasicNode{
Object: o,
},
},
},
}I'm already doing something similar with |
|
To be frank, I'm not really interested in making js/dom more complex (code generation), while also breaking backwards compatibility and reducing features. Right now, you can access the embedded URLUtils and pass it around as its own thing, which is a decent compromise considering we can't use Go interfaces to match struct fields. |
This is definitely what we need to consider carefully. Are you of the opinion therefore that the costs outweigh the benefits for this proposal (from a
Aside from the "feature" of embedded
Where does this "feature" get used? That said, I'm not sure that I would agree this is a "feature", more an unintended side effect. It comes at the cost of the bizarre creation construct I included in my previous comment. In any case, if someone wants to achieve the same effect they can re-use the underlying |
I can't give a proper cost assessment yet.
Well, it's part of the public API of js/dom, so any of its users may make use of that fact. In particular, any library or helper that operates on any URL-like things would benefit from accepting a URLUtils and instantly work on all link-like objects in the DOM.
I would very much call it a feature. It's composition like it is typical for Go. A thing that consists of other things.
Other than having to specify the same More importantly, if we move past my specific case, we arrive at #633 (comment) and not being able to intuitively embed structs as soon as any of these structs is backed by a |
|
@myitcv said:
Not sure I agree. There's a single underlying JS object because the structs were initialized that way: a := &HTMLAnchorElement{
URLUtils: &URLUtils{
Object: o,
},
BasicHTMLElement: &BasicHTMLElement{
BasicElement: &BasicElement{
BasicNode: &BasicNode{
Object: o,
},
},
},
}Given a different use-case there could just as easily be distinct Object slots in URLUtils and BasicNode. My point being, you seem to be saying that that's an inherent property of the compiled code and I don't think that's the case. The core of this problem seems to be the assumption that js-special-ness travels upwards. How can we fix that? Currently, if you have a js-special object as your first field, either directly or embedded however deeply, then you can access that object by adding a The key there is that the field "asks" for special treatment, rather than some other part of the struct (field 0) "imposing" special treatment at a distance. Could we require that if you embed a js-special struct and want its special-ness to include the current struct (and thus want the current struct to be a "pure Javascript" object), then you have to tag the embedded field, too? Example: type BasicNode struct {
*js.Object `jsspecial:"true"`
}
type BasicElement struct {
*BasicNode `jsspecial:"true"`
}
type BasicHTMLElement struct {
*BasicElement `jsspecial:"true"`
}
type URLUtils struct {
*js.Object `jsspecial:"true"`
Href string `js:"href"`
// other fields
}
type HTMLAnchorElement struct {
*BasicHTMLElement `jsspecial:"true"`
*URLUtils `js:"-"`
HrefLang string `js:"hreflang"`
// other fields
}This requires the Alternatively, without the type HTMLAnchorElement_Inner struct {
*BasicHTMLElement `jsspecial:"true"`
HrefLang string `js:"hreflang"`
// other js-only fields
}
type HTMLAnchorElement struct {
*HTMLAnchorElement_Inner
*URLUtils
// other Go-only fields
}It makes initialization somewhat more complex: a := &HTMLAnchorElement{
URLUtils: &URLUtils{
Object: o,
},
HTMLAnchorElement_Inner: &HTMLAnchorElement_Inner{
BasicHTMLElement: &BasicHTMLElement{
BasicElement: &BasicElement{
BasicNode: &BasicNode{
Object: o,
},
},
},
},
}but you could also say this, I think: a := &HTMLAnchorElement{}
a.HTMLAnchorElement_Inner.Object = o
a.URLUtils.Object = o(although you could also do that now). Without the Edit: Typo. |
|
I need to address the point that @theclapp made earlier (and @dominikh referenced) that starts:
There is an issue here to discuss, you're right... will respond when I get a chance over the weekend. |
|
@dominikh @theclapp - thanks for the various responses. By way of a preamble, the motivation behind this proposal was to make the definition of I completely concede, @theclapp, that given the current implementation it's entirely possible to have But just to emphasise this is my intention alone... yes it was the context for this proposal but it's certainly not set in stone. Indeed the whole point of putting together this proposal was to flush out problems, like the gap you have found in the specification. So picking up this problem, namely not being able to intuitively embed structs as soon as any of these structs is backed by a type BasicNode struct {
*js.Object `jsspecial:"true"`
}
type BasicElement struct {
*BasicNode `jsspecial:"true"`
// ...
}It enables us to unambiguously specify/distinguish between the following two types: type T1 struct {
*BasicElement `jsspecial:"true"`
Banana string
}
type T2 struct {
*BasicElement
Apple string
}
@dominikh @theclapp I think this deals with the ambiguity - please shout if you disagree. So back to @dominikh's Here is the current (i.e. pre-proposal) implementation annotated with the proposed tags: type HTMLAnchorElement struct {
*BasicHTMLElement `jsspecial:"true"`
*URLUtils `jsspecial:"true"`
HrefLang string `js:"hreflang"`
// other fields
}@dominikh I've definitely come round to the idea that embedding such types means that you're extending/mixing-in/whatever the containing type (if we gloss over the fact that name conflicts in Go and Javascript worlds are handled differently). In the example above I've annotated two such fields with the special tag. Indeed we could even provide a convenience function called package js
// Wrap takes o, a *js.Object Javascript object and a non-nil pointer i
// to a *js.Object-special struct type and sets the underlying *js.Object
// of i to o
//
func Wrap(o *js.Object, i interface{})But the need for such constructors (or indeed the In an attempt to flesh out an alternative I put together the following, where essentially the code re-use (of // *********************
// interface definitions
//
type URLUtilsIntf interface {
URLUtils() *URLUtils
}
type HTMLElementIntf interface {
HTMLElement() *HTMLElement
}
type HTMLAnchorElementIntf interface {
HTMLAnchorElement() *HTMLAnchorElement
}
// *********************
// implementation definitions
//
type URLUtils struct {
*js.Object `jsspecial:"true"`
// interface URLUtils
//
Href string `js:"href"`
Protocol string `js:"protocol"`
// ...
}
var _ URLUtilsIntf = new(URLUtils)
func (u *URLUtils) URLUtils() *URLUtils {
return u
}
type HTMLElement struct {
*js.Object `jsspecial:"true"`
// interface HTMLElement
//
Title string `js:"title"`
Lang string `js:"lang"`
// ...
}
var _ HTMLElementIntf = new(HTMLElement)
func (h *HTMLElement) HTMLElement() *HTMLElement {
return h
}
type HTMLAnchorElement struct {
*js.Object `jsspecial:"true"`
// interface HTMLAnchorElement
//
Target string `js:"target"`
Download string `js:"download"`
HrefLang string `js:"hreflang"`
// ...
// interface HTMLElement
//
Title string `js:"title"`
Lang string `js:"lang"`
// ...
// interface URLUtils
//
Href string `js:"href"`
Protocol string `js:"protocol"`
// ...
}
var _ HTMLElementIntf = new(HTMLAnchorElement)
var _ HTMLAnchorElementIntf = new(HTMLAnchorElement)
var _ URLUtilsIntf = new(HTMLAnchorElement)
func (h *HTMLAnchorElement) HTMLElement() *HTMLElement {
return &HTMLElement{Object: h.Object}
}
func (h *HTMLAnchorElement) HTMLAnchorElement() *HTMLAnchorElement {
return h
}
func (h *HTMLAnchorElement) URLUtils() *URLUtils {
return &URLUtils{Object: h.Object}
}This isn't perfect by any stretch:
On reflection I can definitely see arguments both ways... and as you've pointed out, we can't lose sight of the fact that any change from the status quo has a cost. Thoughts/feedback welcomed... |
|
Any further thoughts on this, now that we've let it simmer for a while? Also: Are we kind of dead in the water without a migration plan? If we provided a |
|
@theclapp no further thoughts from me... but still keen to get thoughts from others if there are outstanding concerns/questions. #617 is being worked on first in any case (per @shurcooL) but if we can round out discussion on this issue in parallel that would be a big help.
Tooling is a must to my mind if we're making such a breaking change (and should be relatively easy to write, if |
Uh oh!
There was an error while loading. Please reload this page.
Following discussion with @neelance and @shurcooL
Currently (as of 265040a to be explicit)
*js.Object-special struct types are inferred from struct types which have a*js.Object-typed field as the first field and a least one field with ajstag (as described in the wiki):We propose changing this to a more explicit approach where the
*js.Object-typed field is tagged withjsspecial:"true", and thejsfield tags are optional:Under this proposal it would be an error for a struct to declare more than one field annotated with
jsspecial:"true"(after embedding).The impact of this change would be:
*js.Object-special struct types more explicit and therefore easier to explain/understand. We could even make it a compiler error to use a non-pointer value of such types (a point which is currently only enforced via guidance in the wiki)*js.Objectvalue). This helps remove any sort of confusion around there being two parts: every assignment to a field is redirected onto the*js.Objectvalue in thejsspecialtagged fieldjstag optional for fields we follow a pattern much closer to, for example,encoding/jsontags, i.e. a familiar pattern (with the exception that even unexported fields are supported). Thejstag would then only be required where a different name is required on the underlying*js.ObjectHere's a quick example under this proposal:
to be clear, under the current implementation this outputs simply
{ name: 'Lorem' }The following would however be a compile error for struct type
S:This would be a "breaking" change:
*js.Objectfields with the new tagThe first of these points is easily fixed; making the change also allows the new code to be backwards compatible with previous versions of GopherJS.
It's assumed that very, very few people (indeed anyone?) are relying on the "outer" part in the current implementation; please shout if that's a bad assumption
Related:
struct literal#236Open questions
#### Q1: Do we make methods defined on*js.Object-special struct types an error?ContextProbably, yes.Q2: What should the identifying tag for such struct types be?
jsspecial:"true"- ruling this out because it was intentionally provocatively badmapjsfields:"true"@neelancejs:"*"@flimzyQ3: Add migration plan to the proposal for feedback
That. This proposal is recognised as a "breaking" change... so we need to make clear what that means and how the migration path will minimise impact, the process by which breakages are fixed etc.
The text was updated successfully, but these errors were encountered: