CC @neelance @hajimehoshi

@abrander Could you please provide some real world examples that show why it is not a good solution to simply get the js.Value at the caller?

a.DoStuffInJavascriptLand(obj.JSValue())

@abrander Could you please provide some real world examples that show why it is not a good solution to simply get the js.Value at the caller?

a.DoStuffInJavascriptLand(obj.JSValue())

If JSValue() still existed, I think we could get by. Then we could add a type Wrapper interface { JSValue() js.Value } to all packages where it was needed.

But to answer your question. We often do stuff like this:

package a

import (
	"syscall/js"
)

type Displayable interface {
	js.Wrapper

	Display()
}

type RevealContainer struct {
	js.Value

	children []Displayable
}

var _ Displayable = &RevealContainer{}

func NewRevealContainer() *RevealContainer {
	return &RevealContainer{
		Value:    js.Global().Get("revealContainer").New(),
	}
}

func (r *RevealContainer) Add(element Displayable) {
	r.children = append(r.children, element)
}

func (r *RevealContainer) Display() {
	for _, c := range r.children {
		r.Call("append", c.JSValue())
		c.Display()
	}
}

package b

import (
	"syscall/js"
)

type Widget struct {
	js.Value
}

var _ a.Displayable = &Widget{}

func NewWidget() *Widget {
	return &Widget{js.Global().Get("widget").New()}
}

func (w *Widget) Display() {
	w.Call("display")
}

And putting it all together:

package main

import (
	"a"
	"b"
)

func main() {
	c := a.NewRevealContainer()

	w1 := b.NewWidget()
	w2 := b.NewWidget()

	c.Add(w1)
	c.Add(w2)

	c.Display()
}

This produces very readable code that's easy to understand and write.

If every type implementing a.Displayable were to provide its own JSValue() (or similar) it will result in a lot of boilerplate - which I fear users will mitigate by wrapping syscall/js instead. Implementing the above example without any Wrapper-like interface will very quickly become cumbersome, and users will mitigate somehow.

Some convention need to exist for package interoperability. js.Wrapper provided that. Maybe the community will converge on a similar interface thou.

Maybe I'm missing something. Maybe I'm seeing monsters where none is to be seen ;)

Personally, I think the syscall/js package is rather a low level package (as the name "syscall" suggests), and it should include only things that directly interact with "the system". In this criteria, "solv[ing] this case of inter-package calling" is not such a low level thing. Maybe a package out of the standard library is a better place for this.

Is it possible to write this function / interface in a third-party package?
Or does it fundamentally have to be in syscall?

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

Is it possible to write this function / interface in a third-party package?
Or does it fundamentally have to be in syscall?

The Wrapper interface or similar can easily be added to a third-party package, as long as JSValue() or a similar method is available in syscall/js.

To implement JSValue(), a package must wrap js.Value in something, and then we're back to nothing being compatible.

I see three ways forward:

A: Reinstate both method and interface

Packages can embed the Wrapper-interface as they did in v1.17. All good.

B: Reinstate just the method

Packages can implement Wrapper themselves. All is okay.

C: Reinstate nothing

Packages cannot implement Wrapper without also implementing a type wrapping js.Value providing a JSValue() method. If packages wish for compatibility, they must all import the same wrapper or implement compatible wrappers. I fear this will be a hindrance to the Go-wasm ecosystem.

I can understand why the method and interface were removed. They both looked like a leftover from the undocumented feature removed from ValueOf(), but both the interface and the method found other uses along the way, and I strongly feel that A or B is the best way forward.

TL;DR:
Wrapper made it very simple to communicate that a type must be backed by a js.Value - or wrapped as the name suggested. We need at least a JSValue() method to have that again.

I think it does not fundamentally have to be in syscall. The Wrapper interface or the JSValue method is not used anywhere in the syscall/js package or anywhere in the standard library. And JSValue, being an identity function, doesn't provide any functionality that cannot be accessed otherwise.

A third party package could wrap a js.Value and has a JSValue method, and let other packages to use/wrap that type.

I believe the goal is to have a type that says "js.Value or a wrapper of js.Value". This is not really possible any more.

Maybe we could do without accepting a plain js.Value, so every value passed in needs to be a wrapper. This would be possible with an external package.

However, this might cause a lot of boilerplate, e.g. instead of &Widget{js.Global().Get("widget").New()} you would need to do &Widget{jswrapper.New(js.Global().Get("widget").New())}.

@abrander Maybe you could post a full example of how such a jswapper package would look like and how your example code above would look like.

I believe the goal is to have a type that says "js.Value or a wrapper of js.Value". This is not really possible any more.

Exactly! And I see interfaces like

type Thing interface {
	js.Wrapper

	Something() error
}

And as of Go 1.18beta1 we lost the ability to define interfaces like that without wrapping js.Value, which in effect means re-implementing syscall/js because js.Value is everywhere.

@abrander Maybe you could post a full example of how such a jswapper package would look like and how your example code above would look like.

This is an actual wrapper for syscall/js from a small project ported to Go 1.18. It's written to be a drop-in replacement for syscall/js, but I doubt it's really a drop-in replacement. The example above will look the same besides syscall/js will be replaced by internal/js.

package js

import (
	"syscall/js"
)

type Value struct{ js.Value }

type Func struct{ js.Func }

type Wrapper interface {
	JSValue() Value
}

var ValueOf = js.ValueOf

func FuncOf(fn func(this Value, args []Value) interface{}) Func {
	wrapper := func(realThis js.Value, realArgs []js.Value) interface{} {
		wrappedArgs := make([]Value, len(realArgs))

		for i, a := range realArgs {
			wrappedArgs[i] = Value{a}
		}

		return fn(Value{realThis}, wrappedArgs)
	}

	return Func{js.FuncOf(wrapper)}
}

func CopyBytesToGo(dst []byte, src Value) int {
	return js.CopyBytesToGo(dst, src.Value)
}

func Undefined() Value {
	return Value{js.Undefined()}
}

func Global() Value {
	return Value{js.Global()}
}

const TypeFunction = js.TypeFunction

func (v Value) JSValue() Value {
	return v
}

func (v Value) Get(p string) Value {
	return Value{v.Value.Get(p)}
}

func translateArgs(args []interface{}) []interface{} {
	realArgs := make([]interface{}, len(args))

	for i, a := range args {
		switch v := a.(type) {
		case Value:
			realArgs[i] = v.Value
		case Func:
			realArgs[i] = v.Func
		default:
			realArgs[i] = a
		}
	}

	return realArgs
}

func (v Value) Invoke(args ...interface{}) Value {
	return Value{v.Value.Invoke(translateArgs(args)...)}
}

func (v Value) Call(m string, args ...interface{}) Value {
	return Value{v.Value.Call(m, translateArgs(args)...)}
}

func (v Value) New(args ...interface{}) Value {
	return Value{v.Value.New(translateArgs(args)...)}
}

func (v Value) Equal(w Value) bool {
	return v.Value.Equal(w.Value)
}

func (v Value) Index(i int) Value {
	return Value{v.Value.Index(i)}
}

(Note the beautiful translateArgs() 😢)

Less code will do if the JSValue() method exists.

I'm sure there are other ways of making this possible, though.

It seems clear that js.ValueOf cannot contain a test for a JSValue method (named interface or not), because that leads to the performance problem that #44006 aimed to fix.

It seems like the minimum thing we'd need to do to satisfy the request in this issue would be to add a JSValue method back to js.Value.

But if we do put a JSValue method on js.Value, it seems very weird for js.ValueOf not to use it.

What would be the explanation for js.ValueOf not using a JSValue method? And how would we make sure it didn't get added back? Just a wart for performance reasons?

The question seems to be: how often does this come up?
@abrander seems to think that this happens often, while @neelance does not.
Do we have any data about why we need to support this?

Third-party packages can define their own interfaces and agree on the method name and signature without coordination. For example if they agree on interface { JSValue() js.Value } then that should be enough, much like there are multiple definitions of fmt.Stringer floating around but anything that has String() string implements all of them.

But if we do put a JSValue method on js.Value, it seems very weird for js.ValueOf not to use it.

Yep. Very weird indeed.

The question seems to be: how often does this come up?

I believe it's not an issue right now (!). Go-wasm projects are fragmented, and the community hasn't consolidated on anything yet. But I have a feeling it will matter shortly.

Third-party packages can define their own interfaces and agree on the method name and signature without coordination. For example if they agree on interface { JSValue() js.Value } then that should be enough, much like there are multiple definitions of fmt.Stringer floating around but anything that has String() string implements all of them.

And that's where I think we'll end up. Or projects implement wrappers for syscall/js, adding things like Promise wrappers while they are at it.

fmt.Stringer being in the standard library provides a powerful incentive to implement it, by the way. I would love a similar incentive for JS-backed types, but I can understand the pushback.

Referring back to my last comment, we have:

(1) js.ValueOf cannot contain a test for a JSValue method [performance]
(2) adding a JSValue method to js.Value would really imply that js.ValueOf should use it.

The implication then seems to be that we shouldn't add a JSValue method to js.Value.

Especially since @abrander says this is not causing any problems right now, let's leave it out for Go 1.18.
We can always add something later.

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

No change in consensus, so declined.
— rsc for the proposal review group