glenjamin / transit-immutable-js

Transit serialisation for Immutable.js
MIT License
250 stars 31 forks source link

`undefined` Map key is serialized to `null` #33

Open nkbt opened 7 years ago

nkbt commented 7 years ago

Codepen: http://codepen.io/nkbt/pen/xgebxL

This code

const {toJSON, fromJSON} = pkgzip.transitImmutableJs;
const {Map} = pkgzip.immutable;

document.querySelector('#app').innerHTML = `
  toJSON(Map().set(undefined, 'whatever')): ${toJSON(Map().set(undefined, 'whatever'))}
  toJSON(Map().set(null, 'whatever')):      ${toJSON(Map().set(null, 'whatever'))}
`;

Resulted in

  toJSON(Map().set(undefined, 'whatever')): ["~#iM",[null,"whatever"]]
  toJSON(Map().set(null, 'whatever')):      ["~#iM",[null,"whatever"]]
nkbt commented 7 years ago

Not like I am trying to abuse this library (though probably it looks like I very much do...), but just spent another hour debugging one edge case, where we rely on key being undefined.

glenjamin commented 7 years ago

There is no undefined in JSON, so there's not much I can do about this unfortunately.

nkbt commented 7 years ago

It is possible.

Something like:

function __UNDEFINED () {}
const value = typeof input === 'undefined' ? new __UNDEFINED() : value

But I feel like it may be done on transit-js level...

glenjamin commented 7 years ago

I'm not following that last post - the json spec doesn't include undefined, so short of another fancy encoding I'm not really sure what your options are.

Can you change the code so you don't rely on undefined and null needing to be distinct keys?

nkbt commented 7 years ago

Sure, I've changed the code first thing, so this is definitely not a blocker. And yes jdon does not support undefined. But I assumed that transit is meant to be used to overcome json limitation on transferring valid JS structures, so from that prospective this issue still looks valid to me

glenjamin commented 7 years ago

Ah, I see what you mean - yeah.

I think transit mostly expects to deal with classes and instances, but the default fallback handler might be a way to encode undefined.

lukaswelinder commented 7 years ago

@nkbt This might help you: https://github.com/deanlandolt/bytewise

nkbt commented 7 years ago

@lukaswelinder that's really cool, thank you!

lukaswelinder commented 7 years ago

@nkbt Glad I could help!

Ran into this issue at work. We built out an Immutable.js data store w/ type and order persistence on top of LevelUP and Thread.js and needed a way to serialize Immutable structures for both the DB and to pass messages on Thread.js.

In short, our solution was to break down structures into keyPath (arrays) serialized using bytewise w/ our own semantic for identifying structure type.

The result takes up more memory than transit-immutable-js however, so you would probably want to only use it for undefined.

nkbt commented 7 years ago

So far the easy solution was to switch to null instead. It did not make too much difference for the code around it.

But I see bytewise lib as potentially very helpful in some near future for our projects.