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

Preserving references fails with user-defined conversions #11

Open
HannesSommer opened this issue May 31, 2019 · 3 comments
Open

Preserving references fails with user-defined conversions #11

HannesSommer opened this issue May 31, 2019 · 3 comments
Labels

Comments

@HannesSommer
Copy link

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
Copy link
Author

@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.

@brettz9
Copy link
Collaborator

brettz9 commented Jun 7, 2019

@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. :)

@HannesSommer
Copy link
Author

HannesSommer commented Jun 7, 2019

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants