golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
122.67k stars 17.49k forks source link

misc/wasm: "this" missing in wasm callbacks #27441

Closed theclapp closed 5 years ago

theclapp commented 6 years ago

What version of Go are you using (go version)?

go version go1.11 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env: GOARCH="amd64" GOBIN="" GOCACHE="/Users/lmc/Library/Caches/go-build" GOEXE="" GOFLAGS="" GOHOSTARCH="amd64" GOHOSTOS="darwin" GOOS="darwin" GOPATH="/Users/lmc" GOPROXY="" GORACE="" GOROOT="/usr/local/go" GOTMPDIR="" GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64" GCCGO="gccgo" CC="clang" CXX="clang++" CGO_ENABLED="1" GOMOD="" CGO_CFLAGS="-g -O2" CGO_CPPFLAGS="" CGO_CXXFLAGS="-g -O2" CGO_FFLAGS="-g -O2" CGO_LDFLAGS="-g -O2" PKG_CONFIG="pkg-config" GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/32/8nd008752lv7v3vsgzlrw6qr0000gn/T/go-build736262546=/tmp/go-build -gno-record-gcc-switches -fno-common"

Edited to add GOARCH=wasm GOOS=js go env:

GOARCH="wasm" GOBIN="" GOCACHE="/Users/lmc/Library/Caches/go-build" GOEXE="" GOFLAGS="" GOHOSTARCH="amd64" GOHOSTOS="darwin" GOOS="js" GOPATH="/Users/lmc/src/goget:/Users/lmc/src/go_appengine/gopath" GOPROXY="" GORACE="" GOROOT="/usr/local/go" GOTMPDIR="" GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64" GCCGO="gccgo" CC="clang" CXX="clang++" CGO_ENABLED="0" GOMOD="" CGO_CFLAGS="-g -O2" CGO_CPPFLAGS="" CGO_CXXFLAGS="-g -O2" CGO_FFLAGS="-g -O2" CGO_LDFLAGS="-g -O2" PKG_CONFIG="pkg-config" GOGCCFLAGS="-fPIC -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/32/8nd008752lv7v3vsgzlrw6qr0000gn/T/go-build048369190=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Wrote a Go/wasm callback and wanted to access this from the callback.

What did you expect to see?

Some way to access this.

In Javascript, if you call some_object.some_method(), then inside some_method, a special variable called this will be set to some_object. If you call some_other_object.some_method(), then this will be set to some_other_object.

In Go/wasm, the js.NewCallback() function is the only way to get a function that Javascript can call. (Your callback is not called directly, but pushed onto a list of callbacks to be run by a separate goroutine.) js.NewCallback arranges for your function to get any function arguments passed to the actual callback, but there's no way to get to the value of this.

What did you see instead?

There's no way to access this.

Every Javascript function has a this, even if it's undefined.

I think either NewCallback should have an explicit this argument:

func NewCallback(fn func(this Value, args []Value)) Callback

or put this as args[0] for every callback.

Discussion

I believe this issue is independent of (a)synchronous callbacks (#26045). The value of this could be captured when the "real" callback actually runs, saved, and passed in to the Go code when it runs, whether it's synchronous or asynchronous.

Sample code:

diff --git a/misc/wasm/wasm_exec.js b/misc/wasm/wasm_exec.js
index 94b9552c59..94036c2685 100644
--- a/misc/wasm/wasm_exec.js
+++ b/misc/wasm/wasm_exec.js
@@ -390,7 +390,7 @@

                static _makeCallbackHelper(id, pendingCallbacks, go) {
                        return function() {
-                           pendingCallbacks.push({ id: id, args: arguments });
+                         pendingCallbacks.push({ id: id, args: arguments, this: this });
                                go._resolveCallbackPromise();
                        };
                }
diff --git a/src/syscall/js/callback.go b/src/syscall/js/callback.go
index 9d573074cb..ded9cde03c 100644
--- a/src/syscall/js/callback.go
+++ b/src/syscall/js/callback.go
@@ -109,9 +109,10 @@ func callbackLoop() {
                        }

                        argsObj := cb.Get("args")
-                   args := make([]Value, argsObj.Length())
-                   for i := range args {
-                           args[i] = argsObj.Index(i)
+                 args := make([]Value, argsObj.Length()+1)
+                 args[0] = cb.Get("this")
+                 for i := range args[1:] {
+                         args[i+1] = argsObj.Index(i)
                        }
                        f(args)
                }
agnivade commented 6 years ago

I have encountered a need for this only in DOM event callbacks to access the original element. A common way to achieve this is to use the NewEventCallback, and do ev.Get("target") to get the source element.

What is the scenario where you need to access the original object which is other than the source element in an event callback ?

theclapp commented 6 years ago

@ALTree Thanks for cleaning up this issue.

@agnivade I'm writing a wasm wrapper for Vue.js. (Actually converting one from GopherJS.) Vue.js frequently passes what it calls "the current vm / Vue instance" as this in various handlers (see for example here, "All lifecycle hooks are called with their this context pointing to the Vue instance invoking it"), and I see no other way to get that piece of information.

Even if there were such a way (in my particular context of writing a Vue.js wrapper), in other contexts it's easy to imagine that that would not be the case. Not having access to this is analogous to only being able to write regular Go methods like func (_ T) funcName(...), i.e. with no access to the receiver.

I understand that one way or the other adding this will change the API and that it should not go in sooner than 1.12, and probably concurrently with or as a part of #26045, I just didn't feel that that issue sufficiently called out the need for this.

agnivade commented 6 years ago

Understood. Thanks for clarifying.

/cc @neelance

myitcv commented 6 years ago

@theclapp and I caught up offline on this.

Whilst not explicitly called out, https://github.com/golang/go/issues/26045 does silently imply the addition of the this parameter: @neelance and I discussed as much offline.

So this issue might be useful in case it's worth adding this parameter support independent of https://github.com/golang/go/issues/26045, but as @theclapp and I discussed offline, there are issues with doing so (this might not be in the state that the callback handler expects, much like the event/other arguments)

theclapp commented 6 years ago

For folks following along at home: I've had good luck with a JS factory function that captures this and calls the Go function with it explicitly:

// Javascript factory function
function call_with_this(f) {
    return function() {
        f(this, ...arguments);
    }
}
// Go factory function; wraps js.NewCallback
func NewCallback(f func(this js.Value, args []js.Value) interface{}) js.Value {
    return js.Global().Call("call_with_this",
        js.NewCallback(func(args []js.Value) {
            f(args[0], args[1:])
        }))
}
gopherbot commented 5 years ago

Change https://golang.org/cl/142004 mentions this issue: all: add support for synchronous callbacks to js/wasm