Exported methods not available on named basic types

[mvdan]

I found this in my JS wrapping of https://godoc.org/mvdan.cc/sh/syntax. In particular, for a particular node, node.SomePos.String() works, but node.SomeOp.String() does not. The only difference appears to be that the former is a struct and the latter a basic type that can be translated as a float64 in JS.

Below is a minimised and self-contained reproducer:

$ cat main.go
package main

import (
        "fmt"

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

type Container struct {
        Basic  Basic
        Struct Struct
}

type Basic int

func (b Basic) String() string {
        return fmt.Sprintf("basic %d", b)
}

type Struct struct {
        Field int
}

func (s Struct) String() string {
        return fmt.Sprintf("struct { %d }", s.Field)
}

func main() {
        exps := js.Module.Get("exports")

        exps.Set("Basic", js.MakeFullWrapper(Basic(3)))
        exps.Set("Struct", js.MakeFullWrapper(Struct{4}))
        exps.Set("Container", js.MakeFullWrapper(Container{
                Basic: Basic(3),
                Struct: Struct{4},
        }))
}
$ cat test.js
const main = require('./main.js')

var struct = main.Struct
console.log(struct)
console.log(struct.String())

var basic = main.Basic
console.log(basic)
console.log(basic.String())

var struct = main.Container.Struct
console.log(struct)
console.log(struct.String())
var basic = main.Container.Basic
console.log(basic)
console.log(basic.String())
$ gopherjs build main.go && node test.js
{}
struct { 4 }
{}
basic 3
{}
struct { 4 }
3
/home/mvdan/test.js:17
console.log(basic.String())
                  ^

TypeError: basic.String is not a function
    at Object.<anonymous> (/home/mvdan/test.js:17:19)
    at Module._compile (internal/modules/cjs/loader.js:689:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Module.load (internal/modules/cjs/loader.js:599:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
    at Function.Module._load (internal/modules/cjs/loader.js:530:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:742:12)
    at startup (internal/bootstrap/node.js:279:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:752:3)

As you can see, this only happens when the type is hidden behind multiple levels in a js.MakeFullWrapper call, so it sounds like a bug to me.

[mvdan]

Forgot to mention - this is with d1c34fc540e3d132a301bf765b7a3beb3700a8dd built on go version go1.11.1 linux/amd64.

[myitcv]

Thanks - will take a look at what's going on here.

[mvdan]

I tried to have a look, but I couldn't wrap my head around the multiple levels of recursion and code generation :)

[myitcv]

Based on how MakeFullWrapper (and by extension, externalize, which it uses under the covers) is currently implemented, this is working as intended.

Reason being is that, per https://godoc.org/github.com/gopherjs/gopherjs/js, defined types which have an underlying type that is boolean, numeric or string, get externalize-d from Go to JavaScript as the JavaScript equivalent primitive type. No properties can be defined on values of these types.

var x = 5;
typeof x;            // "number"
x instanceof Number; // false
x["hello"] = "world";
console.log(x);      // 5

We could "fix" this by externalize-ing such values as instances of their JavaScript type equivalent:

var x = new Number(5);
typeof x;            // "object"
x instanceof Number; // true
x["hello"] = "world";
console.log(x);      // Number {5, hello: "world"}

But this would likely do more harm than good, because then where you previously (rightly) expected strict equality (===) of values you would no longer have such equality.

I suspect the thing to do here is document that this is the unfortunate result of an asymmetry between Go and JavaScript's notions of types/values.

We could define a function in github.com/gopherjs/gopherjs/js that takes a value of any defined type and returns an object with methods that are the method expressions of the defined type's methods... would that help? But I haven't really given this any thought.

Unless you see another solution?

[mvdan]

Thanks, that makes sense. I hadn't thought about basic types having this restriction, as they can't have "methods" over in the JS world.

Still, the example that I gave originally still puzzles me. Basic types get translated as objects with "methods" if they are passed to MakeFullWrapper directly, but as regular basic types with no methods if they're any number of levels below the passed value. Whatever MakeFullWrapper does, I think it should be consistent. I hadn't caught the bug in my JS package precisely because I assumed GopherJS would treat nested values and types just like the top-level ones.

[mvdan]

We could define a function in github.com/gopherjs/gopherjs/js that takes a value of any defined type and returns an object with methods that are the method expressions of the defined type's methods... would that help? But I haven't really given this any thought.

I don't think it would help that much in my case. This is about exposing a syntax tree, so what I'm after is really just translating each node and value in a way that makes sense, on a case-by-case basis. For example, operators are stringer enums, so they should likely be translated as just strings - which is neither an integer, nor an object type.

The long-term solution will be for me to implement my own custom "translate to JS object" code. There's other things that I'd like to behave in a custom way aside from this, so it just makes sense.

[myitcv]

I see what's going on here; thanks for that catch.

Keeping your original main.go, but substituting a different test.js:

const main = require('./main.js')

console.log("typeof main.Basic            ", typeof main.Basic)
console.log("typeof main.Container.Basic  ", typeof main.Container.Basic)

console.log("main.Basic === 3           ", main.Basic === 3)
console.log("main.Container.Basic === 3 ", main.Container.Basic === 3)

gives

typeof main.Basic             object
typeof main.Container.Basic   number
main.Basic === 3            false
main.Container.Basic === 3  true

MakeFullWrapper has inherited what I would consider broken behaviour from MakeWrapper by not panicking on values that have an underlying type of bool, numeric or string.

Your test.js was effectively giving you a false positive in that the values were showing as 3 in both cases.

So what we could here is to "break" MakeFullWrapper by not allowing types with an underlying of bool, numeric or string... or we could simply document this asymmetry in behaviour. I'm easy. You're the main/only user of this function :)