msgpack/msgpack-javascript

`undefined` becomes `null` after deserializing

mischnic opened this issue · 5 comments

const msgpack = require('@msgpack/msgpack');

let original = {x: undefined};
let serialized = msgpack.encode(original, {extensionCodec});
let deserialized = msgpack.decode(serialized, {extensionCodec});
console.log(original, deserialized, original.x === deserialized.x);

prints "{ x: undefined } { x: null } false". This is a problem because now code checking for strict equality fails.

I was also not able to handle this in an extension codec because that is only called for non-primitive values.

ignoreUndefined: true doesn't work in all cases:

let original = undefined;
let serialized = msgpack.encode(original, {ignoreUndefined: true});
let deserialized = msgpack.decode(serialized, {ignoreUndefined: true});
console.log(original, deserialized, original === deserialized); // undefined null false

This is stated in the readme, but is there no way to retain this with an extension codec?

gfx commented

Can you elaborate on your use case? In general, it's not a good idea to distinguish null and undefined in JavaScript, and supporting it with extension types could lead to extra overheads, so I'd like to know your use cases are worth adding more overheads.

Once case that I found was where we serialize/deserialize an entry of process.env (so {key: "foo", value: process.env["foo"]}) and then check if deserialized.value !== process.env.foo (and this breaks if the value is undefined). It's easy to workaround, but I fear there might be more situations like this in the codebase that haven't surfaced yet.

(I'm using msgpack as a browser replacement for require("v8").(de)serialize)

May I suggest allowing for overriding this in a codec? Currently undefined is handled internally by this package, and you can configure whether to a) ignore it (not include it in the encoded buffer) or b) coerce it into null. I understand that this makes sense when communication between different languages, where there is no distinction between undefined and null, which is why it's nil in the spec.

If this could instead be allowed to be overridden though, users can encode it if they want.

My use case is to transfer an object from one place to another and maintain it exactly as is. Turning undefined into null alters the datatype, and allowing undefined to be there is different from not having it. Object.keys(obj) e.g.

genki commented

@gfx
Suppose to encode the entire arguments of functions that have optional ones.
The values of the optional arguments are not null but undefined.
If the msgpack coearses them into null, the information that they were not supplied is lost.
We have to provide the additional bitmask that informs which parameters were really supplied.
The undefined means the existence of the key in the object.
The null means the absence of the value. They can't be treated as same things.