ryansolid / dom-expressions

A Fine-Grained Runtime for Performant DOM Rendering
MIT License
863 stars 125 forks source link

Serializer fails on recursive objects #201

Closed lxsmnsyc closed 1 year ago

lxsmnsyc commented 1 year ago

I was writing my own serializer when I learned that DX has its own serializer, so I tried it out since I thought it was sophisticatedly written. Turns out, DX's serializer fails at some scenarios. For example:

Case A (using Maps):


const a = new Map();
const b = new Map();

a.set('b', b);
b.set('a', a);
a.set('a', a);
b.set('b', b);

console.log(mySerialize({ a, b }));
console.log(dxSerialize({ a, b }));

My serializer:

((h,j,k)=>(k={a:h=new Map([["b",j=new Map]]),b:j},j.set("a",h).set("b",j),h.set("a",h),k))()
{
  a: <ref *1> Map(2) {
    'b' => <ref *2> Map(2) {
      'a' => [Circular *1],
      'b' => [Circular *2]
    },
    'a' => [Circular *1]
  },
  b: <ref *2> Map(2) {
    'a' => <ref *1> Map(2) {
      'b' => [Circular *2],
      'a' => [Circular *1]
    },
    'b' => [Circular *2]
  }
}

DX:

(function(h,j){return {a:h=new Map([["b",j=new Map([["a",h],["b",j]])],["a",h]]),b:j}}())
{
  a: Map(2) {
    'b' => Map(2) { 'a' => undefined, 'b' => undefined },
    'a' => undefined
  },
  b: Map(2) { 'a' => undefined, 'b' => undefined }
}

Case B (using Sets):

const a = new Set();
const b = new Set();

a.add(b);
b.add(a);
a.add(a);
b.add(b);

console.log(mySerialize({ a, b }));
console.log(dxSerialize({ a, b }));

My serializer:

((h,j,k)=>(k={a:h=new Set([j=new Set]),b:j},j.add(h).add(j),h.add(h),k))()
{
  a: <ref *1> Set(2) {
    <ref *2> Set(2) { [Circular *1], [Circular *2] },
    [Circular *1]
  },
  b: <ref *2> Set(2) {
    <ref *1> Set(2) { [Circular *2], [Circular *1] },
    [Circular *2]
  }
}

DX serializer:

(function(h,j){return {a:h=new Set([j=new Set([h,j]),h]),b:j}}())
{
  a: Set(2) { Set(1) { undefined }, undefined },
  b: Set(1) { undefined }
}

Link to my serializer: https://github.com/lxsmnsyc/seroval

lxsmnsyc commented 1 year ago

Other known issues:

My serializer:

(h=>(h={},h.self=h,h))()
ryansolid commented 1 year ago

The second issue definitely looks like a small mistake in building. Like h just didn't assigned. The first one looks like a miss though. If you managed to find a good solution I'd be interested. I picked up a older version of valav that probably needs some love.

lxsmnsyc commented 1 year ago

@ryansolid I talked to @DylanPiercey and it seems that he has acknowledged the issues, not sure if he plans on fixing them, which I hope he would because I cannot really figure out the flow of valav.

There's also another issue which is the existence of undefined values in objects. The serializer skips them for some reason, which causes key in object tests to fail

DylanPiercey commented 1 year ago

To be clear the lack of recursive support for custom constructors (Map/Set) isn't there, so good to add. However avoiding serialization of undefined values is intentional to reduce the size of the serialized string (same choice JSON.stringify makes). That one is more of a trade off you have to weigh.

lxsmnsyc commented 1 year ago

Updated links and outputs for my serializer

lxsmnsyc commented 1 year ago

Since DX now already uses seroval which has the parity output with valav, this issue has been fixed.