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

Expose Deno.core.(de)serialize as a public API #12379

Closed
DjDeveloperr opened this issue Oct 9, 2021 · 14 comments
Closed

Expose Deno.core.(de)serialize as a public API #12379

DjDeveloperr opened this issue Oct 9, 2021 · 14 comments
Labels
declined thank you, but respectfully declined suggestion suggestions for new features (yet to be agreed)

Comments

@DjDeveloperr
Copy link
Contributor

DjDeveloperr commented Oct 9, 2021

Deno.core.serialize and Deno.core.deserialize seem to serialize and deserialize v8's binary format. I'm working on something that need those functions so I have to use them from Deno.core right now, which is not a public API and considered unstable (and it also doesn't have any TS types either).

Node.js exposes this functionality through v8 module: https://nodejs.org/api/v8.html#v8_serialization_api

Could these two functions be exposed as a public API?

@MierenManz
Copy link

might also be a good idea to expose it as a public api now that ffi supports buffers.

@kitsonk kitsonk added the suggestion suggestions for new features (yet to be agreed) label Oct 12, 2021
@kitsonk
Copy link
Contributor

kitsonk commented Oct 12, 2021

It is very intentional that they are on Deno.core and non-public, as we really don't want people to couple to them. They are quite "advanced" APIs. The web platforms structuredClone() is the effective public API.

What is the use case?

@DjDeveloperr
Copy link
Contributor Author

I need these APIs to pass and return JS values to/from a native library, structuredClone is not much helpful in this case.

@andreubotella
Copy link
Contributor

I need these APIs to pass and return JS values to/from a native library, structuredClone is not much helpful in this case.

We don't currently support for-storage serialization, meaning that if you serialize ArrayBuffer, SharedArrayBuffer or WebAssembly.Module objects and then try to deserialize that from a different process, you'll get a panic.

@DjDeveloperr
Copy link
Contributor Author

DjDeveloperr commented Oct 12, 2021

From what I tested it seems serializing ArrayBuffer works, except SharedArrayBuffer and such which use a transfer ID would of course not work. Aside, native library is on same process I think, it's loaded through the new FFI API.

Note: I chose this binary format over JSON because it's easier to parse and likely more efficient, and it supports (de)serializing more type of values, of which I need at the moment is Array Buffer and Views.

@andreubotella
Copy link
Contributor

I misspoke, serializing ArrayBuffer objects will work, but transferring them won't, because they do use a transfer ID to allow the zero-copy transfer.

Regardless, making Deno.core.(de)serialize available as a public API would expose users to the internals of how transfer IDs work, and might allow them to trigger UB if a transferred ArrayBuffer is deserialized multiple times, which is something the message passing and structured clone APIs don't allow.

@DjDeveloperr
Copy link
Contributor Author

Seems like Node.js straight away errors that SharedArrayBuffer cannot be cloned, I think we can do same to avoid UB?

@andreubotella
Copy link
Contributor

andreubotella commented Oct 12, 2021

Seems like Node.js straight away errors that SharedArrayBuffer cannot be cloned, I think we can do same to avoid UB?

That would no longer be exposing the serialization internals directly, but a version of for-storage (de)serialization, which is used in the web platform for storing serialized values in persistent storage. We don't yet implement for-storage serialization, since Deno CLI doesn't yet implement Indexed DB or any of the other web APIs that use it – but we will probably need to implement it for #10750.

So this would be technically feasible. I'll leave it to the core team to decide whether it's worth exposing or not.

@petamoriken
Copy link
Contributor

related whatwg/html#3517

@kitsonk
Copy link
Contributor

kitsonk commented Oct 31, 2021

That is pretty stale, and with structured clone having moved forward, I suspect it is unlikely that it will ever get standardized, at it would require agreement on what the serialization format would be, which would be very specific to the engine, or agreement of the least worst version.

Of course if something gets agreement we would be glad to support it, but I wouldn't hold your breath.

@cjihrig
Copy link
Contributor

cjihrig commented Jun 24, 2022

Another reason not to make this public API: V8 sometimes changes the serialize/deserialize format in ways that would be difficult for us to maintain. This has impacted Node.js and Cloudflare Workers already.

@MierenManz
Copy link

MierenManz commented Jun 27, 2022

I still feel like eventho it has breaking changes because of a format that is not backwards compatible that it can still be useful in the context of passing stuff from and to shared libraries via the FFI

But in that regard I have a repo that supports the latest format used for Deno.core.deserialize and Deno.core.serialize so technically there is not a need to support it from deno's perspective.

@kitsonk kitsonk added the declined thank you, but respectfully declined label Jul 4, 2022
@kitsonk
Copy link
Contributor

kitsonk commented Jul 4, 2022

There are many arguments for not exposing it publicly. Respectfully declining.

@kitsonk kitsonk closed this as not planned Won't fix, can't repro, duplicate, stale Jul 4, 2022
@johnspurlock
Copy link
Contributor

With the advent of Deno KV and KV Connect, could you reconsider this?

Working with v8-persisted KV values in userland in other Deno processes would benefit greatly from this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
declined thank you, but respectfully declined suggestion suggestions for new features (yet to be agreed)
Projects
None yet
Development

No branches or pull requests

7 participants