Gopherjs: Inconsistent type conversion for *js.Object

Created on 19 Aug 2017  路  4Comments  路  Source: gopherjs/gopherjs

I've discovered that doing a type switch on a *js.Object behaves differently than an explicit type assert. Playground link

package main

import (
    "fmt"

    "github.com/gopherjs/gopherjs/js"
)

func main() {
    obj := js.Global.Get("Object").New()
    obj.Set("foo","bar")

    fmt.Printf("obj.foo = %s\n", obj.Get("foo").String())

    var x interface{} = obj
    switch t := x.(type) {
    case *js.Object:
        fmt.Printf("t.foo = %s\n", t.Get("foo").String())
    }

    if y, ok := x.(*js.Object); ok {
        fmt.Printf("y.foo = %s\n", y.Get("foo").String())
    }
}

Produces the following output:

obj.foo = bar 
t.foo = undefined 
y.foo = bar

where I would have expected:

obj.foo = bar 
t.foo = bar
y.foo = bar
NeedsFix bug

All 4 comments

Thanks for reporting. This looks like a clear bug.

I'll look to try and pick this up as part of #617

@myitcv In general, I would make the suggestion that we (all) aim to make PRs as small and self-contained as possible. That approach makes it easiest and fastest to review, merge, and revert if neccessary, spot issues, etc. On the other hand, large PRs are hard to review, slow to merge, hard to revert, etc.

Of course, you should use your judgement each time. If this change is significantly easier to fix as part of a bigger change, then it's fine. I just wanted to mention this point for awareness.

Thanks @shurcooL - completely agree with all your points.

I should have been clearer in my previous comment. I dug into the bug briefly and it looked like there was more overlap with #617 than not, hence my comment to pick it up as part of that issue and PR.

But having just looked further the fix is actually unrelated to #617. Pushing up a proposed fix in a separate PR shortly.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

marwan-at-work picture marwan-at-work  路  4Comments

4ydx picture 4ydx  路  3Comments

hajimehoshi picture hajimehoshi  路  6Comments

nytehauq picture nytehauq  路  7Comments

DapperDodo picture DapperDodo  路  6Comments