Skip to content

proposal: syscall/js: Reinstate js.Wrapper interface #50310

Closed
@abrander

Description

@abrander

js.Wrapper was removed when removing support for js.Wrapper in js.ValueOf() in 6c0daa7. In #39740 @finnbear suggested writing this proposal after I asked about the decision in #39740 (comment) and #39740 (comment).

I understand the reasoning for removing the case Wrapper: from js.ValueOf(), which makes good sense.

What I regret is the removal of the interface itself, it served a purpose besides the case in js.ValueOf(). It allowed packages and functions to signal very easily and precisely that they accept a "type backed by an underlying javascript object.".

js.Wrapper was perfect for this purpose, it provided glue for packages in the Go wasm community. We could write packages like this, and inter-package calling was well-defined:

package a

import (
	"syscall/js"
)

func DoStuffInJavascriptLand(obj js.Wrapper) {
	js.Global().Call("some_js_function", obj.JSValue())
}

Users of package a could satisfy this interface either by embedding js.Value or by satisfying the interface in another way. It was very convenient and ensured that all packages agreed allowing simple inter-package calling.

I propose the following:

  • Reinstate the js.Wrapper interface.
  • Reinstate the JSValue() method on js.Value.

This will solve this case of inter-package calling in the Go wasm community, and we can avoid packages defining their own (incompatible) interfaces for this purpose.

Activity

added this to the Proposal milestone on Dec 22, 2021
ianlancetaylor

ianlancetaylor commented on Dec 22, 2021

@ianlancetaylor
Contributor
neelance

neelance commented on Dec 22, 2021

@neelance
Member

@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

abrander commented on Dec 23, 2021

@abrander
Author

@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 ;)

cherrymui

cherrymui commented on Dec 23, 2021

@cherrymui
Member

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.

rsc

rsc commented on Jan 5, 2022

@rsc
Contributor

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

rsc

rsc commented on Jan 5, 2022

@rsc
Contributor

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

abrander

abrander commented on Jan 11, 2022

@abrander
Author

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.

cherrymui

cherrymui commented on Jan 11, 2022

@cherrymui
Member

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.

neelance

neelance commented on Jan 12, 2022

@neelance
Member

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.

abrander

abrander commented on Jan 12, 2022

@abrander
Author

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.

rsc

rsc commented on Jan 12, 2022

@rsc
Contributor

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.

22 remaining items

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @neelance@rsc@dmitshur@ianlancetaylor@abrander

        Issue actions

          proposal: syscall/js: Reinstate js.Wrapper interface · Issue #50310 · golang/go