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

Structured cloning preset does not handle ArrayBuffers like structured cloning in browser #8

Closed
dumbmatter opened this issue Jan 25, 2018 · 4 comments

Comments

@dumbmatter
Copy link

dumbmatter commented Jan 25, 2018

I wrote https://github.com/dumbmatter/realistic-structured-clone a while back, and was recently linked to dexie/Dexie.js#647 (comment) which suggests that typeson could do a better job at achieving the same goal.

So I made a new branch https://github.com/dumbmatter/realistic-structured-clone/tree/typeson which basically just wraps typeson.

My hope was that this would pass all my unit tests, and then some. My tests only cover a subset of structured cloning, but that's all most people use (except this guy dumbmatter/realistic-structured-clone#5 :) ).

Mostly it worked, but one test failed, related to ArrayBuffers. This is the code:

var shared = new ArrayBuffer(7);
var obj = {
    wrapper1: new Uint8Array(shared),
    wrapper2: new Uint16Array(shared, 2, 2)
};
obj.wrapper1[0] = 1;
obj.wrapper2[1] = 0xffff;

var obj2 = structuredClone(obj);

console.log(obj2.wrapper1.buffer === obj2.wrapper2.buffer);

That should output "true", but with a Typeson-backed structuredClone function it says "false".

Here is a self-contained example of the error, including a comparison to structured cloning in the browser which does indeed output "true": https://github.com/dumbmatter/realistic-structured-clone/blob/typeson-bug/bug.js

@brettz9
Copy link
Collaborator

brettz9 commented Feb 17, 2018

I've got a fix coming for this... Need some time to finish off tests, push, etc., but it is working...

Essentially, our buffer-related code (including typed arrays and DataView) did not preserve the underlying buffer, which is a problem both for reestablishing circular references as well as for merely restoring a single typed array or data view with its full underlying buffer.

We do have code already for catching circular references, but the cyclic detection does not recurse on Typeson-replacements. Rather than trying a recursive solution which leverages that code (which I think may not even be possible given that such as a typed array's buffer property, for example, is not even iterable), I've gotten some code working in DataView, ArrayBuffer, and typed arrays which checks a buffer cache during replacement and revivification. (It requires an addition to Typeson I also intend to add shortly which passes a state object between revivers).

@brettz9
Copy link
Collaborator

brettz9 commented Feb 17, 2018

And thanks very much for the very helpful report...

@brettz9
Copy link
Collaborator

brettz9 commented Feb 17, 2018

This should now be fixed in v1.0.0-alpha.19 (commit 8da1f2c ). Note that it is important to update your dependencies, including Typeson.

Note that I added tests for typeson and typeson-registry but I haven't yet tested the changes here within IndexedDBShim.

If you would kindly confirm if it is working for you now--otherwise, my plan is to close this issue shortly, as I think it should be fixed.

@dumbmatter
Copy link
Author

Yep, it's fixed! Thanks!

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

No branches or pull requests

2 participants