js-yaml
js-yaml copied to clipboard
Trying to use an object as a key results in the string '[object Object]' as a key
If I throw the following markup (which I believe is not valid YAML, though I'm not even remotely an expert) into the online YAML parser at http://yaml-online-parser.appspot.com/ ...
{{"foo": "bar"}: {"qux": "baz"}}
... then I get:
ERROR:
while constructing a mapping in "
", line 1, column 1: {{"foo": "bar"}: {"bla": "bla"}} ^ found unacceptable key (dict objects are unhashable) in " ", line 1, column 2: {{"foo": "bar"}: {"bla": "bla"}}
If I throw it into yaml.safeLoad, on the other hand, I don't get an error. Instead I get this:
> yaml.safeLoad('{{"foo": "bar"}: {"bla": "bla"}}')
{ '[object Object]': { bla: 'bla' } }
Whatever the correct behaviour here is (and I defer that question to those more knowledgable than me), this is plainly not it.
I guess, it should throw, because JSON is invalid.
which I believe is not valid YAML
It is valid.
http://yaml-online-parser.appspot.com/
It is not, obviously. YAML allows to use anything as a mapping key. However, JS-YAML loads YAML mappings into JS objects, and we have to convert YAML key objects into strings. This will be fixed in future with Map support.
The YAML being valid doesn't mean this should be closed, surely? Even if it's valid YAML, the behaviour here isn't useful; throwing an error indicating that this is valid YAML but js-yaml can't parse it seems more reasonable?
There are other options too, like parsing it into some structure other than an Object or taking an argument that indicates whether to throw on object keys or not. But silently turning object keys into "[object Object]" by default seems like it's going to be the wrong thing for the majority of users.
Sorry. I closed this issue because that behavior of JS-YAML is really old and even described in the README. However, you have a good point, and it's time to fix the problem. We've just discussed this internally, and here is the plan:
- If
Mapis available, use it for mappings with non-string keys. - Otherwise throw a warning (exception by default) and continue with the old behavior.
I doubt this problem can be solved easy, because maps have completely different interface, and it will be very hard to make both (objects + map) support AND fast implementation. May be we could add maps support for explicits, but i should be sure, that it's really needed.
I'd suggest this behaviour:
- throw warning
- if intercepted - use stringified value for maps.
That's enougth to show clear that shit happened.
Yeah, I'm not too familiar with ES6 maps but given the difference in interface it seems that using objects where possible and maps only where non-string keys are needed would be pretty unhelpful and confusing behaviour. And swapping to maps altogether, besides being a significant breaking change, is current inviable for anybody who needs to support environments that don't have maps.
I'd guess that the ideal thing for you to do would be:
- Add a parsing option, off by default, that causes JavaScript maps instead of objects to be used to represent YAML maps
- Throw a warning if that option is not set and a YAML map with an object key is encountered during parsing. The warning should probably note that using the maps option will allow the YAML to be parsed properly.
You missedt the main question - why maps support should be added at all :) . If nobody use it - no reasons to spend time for adding support.
Something else relevant: using an array as a key is apparently legal YAML, but results in a string representation of the array being used as the key.
@puzrin:
You missedt the main question - why maps support should be added at all :) . If nobody use it - no reasons to spend time for adding support.
that's ultimately a call for you guys, and you're probably better qualified to make it than me, but let me toss out my thoughts for whatever it's worth.
-
I find it hard to predict whether use of objects and arrays as keys will take off in JavaScript once we all have
Maps. But there are reasonable uses of that functionality. In the Python world you can use tuples as keys and that's often useful. For example, suppose you have a game with an infinitely large, sparsely-populated 2D game board, where each tile can either be empty or contain a playing piece. In Python you could represent the board's contents as a mapping of 2-tuples to pieces, like{(0,0): "something", (6,8): "something else", (1000001, -12345): "some other thing"}. I can't think of an idiomatic way of representing the same data in today's JavaScript that is quite as clean as the Pythonic way, but once we haveMaps everywhere it would be possible to do things the same way as in Python usingMaps withArrayorObjectkeys.And if such usage becomes common, then we'll probably also want to serialise those data structures, and JSON will be inadequate for the task.
-
Using Maps instead of Objects also has the advantage of guaranteeing key ordering, since YAML and JavaScript maps are ordered but JavaScript objects are (technically) not. There are plenty of situations where I might like to use an ordered map, like having a map of inputs to expected outputs representing test cases for some function and wanting to loop over them and run the tests in a sensible order. And again, if the data structure is potentially useful, then I potentially might want to serialise it or load it from a file.
-
Hard use cases aside, doesn't it just feel wrong to you for a YAML-parsing library to only be able to parse a subset of YAML?
I can understand classifying map support as low priority, and it might make sense to add the warning without the Map support for now, but it seems to me like it would be very strange to actively decide against adding it.
I have no motivation to spend resources for adding native maps support. If someone wish to do it - PR will be accepted. AFAIK, even in current state js-yaml is significantly better than alternative implementations. So, i'd prefer to focus on another projects.
I just ran into this. I am trying to use YAML as a kind of language-independent graph serialization protocol. I have a graph of arbitrary Java objects on a server that I want to display over the web. Those server-side objects include Maps that use Objects as keys.
For what it's worth, I took at stab at replacing use of Object with Map in the loader. I'm not sure that I got it completely correct, I did not measure performance, I did not worry about compatibility, I did not make it an opt-in change, etc., but the primary work took less than an hour. It got me past the duplicate key problem resulting from use of String(object), but then I hit an aliasing problem and I don't know whether it was related. Ultimately I don't think I'll be continuing with YAML.
I also ran on this issue here. I thought about allowing users to represent a request object with key/value pairs that could be either function or scalars like:
? !someTypeBecomingAJsFunction someData : someValue
? someKey : !someTypeBecomingAJsFunction someData
I read the YAML specification and was ecstatic to find out it supported my strange use case out of the box.
After running the YAML parser I could easily transform this into a function that if called with arguments produces an object representing the intended structure.
But I also agree that the default object behavior is better for ease of use (the Map API is sadly not accepting . and [] operators which would have allowed them to act as drop in replacement for objects). I believe the correct way to do this would be to opt into map representation using an option as suggested by @ExplodingCabbage. It should throw properly if opt in was not done and a key that is not a string comes along during parsing.
- Add a parsing option, off by default, that causes JavaScript maps instead of objects to be used to represent YAML maps
- Throw a warning if that option is not set and a YAML map with an object key is encountered during parsing. The warning should probably note that using the maps option will allow the YAML to be parsed properly.
This sounds like the best approach. Now we just need somebody to implement it 😛
Now we just need somebody to implement it
...and without significant speed loss for legacy mode.
I created a little script to compare how different JavaScript based YAML parser handle this issue: https://gist.github.com/adius/0bb08111c16a15fb34c68b5a24b817d0