golang / go

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

syscall/js: unclear what the zero js.Value represents #27592

Closed dmitshur closed 5 years ago

dmitshur commented 5 years ago

The godoc of js.Value type currently says:

Value represents a JavaScript value.

It's not easy to figure out what the zero value represents, even after reading the rest of syscall/js docs.

This question came up when I had a function with the return types (js.Value, error) and I wanted to figure out what js.Value to return when an error happened. Typically, one returns the zero value, but it helps to understand what the zero value of that type represents.

From looking at the implementation, I understand that the zero js.Value value currently represents the JavaScript number zero, is that right @neelance? I.e.:

js.Value{} == js.ValueOf(int(0))   // true
js.Value{}.Type() == js.TypeNumber // true

It's also possible that what the zero js.Value value represents is considered an internal implementation detail and not meant to be a part of the public API. Hence, users shouldn't rely on it to have any well-defined and stable meaning.

I think it would be helpful to document what the zero value represents, so people can answer this question by reading the documentation. Depending on the intention, perhaps one of these:

// Value represents a JavaScript value.
// The zero value for a Value represents the JavaScript number 0.
type Value ...

(See https://godoc.org/math/big#Int documentation for a similar example.)

Or (an improved version of):

// Value represents a JavaScript value.
//
// The zero value for Value is undefined. It is not intended for direct use
// by clients, other than to signify that the value is not defined.
type Value ...

/cc @neelance

neelance commented 5 years ago

Yes, currently the zero js.Value is the number 0. However, I haven't explicitly defined it yet, because it was just a result of the current encoding of js.Value. We could either keep the 0 and add documentation, or we could decide to add a special case to the encoding so the zero js.Value would map to js.Undefined(). Open to debate.

myitcv commented 5 years ago

Choosing one option so it's clear what I'm 👍-ing:

we could decide to add a special case to the encoding so the zero js.Value would map to js.Undefined().

Agreed.

dmitshur commented 5 years ago

Open to debate.

I have a relevant mini-experience-report to share.

In the WebGL 1 spec, two of the functions are defined as follows:

WebGLProgram? createProgram()

void useProgram(WebGLProgram? program)

The ? means createProgram() can return null instead of a reference to a WebGLProgram object if it fails, and that it's valid to pass null to useProgram to unset the program. From OpenGL ES spec:

If UseProgram is called with program set to zero, then the current rendering state refers to an invalid program object, and the results of vertex and fragment shader execution due to any DrawArrays or DrawElements commands are undefined.

This is sometimes used during development to reset state after a program is done being used, to avoid accidentally using it elsewhere. E.g.:

gl.useProgram(program)
// use program
gl.useProgram(null)

I was working on porting some cross-platform gl and glfw packages to add WebAssembly support.

We know it's a good practice in Go to make the zero value useful. So, in the gl package, an invalid program is represented by the zero value of gl.Program:

program := gl.CreateProgram()
if !program.Valid() {
    return fmt.Errorf("no programs available")
}

gl.UseProgram(program)
// use program
gl.UseProgram(gl.Program{})

Inside the gl package, for GopherJS, the Program type and UseProgram function are implemented roughly as:

type Program struct {
    program *js.Object
}
func (p Program) Valid() bool { return p.program != nil }

func UseProgram(p Program) {
    c.Call("useProgram", p.program)
}

If I were to implement the WebAssembly version analogously, it would look like this:

type Program struct {
    program js.Value
}
func (p Program) Valid() bool { return p.program != js.Null() }

func UseProgram(p Program) {
    c.Call("useProgram", p.program)
}

One immediately obvious problem with that is that the zero Program value would report that it's valid, since p.program != js.Null() will be true.

The other problem is that gl.UseProgram(gl.Program{}) would cause useProgram to be called with a JavaScript number 0 rather than null, which causes:

panic: JavaScript error: Failed to execute 'useProgram' on 'WebGLRenderingContext': parameter 1 is not of type 'WebGLProgram'.

So I'd actually need to use the following code as a work around:

type Program struct {
    program js.Value
}
func (p Program) Valid() bool { return p.program != js.Null() && p.program != js.Value{} }

func UseProgram(p Program) {
    // Workaround for js.Value zero value.
    if p.program == (js.Value{}) {
        p.program = js.Null()
    }
    c.Call("useProgram", p.program)
}

It seems that these complications would go away if the zero value of js.Value were to be null.

myitcv commented 5 years ago

@dmitshur

It seems that these complications would go away if the zero value of js.Value were to be null.

I think the WebGL usage (and your description) corresponds well with: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/null#Description

But I think the primitive value undefined is a better fit for the zero value, along the lines of the description here https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/undefined#Description:

i.e. it is the "default" value in all situations in JavaScript. By contrast, null is a value that is assigned and has meaning.

dennwc commented 5 years ago

I agree with @myitcv regarding zero value that should to mapped to undefined. See #28174 for an example.

gopherbot commented 5 years ago

Change https://golang.org/cl/143137 mentions this issue: syscall/js: make zero js.Value represent "undefined"

gopherbot commented 5 years ago

Change https://golang.org/cl/154618 mentions this issue: doc/go1.12: add notes for syscall/js CLs 141644, 143137, 144384