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

Introduce a generic object serializer/deserializer for WebWorker transfers #5143

Closed
jfirebaugh opened this issue Aug 14, 2017 · 3 comments
Closed

Comments

@jfirebaugh
Copy link
Contributor

See sketch here.

The idea is to replace all our manually-defined serialize methods with a generic serializer function that recursively serializes the own-properties of an object and records the constructor name, and all our manually-defined deserialize methods with a generic deserializer that looks up the constructor name in a class registry, then uses Object.create to reconstitute an instance without triggering the constructor.

Properties that should not be serialized (e.g. cached computed values) shall be marked as non-enumerable.

In the sketch above, Tranferables that should be transferred can be flagged by setting a transfer: true property on the Transferable. But I think it would be better to make serialize a transfer-only operation, and require you to copy ahead of time of any Transferables you need to copy.

@anandthakker
Copy link
Contributor

Properties that should not be serialized (e.g. cached computed values) shall be marked as non-enumerable.

This is elegant from the point of view of the serialize/deserialize code, but I think the verbosity it requires in client code is a bit much:

(Object.defineProperty: any)(this,  '_cached', {
  writable: true,
  enumerable: false,
  value: this.n * this.n
});

Since classes have to be registered anyway, an alternative would be to allow an optional { include: [property name] } argument to register().

@anandthakker
Copy link
Contributor

I suppose we could also reduce verbosity with a helper function addTemporaryProperty(this, '_cached', this.n * this.n), but that still feels more complicated / less readable to me.

@jfirebaugh
Copy link
Contributor Author

I'm 👍 on a register option with reversed sense: { omit: [...] }.

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

No branches or pull requests

2 participants