Add optional support for using JS Map object for MessagePack maps

[cvermilion]

Unlike JavaScript objects, Maps, like MP maps, can have arbitrary values as keys (not just strings). Map is an ES6 addition, I believe, so not all platforms will have it. But the latest browsers and Node all support it.

The first commit is an unrelated bug in unpacking fixed-length extension types; I found it while working on this. I can break that out as its own PR if desired.

[kawanet]

Thank you for the contribution.

I'd like to ask you a small modification as below:

function map(decoder, len) {
  var usemap = decoder.options && decoder.options.usemap && HAS_MAP;
  return usemap ? map_to_map(decoder, len) : map_to_obj(decoder, len);
}

The decode-time rule like above would add a tiny cost compared to before. Please see how binarraybuffer option works. It's an option for createCodec.

function getReadFormat(options) {
  var binarraybuffer = HAS_UINT8ARRAY && options && options.binarraybuffer;
  var readFormat = {
    map: map,
    array: array,
    str: str,
    bin: (binarraybuffer ? bin_arraybuffer : bin_buffer),

It means it checks binarraybuffer option on the createCodec-time but not on each decode-times. I guess your usemap option would work as well as binarraybuffer does. It'd something like below:

function getReadFormat(options) {
  var binarraybuffer = HAS_UINT8ARRAY && options && options.binarraybuffer;
  var usemap = HAS_MAP && options && options.usemap;
  var readFormat = {
    map: (usemap ? map_to_map : map_to_obj),

I prefer the small trick which costs a bit less, as it's one of the keys to keep the encoding/decoding speed when adding some new features.

[kawanet]

By the way, I have a question (not a request).
Do you need to supporting Set, WeakMap, WeakSet in addition to Map?

[cvermilion]

Thanks for the feedback! I've restructured the options check, let me know if that's what you had in mind. I've also tried to fix the tests for running on platforms with a Map object, since I noticed they were failing some of the Travis checks.

Finally, I added a test for the unrelated bugfix in c78ebd9; this test fails on current master but passes here.

[cvermilion]

Oh, and I don't need Set, WeakMap, or WeakSet. They also don't have a direct analogue to any MessagePack native types, so I would see less cause to support them natively in this library.

[kawanet]

I've also tried to fix the tests for running on platforms with a Map object, since I noticed they were failing some of the Travis checks.

It seems that Sauce's browser test platform itself is unstable and breaking sometimes, by the way.

They also don't have a direct analogue to any MessagePack native types, so I would see less cause to support them natively in this library.

I see.

[kawanet]

Sauce's test failure around 16.binarraybuffer.js will be fixed by d3bed93 which is not merged yet as I was too busy for those days. Could you please do git cherry-pick d3bed93 on your branch?

[kawanet]

It seems all tests pass except for some failures which are caused by Sauce platform.

I'll be merging it.

+var isBrowser = ("undefined" !== typeof window);
+var Map = isBrowser && window.Map || global.Map;
+var HAS_MAP = ("undefined" !== typeof Map);

Do you have any reason to add those three lines?
HAS_MAP check is necessary but other two are not, I guess.

+var HAS_MAP = ("undefined" !== typeof Map);

Map should be added to globals at package.json to pass jshint.

  "jshintConfig": {
    "es3": true,
    "globals": {
      "JSON": true,
+     "Map": true,
      "Symbol": true,
      "window": true
    },
    "mocha": true,
    "node": true,
    "undef": true
  },

[cvermilion]

Do you have any reason to add those three lines? HAS_MAP check is necessary but other two are not, I guess.

Ah, you're right! I'd misinterpreted the jshint output as an error running the tests. I'm still pretty new to JS and I'll admit I'm still a bit confused about how different runtimes handle globals.

Dropping the lines and adding the jshint config works for me locally; I've pushed the change to my branch. Since you've already merged this, I can do another PR if you like.