Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

syscall/js: the name "Callback" does not fit to all of its use cases #28711

Closed
neelance opened this issue Nov 10, 2018 · 10 comments
Closed

syscall/js: the name "Callback" does not fit to all of its use cases #28711

neelance opened this issue Nov 10, 2018 · 10 comments
Labels
arch-wasm WebAssembly issues FrozenDueToAge

Comments

@neelance
Copy link
Member

neelance commented Nov 10, 2018

Wikipedia says:

In computer programming, a callback, also known as a "call-after" function, is any executable code that is passed as an argument to other code that is expected to call back (execute) the argument at a given time.

This does not apply to all use cases of js.Callback, for example it may be used to register an event listener.

In #26045 the alternative names js.Callable and js.Function have been suggested. While js.Function seems like the simplest name, it suffers from being too common, so the documentation becomes hard to read because the term "function" does not only identify js.Function.

If we want to change the name, we should do so for Go 1.12 because the signature of js.NewCallback already got changed in this cycle, so we should do this breaking change in the same cycle.

(syscall/js is experimental and exempt from Go's compatibility promise)

@neelance
Copy link
Member Author

/cc @theclapp @myitcv

@theclapp
Copy link
Contributor

Summary: I like type Callable and func NewCallable.


This is a surprisingly difficult issue to nail down. Lots of things are in play: Ease of usage of documentation, point of view, being accurate, and being succinct, to name a few.

  • I agree that the many places the word "function" is used would make NewFunction problematic.
  • If I throw out my desire for succinctness, I like NewCallableFromJS, which is accurate but a mouthful.
  • From the point of view of JavaScript, a Go function is "foreign" or "external", so NewForeign or NewExtern kind of strike my fancy. But obviously we're writing Go and will tend to look at things from the Go point of view, so I also like NewNative or NewNativeFunc. But a new native function in Go is of course normally just func ..., so I don't like any of these four enough to actually advocate.

All of that leads me to prefer Callable as the type and NewCallable as the constructor.

@deanveloper
Copy link

I like Callable as well. My only concern is that the "-able" suffix is typically used for interfaces in many languages. I'm not sure how I feel about Native/NativeFunc. I liked JSFunc in my head, until I realized that the external notation would be js.JSFunc, which isn't ideal.

I like Callable though. And it's definitely better than Callback.

@myitcv
Copy link
Member

myitcv commented Nov 15, 2018

I'm not great when it comes to naming things 😄

  • Callback is too specific to my mind to one particular use case for such values
  • Function will, as you say, clash/look too similar to "function"
  • Similarly, Func (memories of GopherJS's MakeFunc) will clash/look too similar to func
  • WrappedFunc is the closest as far as naming what such a value actually represents, but is perhaps a bit cumbersome

cc @bradfitz for some better name ideas

@theclapp
Copy link
Contributor

@myitcv So you don't like Callable either?

@myitcv
Copy link
Member

myitcv commented Nov 15, 2018

@theclapp

So you don't like Callable either?

I prefer WrappedFunc. Because then you could have:

func Wrap(func(this Value, args []Value) interface{}) WrappedFunc

which would then be used as js.Wrap(...)

@deanveloper
Copy link

func Wrap(func(this Value, args []Value) interface{}) WrappedFunc

Ohh, I like this

@neelance
Copy link
Member Author

What does js.Wrap wrap? A value? A function? It should at least be js.WrapFunc, but this is still a bit inconsistent with js.ValueOf and js.TypedArrayOf.

But this got me thinking: The problem around js.Function isn't as bad if you call it wrapped function in the documentation. Here's a draft:

// FunctionOf returns a wrapped function.
//
// Invoking the JavaScript function will synchronously call the Go function fn with the value of JavaScript's
// "this" keyword and the arguments of the invocation.
// The return value of the invocation is the result of the Go function mapped back to JavaScript according to ValueOf.
//
// A wrapped function triggered during a call from Go to JavaScript gets executed on the same goroutine.
// A wrapped function triggered by JavaScript's event loop gets executed on an extra goroutine.
// Blocking operations in the wrapped function will block the event loop.
// As a consequence, if one wrapped function blocks, other wrapped functions will not be processed.
// A blocking function should therefore explicitly start a new goroutine.
//
// Function.Release must be called to free up resources when the function will not be used any more.

So maybe we can have js.Function and js.FunctionOf?

@neelance neelance self-assigned this Nov 21, 2018
@gopherbot
Copy link

Change https://golang.org/cl/153559 mentions this issue: syscall/js: rename js.Callback to js.Function

@gopherbot
Copy link

Change https://golang.org/cl/154059 mentions this issue: doc/go1.12: add note about CL 153559's syscall/js.Callback rename

gopherbot pushed a commit that referenced this issue Dec 13, 2018
Updates #28711

Change-Id: I03139a394fdf0540db07d6d1e38b3fa223b06d58
Reviewed-on: https://go-review.googlesource.com/c/154059
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators Dec 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly issues FrozenDueToAge
Projects
None yet
Development

No branches or pull requests

6 participants