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

Add option to prefer decoding as Map #95

Merged
merged 5 commits into from
Mar 2, 2021

Conversation

ninevra
Copy link
Contributor

@ninevra ninevra commented Feb 28, 2021

This PR adds an option, preferMap, which when set causes all MessagePack maps to be decoded to javascript Maps.

The preexisting default behavior is to decode as Map if non-string keys exist and as a plain object otherwise. This means that decode(encode(new Map()) is {}, i.e., Maps do not round-trip reliably. It also prevents the user from reliably determining the order of entries in the encoded MessagePack map. (It's not clear to me if MessagePack intends users to care about map order, but the spec doesn't appear to prohibit it.) An example:

const {decode, encode} = require('msgpack5')();
const map = new Map()
  .set('b', 1)
  .set('1', 2)
  .set('01', 3)
  .set('0', 4)
  .set('a', 5);
const buffer = encode(map); // <Buffer 85 a1 62 01 a1 31 02 a2 30 31 03 a1 30 04 a1 61 05>
const object = decode(map);
const keys = Object.keys(object); // ['0', '1', 'b', '01', 'a']

The Map's order is reflected by the encoded data but replaced by js object traversal order in the decoded data.

With preferMap: true, decode(encode(new Map()) is new Map(), and the example above decodes to a Map with key ordering 'b', '1', '01', '0', 'a'.

Copy link
Owner

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina merged commit 9f2841c into mcollina:master Mar 2, 2021
This was referenced Mar 12, 2021
This was referenced Mar 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants