Calling an instance method that uses this fails with undefined

[genevieve]

I have a Response object with a constructor that accepts a body as argument, and has a method called text() that returns said body as string. When I get the response as an object and call the method, I get Cannot read property 'body' of undefined.

Here's the example go code:

package main

import (
	"fmt"
	"io/ioutil"

	"rogchap.com/v8go"
)

func main() {
	script, _ := ioutil.ReadFile("script.js")
	iso, _ := v8go.NewIsolate() // creates a new JavaScript VM
	ctx, _ := v8go.NewContext(iso)
	val, _ := ctx.RunScript(string(script), "script.js")
	respObj, _ := val.AsObject()
	textVal, _ := respObj.Get("text")
	textFn, _ := textVal.AsFunction()
	body, err := textFn.Call()
	if err != nil {
		panic(err)
	}
	fmt.Println(body)
}

Here's the script:

class Response {
  constructor(body) {
    this.body = body;
  }
  text() {
    return this.body.toString();
  }
}

new Response("this body");

Here's the error:

panic: TypeError: Cannot read property 'body' of undefined

goroutine 1 [running]:
main.main()
	/Users/genevieve/workspace/test/main.go:35 +0x37c
exit status 2

I'm assuming I'm doing something wrong but not sure exactly what it is. Any guidance is appreciated 🙏🏻

EDIT: There might be parts of https://github.com/rogchap/v8go/pull/142/files that are relevant.

EDIT 2: Questions - Should a v8go function support bind()? Should a v8go function keep a reference to the object it belongs to with an internal this ptr? Is this function not really executing directly on the instance of response that I think it is?

[genevieve]

I think the way v8go allows us to call functions assumes the function is acting on the global scope and not as a method on the instance of some object (like the response one in this case.) Would it be reasonable for the v8go object to expose a method like Call(fnName string, args ...Valuer) so it executes the method directly on the instance of the object? It doesn't even seem like there's a way to do that with v8 in c++ - https://v8docs.nodesource.com/node-0.8/db/d85/classv8_1_1_object.html

[genevieve]

To experiment, I changed the receiver for fn->Call to the parent object as part of a new ObjectCall function, and this now works but this is introducing something that would change the intention of v8go which is to be a simple wrapper on v8.

genevieve/v8this@0a2dbe9

+RtnValue ObjectCall(ValuePtr ptr, const char* key, int argc, ValuePtr args[]) {
+  LOCAL_OBJECT(ptr);
+  RtnValue rtn = {nullptr, nullptr};
+
+  Local<String> key_val =
+      String::NewFromUtf8(iso, key, NewStringType::kNormal).ToLocalChecked();
+  Local<Value> maybeFn = obj->Get(local_ctx, key_val).ToLocalChecked();
+  if (!maybeFn->IsFunction()) {
+    rtn.error = ExceptionError(try_catch, iso, local_ctx);
+    return rtn;
+  }
+  Local<Function> fn = Local<Function>::Cast(maybeFn);
+  Local<Value> argv[argc];
+  for (int i = 0; i < argc; i++) {
+    m_value* arg = static_cast<m_value*>(args[i]);
+    argv[i] = arg->ptr.Get(iso);
+  }
+  /* Local<Value> recv = Undefined(iso); */
+  MaybeLocal<Value> result = fn->Call(local_ctx, obj, argc, argv);
+  if (result.IsEmpty()) {
+    rtn.error = ExceptionError(try_catch, iso, local_ctx);
+    return rtn;
+  }
+  m_value* rtnval = new m_value;
+  rtnval->iso = iso;
+  rtnval->ctx = ctx;
+  rtnval->ptr = Persistent<Value, CopyablePersistentTraits<Value>>(iso, result.ToLocalChecked());
+  rtn.value = tracked_value(ctx, rtnval);
+  return rtn;
+}

[genevieve]

Shopify@99a0ece

[dylanahsmith]

What you are doing from Go corresponds to javascript like

r = new Response("this body")
f = r.text
f()

which gets the same error, since a function object doesn't carry a receiver with it.

The implicit passing of undefined as the receiver seems like an unfortunate gotcha to the javascript language, which more experienced javascript developers may be familiar with, but can easily cause confusion when first encountered.

v8's API more closely follows the Function.prototype.call javascript method in passing the receiver explicitly, but actually requires the receiver a required parameter. This avoids the confusion of implicitly passing undefined.

It seems like v8go's Function.Call method tries to provide an API that gophers would expect, but with behaviour that they wouldn't expect. Making go developers pass the receiver explicitly seems better, since it would give them the behaviour that they would expect so they avoid this confusion. This would also follow v8's original API which is another project goal.

Would it be reasonable for the v8go object to expose a method like Call(fnName string, args ...Valuer) so it executes the method directly on the instance of the object? It doesn't even seem like there's a way to do that with v8 in c++ - https://v8docs.nodesource.com/node-0.8/db/d85/classv8_1_1_object.html

In terms of naming, we might want to avoid confusion with two ways an object itself can actually be called v8::Object::CallAsFunction and v8::Object::CallAsConstructor. Since it isn't the object itself being called in this case, perhaps MethodCall would be a more appropriate name.

I think that would be useful as a convenience method for this use case. However, the change to Function.Call would provide the more general capability of passing an explicit receiver to a function and avoid additional confusion around the fact that functions obtained from an object are not bound to the object they are obtained from.