gopherjs / gopherwasm

This package is deprecated. Use syscall/js of GopherJS instead.
BSD 2-Clause "Simplified" License
79 stars 5 forks source link

proposal: support wrapping existing native objects #25

Open slimsag opened 5 years ago

slimsag commented 5 years ago

If you are making use of a library which only gives you an underlying *gopherjs/js.Object (GopherJS) or syscall/js.Value (WebAssembly) object, it is impossible to use this library with that object.

This is unfortunate because it makes interoperability hard: when you are given one of these underlying objects, you must use those APIs independently rather than being able to make use of this generic library. It means that in order to use this library, you must effectively switch completely to this library first.

The primary reason I bring this up is because Vecty will not expose this library but rather only the native object type, which means users of Vecty will not be able to use this package. I would like them to be able to.

I thus propose a way to wrap an existing native object and get back a gopherwasm/js.Value. This would be done by a function whose type signature changes based on a build tag:

Under GopherJS:

// Wrap wraps a *gopherjs/js.Object and returns a Value.
//
// When compiling under WebAssembly, this function signature instead takes a
// syscall/js.Value.
func Wrap(object *js.Object) Value

And under WebAssembly:

// Wrap wraps a syscall/js.Value and returns a Value.
//
// When compiling under WebAssembly, this function signature instead takes a
// *gopherjs/js.Object.
func Wrap(value js.Value) Value
hajimehoshi commented 5 years ago

I think your suggestion makes sense. I'm happy that you're trying to use GopherWasm at Vecty :-)

Wrap

My concern is the name: I'm worried the possibility that syscall/js would add a new function named Wrap. WrapAsGopherWasmValue or ToGopherWasmValue? Hmm.

slimsag commented 5 years ago

I considered this, too, but I think it is unlikely as there was some prior discussion about adding a syscall/js.Wrap function and the name was effectively turned down on being too abstract: https://github.com/golang/go/issues/28711#issuecomment-439069168

I think it could be a good choice of naming here because this function would likely often be used when performing interoperability.

hajimehoshi commented 5 years ago

Thank you! After reading the comment, I came up with the same concern: what will Wrap wrap? :-)

I think it could be a good choice of naming here because this function would likely often be used when performing interoperability.

I don't know other examples. Could you show me one?

slimsag commented 5 years ago

Thank you! After reading the comment, I came up with the same concern: what will Wrap wrap? :-)

With syscall/js.Wrap you must wonder: What does it wrap? Does it wrap "js"? Does it wrap a "syscall"? That doesn't make sense.

With gopherwasm/js.Wrap, my thinking is that it wraps gopher or wasm. It wraps gopherwasm, like WrapGopherWasm. But since what it wraps changes based on the build tag, we would end up with WrapGopher and WrapWasm or something. So we omit that and end up with just Wrap.

I don't know other examples. Could you show me one?

Basically, I imagine this will be used often in conjunction with event handling in specific. For example in Vecty any time you define an event handler you will call this once or twice:

event.Input(func(e *vecty.Event) {
    p.Input = gopherwasm.Wrap(e.Target).Get("value").String()
    gopherwasm.Wrap(e.Value).Call("preventDefault")
    vecty.Rerender(p)
}),

But if the name is complex, long, hard to type, etc. it becomes much more verbose:

event.Input(func(e *vecty.Event) {
    p.Input = gopherwasm.ToGopherWasmValue(e.Target).Get("value").String()
    gopherwasm.ToGopherWasmValue(e.Value).Call("preventDefault")
    vecty.Rerender(p)
}),
slimsag commented 5 years ago

WrapNative would be my second choice, but I still worry it is too long / verbose

hajimehoshi commented 5 years ago

Basically, I imagine this will be used often in conjunction with event handling in specific. For example in Vecty any time you define an event handler you will call this once or twice:

I meant that I was wondering if there are other libraries that use a function named Wrap for similar purpose so that I can confirm that Wrap is idiomatic in Go.

Thank you for your example, I think that makes sense.

hajimehoshi commented 5 years ago

I found several examples like https://godoc.org/github.com/pkg/errors#Wrap

OK, I'm now convinced to add Wrap. Would you create a PR?

slimsag commented 5 years ago

Ah, I see, I misunderstood. Yes, I will create a PR in the near future (but possibly not this weekend).

hajimehoshi commented 5 years ago

https://tip.golang.org/doc/go1.12#syscall/js As Go 1.12 tries to introduce Wrapper, I'm now doubtful that Wrap is and will be safe 🤔

slimsag commented 5 years ago

Hmm, interesting. I was actually thinking From might be a better name:

// From creates a gopherwasm Value from a syscall/js.Value or *gopherjs/js.Object.
func From(v js.Value) Value
hajimehoshi commented 5 years ago

Yet another suggestion: would it work if we extend ValueOf to accept js.Value or *js.Object?