-
Notifications
You must be signed in to change notification settings - Fork 8
Fix bigint & HTMLElement serialization #46
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
base: main
Are you sure you want to change the base?
Conversation
Performance improvement issues, but performance problems still exist.function deepCopyAndPrepareForSafeStringify(
obj: any,
hash = new WeakMap()
): any {
// check circular
if (hash.has(obj)) {
return hash.get(obj);
}
if (typeof obj === "bigint") {
return { type: "bigint", value: obj.toString() };
}
if (obj === null || typeof obj !== "object") {
return obj;
}
if (obj instanceof HTMLElement) {
const element = {
tag: obj.tagName,
outerHTML: obj.outerHTML,
};
hash.set(obj, element);
return element;
}
if (Object.prototype.hasOwnProperty.call(obj, "toJSON")) {
const json = obj.toJSON();
for (const key in json) {
json[key] = deepCopyAndPrepareForSafeStringify(json[key], hash);
}
hash.set(obj, json);
return json;
}
const copy = Array.isArray(obj)
? ([] as any[])
: ({ __proto__: obj.__proto__ } as any);
hash.set(obj, copy);
for (const key in obj) {
if (Object.prototype.hasOwnProperty.call(obj, key)) {
copy[key] = deepCopyAndPrepareForSafeStringify(obj[key], hash);
}
}
return copy;
} |
|
I hit a similar issue with one of my projects using this action which also breaks the inspector during serialization. This is the actor that caused the problem: When this actor completes, the snapshot contains Supabase-returned Session[] objects with Date fields (created_at, updated_at), nested arrays (session_participants), and complex object structures. The inspector crashes with TypeError: can't access property "config", s is undefined. The crash happens because of this serialization pattern:
According to the AI assistant I had helping me look at the code:
The deepCopyAndPrepareForSafeStringify workaround handles bigint/HTMLElement, but doesn't handle Date objects from database queries. It would still corrupt those Dates, breaking the snapshot structure. AI suggested swapping to |
|
@Blacktiger Can you try this fix instead? #48 |
I'll see if I can give that a try hopefully later this week. |
| obj: any, | ||
| hash = new WeakMap() | ||
| ): any { | ||
| if (typeof obj === 'bigint') return obj.toString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is pretty ad-hoc. Ideally, the serialization should try to avoid losing information. It is a different thing to have a "12" string and a 12n bigint value. The serialization should try to encode the encoded's type using a wrapper object or something so its decoder counterpart could try to deserialize it closely. Or at least display as much as possible about the original value.
Some inspiration here could be drawn from libraries like superjson, React Flight's serialization algorithm and similar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that the lossy nature of the current approach has already been raised as a concern here: #46 (comment)
No description provided.