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: Add Truthy() and Falsy() to Value methods #28264

Closed
theclapp opened this issue Oct 18, 2018 · 15 comments
Closed

syscall/js: Add Truthy() and Falsy() to Value methods #28264

theclapp opened this issue Oct 18, 2018 · 15 comments
Labels
arch-wasm WebAssembly issues FeatureRequest FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@theclapp
Copy link
Contributor

What version of Go are you using (go version)?

go version go1.11 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

js/wasm


JS has the concept of "truthy" and "falsy". It'd be nice if these were part of syscall/js, either as part and parcel of Value.Bool(), or failing that something along the lines of

func (o Value) Falsy() bool {
	return !o.Truthy()
}

func (o Value) Truthy() bool {
	switch o.Type() {
	case TypeUndefined, TypeNull:
		return false
	case TypeBoolean:
		return o.Bool()
	case TypeNumber:
		if math.IsNaN(o.Float()) {
			return false
		}
		return o.Float() != 0
	case TypeString:
		return o.String() != ""
	case TypeSymbol, TypeObject, TypeFunction:
		return true
	}
	return true
}
@agnivade
Copy link
Contributor

/cc @neelance

@agnivade agnivade added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. FeatureRequest labels Oct 18, 2018
@agnivade agnivade added this to the Unplanned milestone Oct 18, 2018
@neelance
Copy link
Member

I'm not yet sure if it would be good to include it in syscall/js. For which APIs would you need the concept of "truthy"? Could you provide examples please?

@theclapp
Copy link
Contributor Author

I've been updating my Vue.js wrapper to use your wasm-sync-callbacks tree. I was trying to replicate an example from the Vue Guide (here, the bit that starts with "This will render the same result. We can also bind to a computed property that returns an object.") The JS in question is

function () {
    return {
      active: this.isActive && !this.error,
      'text-danger': this.error && this.error.type === 'fatal'
    }
}

So for this.isActive and this.error I wanted "truthy", for !this.error I wanted "falsy". I implemented that part of that example like this:

return hvue.Map2Obj(hvue.M{
	"active": hvue.Truthy(data3.Get("isActive")) &&
		hvue.Falsy(data3.Get("error")),
	"text-danger": hvue.Truthy(data3.Get("error")) &&
		data3.Get("error").Get("type").String() == "fatal",
})

where hvue.Map2Obj(hvue.M{...}) is similar to how GopherJS automagically transforms a GopherJS js.M{} into a JS object, and hvue.Truthy() and Falsy() are similar to the example code I posted at the top of this issue, except obviously not methods on js.Value. (See here.)

One could certainly argue that the absence of such a vague concept as "truthiness" in Go is (one of the reasons) why we use Go and not JavaScript, and I could certainly understand that, and agree that in most cases you shouldn't do it. But pragmatically I think there are clear use-cases for it.

So to sum up, when transliterating JavaScript to Go, since JavaScript uses truthiness a lot, I think it'd be handy and pragmatic for JS-transliterated-to-Go to be able to do the same, "out of the box".

I could see that you might not want Value.Truthy()/Falsy() in syscall/js (although obviously I think having them as methods on Value would be the most useful). There are other utility-functions that might also be handy, that shouldn't be in syscall/js. So perhaps a syscall/js/jsutil or top-level jsutil package would be appropriate? We discussed this a bit in the Gophers #webassembly channel (starting here), and one library with some nice utilities was pointed out: https://github.com/dennwc/dom/tree/master/js (permalink).

@myitcv
Copy link
Member

myitcv commented Oct 18, 2018

I'm not yet sure if it would be good to include it in syscall/js

I'd tend to agree; the implementation of "truthy" or "falsey" feels like something that should be left to the JavaScript VM; we would be reimplementing that logic which feels wrong.

@theclapp
Copy link
Contributor Author

we would be reimplementing that logic

Not necessarily. It could be

// Global JavaScript function
function isTruthy(o) {
  if (o) { return true } else { return false }
  // or even: return !!o
}
func (o Value) Truthy() bool {
   return Global().Call("isTruthy", o).Bool()
}

That said, the concept of "truthy", while admittedly a bit general, is actually pretty well-defined: "All values are truthy unless they are defined as falsy (i.e., except for false, 0, "", null, undefined, and NaN)". (In fact in my initial implementation of Truthy() & Falsy() I had Falsy() checking for false/0/etc, and Truthy was !Falsy, but then I inverted them.) My point being, I suspect that a pure-Go version would be faster and just as accurate as trampolining to JavaScript.

@dmitshur dmitshur added the arch-wasm WebAssembly issues label Oct 18, 2018
@DonMcNamara
Copy link

I suspect that a pure-Go version would be faster and just as accurate as trampolining to JavaScript.

It might be useful to have some hard numbers here. It seems like a lot of work is being done to speed up these types of calls.

@theclapp
Copy link
Contributor Author

It might be useful to have some hard numbers here

Happy to give it a shot, if @neelance (et al) think this is a reasonable feature in the first place. If they don't want to do it, it doesn't matter how fast it is. :)

@bradfitz
Copy link
Contributor

If the JS-inclined people generally agree this is a good idea, I don't object to adding one method. Two seems overkill, though. (e.g. we have utf8.ValidString but we don't also have utf8.InvalidString for the opposite)

@neelance
Copy link
Member

I am quite sure that the pure-Go version will be faster.

In JS, using truthiness is common, but I think this is mostly because it has the simplest syntax. Not using truthiness in JS just looks ugly quite often.

I've seen truthiness hiding bugs. Having a strong expectation about a type of a value is usually safer. However, there are use cases with legacy code that need truthiness.

I don't object to adding just Truthy either. I just hope it will get used sparingly.

@steveoc64
Copy link

I think in the case of porting legacy JS into a Go ecosystem, the Truthiness concept and the need to port this behaviour would appear often, and could be a point of confusion and or bugs.

In the interests of correctness on the Go side of the fence, it would be good if syscall/js provided a correct implementation for all to use, and make porting that little bit more attractive.

Having said that, I hope that I never ever have the need to use this syscall myself.

@agnivade
Copy link
Contributor

I am slightly leaning in favor of this. I suspect more people will ask for this as wasm gains in popularity. I would also prefer a pure-Go version since truthy-ness is quite well-defined.

@neelance
Copy link
Member

@theclapp Mind creating a CL? :)

@theclapp
Copy link
Contributor Author

@neelance Will do. It may be a few days. This'll be my first Go code contribution, so there's that first-timer learning curve to climb.

theclapp added a commit to theclapp/go that referenced this issue Oct 24, 2018
Truthy returns the JavaScript "truthiness" of the given value.  In
JavaScript, false, 0, "", null, undefined, and NaN are "falsy", and
everything else is "truthy".

Fixes golang#28264.
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/144384 mentions this issue: syscall/js: add Truthy() as a method on js.Value

theclapp added a commit to theclapp/go that referenced this issue Oct 26, 2018
Truthy returns the JavaScript "truthiness" of the given value. In
JavaScript, false, 0, "", null, undefined, and NaN are "falsy", and
everything else is "truthy".

Fixes golang#28264.
@gopherbot
Copy link
Contributor

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

gopherbot pushed a commit that referenced this issue Dec 18, 2018
Also update a Go 1 compatibility promise link to canonical URL.

Updates #27592
Updates #28264

Change-Id: I5994a0a63e0870c1795c65016590dfad829d26a7
Reviewed-on: https://go-review.googlesource.com/c/154618
Reviewed-by: Richard Musiol <neelance@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@golang golang locked and limited conversation to collaborators Dec 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly issues FeatureRequest FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

9 participants