typeson icon indicating copy to clipboard operation
typeson copied to clipboard

Preserving references fails with user-defined conversions

Open HannesSommer opened this issue 5 years ago • 3 comments

The following minimal test case (in typescript) reveals a potential bug where a reference is undefined after revive through user defined conversion:

const Typeson = require('typeson');

class ComplexClassThroughArray {
	constructor(public x = {}) {
	}
}

const typeson = new Typeson().register({
	ComplexClassThroughArray: [
		(x: any) => x instanceof ComplexClassThroughArray, // test function
		(d: ComplexClassThroughArray) => [d.x], // encapsulator function
		(d: Array<object>) => { // reviver function
			return new ComplexClassThroughArray(d[0]);
		}
	],
});

it('revive_complex_class_through_array', () => {
	const o = new ComplexClassThroughArray();
	const json = JSON.stringify(typeson.encapsulate({
		o,
		x: o.x
	}));

	const r = typeson.revive(JSON.parse(json));

	expect(r.x).toStrictEqual(o.x);
});

This test fails at the very end because r.x is undefined.

The json variable has the serialized value {"o":[{}],"x":"#o.0","$types":{"o":"ComplexClassThrughArray","x":"#"}}.

The same problem appears if you replace ComplexClassThroughArray with the built-in type Set, which has basically an identical user defined conversion in typeson-registry.

Could it be that the reference in "x":"#o.0" does get resolved after "o":[{}] has been revived rendering the json path invalid?

HannesSommer avatar May 31 '19 14:05 HannesSommer

@brettz9 , @dfahlander, the theory that the reference path is resolved after revive and this way causing the above problem seems correct. With this alternative experimental reviver the above test succeeds:

			(d: Array<SimpleClass>) => { // reviver function
				const o = new ComplexClassThroughArray();
				o.x = d[0];
				// patch the object to make it indexable like an array 
				// to allow the resolution of references into it
				(o as any)[0] = d[0];
				return o;
			}

However, this is of course not a solution and of little more use than to proof the theory. Furthermore, this approach seems to fail when a reference is cyclic.

HannesSommer avatar Jun 07 '19 07:06 HannesSommer

@HannesSommer , my apologies, I haven't had time to take a closer look at where things are or how we may get them to where they need to be and my plate is pretty full at the moment. I just added some stricter linting and jsdocs to hopefully facilitate a deeper dive by whoever may have time to get there first. :)

brettz9 avatar Jun 07 '19 07:06 brettz9

@brettz9 thanks for the explanation! I totally understand. If I find the resources to fixing this problem in a sound way I will open a PR. But unfortunately, as it seems to me at the moment, this isn't a simple problem with an easy fix.

HannesSommer avatar Jun 07 '19 20:06 HannesSommer