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

Map is empty #368

Closed
bravedonquixote opened this issue Jan 13, 2022 · 5 comments
Closed

Map is empty #368

bravedonquixote opened this issue Jan 13, 2022 · 5 comments

Comments

@bravedonquixote
Copy link

Hello,

We are getting an empty map in Go from "new Map([['a',1], ['b', 2]]);" in the following one liner script. We've also tried keys() to see what's going on but that also turns out to be an empty map. We would appreciate your expertise on this matter. Thanks.

  • Testcase go code

package main

import (
"fmt"
"github.com/dop251/goja"
)

func main() {
vm := goja.New()
r, _ := vm.RunString(m = new Map([['a',1], ['b', 2]]);)
v, _ := r.Export().(map[string]interface{})
fmt.Printf("Map is empty: %#v\n", v)
}

  • Execution result
    Map is empty: map[string]interface {}{}
@bravedonquixote
Copy link
Author

bravedonquixote commented Jan 13, 2022

Here's a simpler code without type casting:

vm := goja.New()
r, _ := vm.RunString("var m = new Map([['a',1], ['b', 2]]);m;")
v := r.Export()
fmt.Printf("Map is still empty: %#v\n", v)

@dop251
Copy link
Owner

dop251 commented Jan 15, 2022

Currently Export() for Maps works as for any regular Objects, i.e. exports enumerable properties which there are none. Same way Object.keys(m) would return an empty array.

The problem here is similar to what I described in #336, however I gave it a little bit of thought and I think something like this could work:

  • Export() for Map and Set will return goja.Map and goja.Set which will contain regular access/manipulation methods (Get/Set/Has/Delete/Add). Similar to how it's currently done for Proxy or Promise.
  • ExportTo() for Map will export the Map's contents into the specified typed map.
  • ExportTo() for Set will export the Sets contents into either a map (where values will be set to zero) or a slice.

Note that ExportTo() will panic if the value used as a map key is not hashable (same way as reflect.Value's SetMapIndex() would panic).

What do you (or anyone else interested) think?

@bravedonquixote
Copy link
Author

bravedonquixote commented Jan 16, 2022

Hello Dmitry,

I admire your work and appreciate that you took time to offer your insightful advice.
Here's what we have found after some experiments:

  1. Casting to *goja.Object revealed the regular access/manipulation methods such as 'has' and 'get' at which we were very excited. Then we realized that the challenge we have is that we do not know the keys in advance in order to utilize 'get' or 'has'.

  2. We were delighted to try out ExportTo(). Then we were saddened by the result which showed an empty interface{}:

Code
value, _ := vm.RunString("var m = new Map([['a',1], ['b', 2]]);m;")
var exportTo map[string]interface{}
err := vm.ExportTo(value, &exportTo)
if err == nil {
fmt.Printf("ExportTo: %#v\n", exportTo)
}
Result
ExportTo: map[string]interface {}{}

We wish there were a way to get the keys so that we could at least utilize 'get' that you advised in order to access the values. We would appreciate your expertise on this matter.

Best regards,
Nicholas

@dop251
Copy link
Owner

dop251 commented Jan 17, 2022

First of all I highly recommend you to read the README and the reference documentation. It's not the best in the world, but it's there and at least it has all public methods mentioned.

What I mentioned in the previous comment was a proposal, none of it it implemented right now, and I have doubts about what would be the best approach, especially for the Export(). The problem with the approach I mentioned is that if you override Map methods (such as get() or has()), it will be tricky to make goja.Map.Get() and goja.Map.Has() use those. It also raises a question about exception handling (your custom methods can throw, whereas default Map methods cannot). Also, as you've mentioned, you can still use Map's methods by converting the value to *Object, so this wouldn't add any new functionality.

Another possibility would be that Export() return map[interface{}]interface{}. However it's not possible for a map to have non-hashable keys (for example a slice or a struct with a slice field). What's more, there is no easy way to tell if a type is hashable and there is no 'safe' SetMapIndex() equivalent, one that would return an error instead of panicking. But even if there was, Export() is supposed to always work, it does not return an error. And having a possibility of Export() panic on certain values doesn't sound good to me.

Another option could be returning a list of entries in the form of [[key, value], ...] as [][]interface{}. This would make it somewhat more useful than the current version.

As for your question about map keys, see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/keys. I might add a method to simplify using ES iterators from Go (i.e. ForOf(iterable Value, step func(v Value) (continue bool, err error)) error).

Another advice would be to consider whether you really need to use Map for values exported to Go at all. Using a simple Object may be an easier option.

@bravedonquixote
Copy link
Author

Hello Dmitry,

Thank you for recommending the README and the reference documentation which have been very helpful.

[][]interface{} is a fantastic idea. We hope it can be implemented soon. Unfortunately our requirement is to use Map instead of a simple Object.

We appreciate your help and your wonderful work.

Best regards,
Nicholas

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

No branches or pull requests

2 participants