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

Feature: Strongly typed deserialization of JSON ops #9422

Closed
jbellerb opened this issue Feb 5, 2021 · 2 comments
Closed

Feature: Strongly typed deserialization of JSON ops #9422

jbellerb opened this issue Feb 5, 2021 · 2 comments
Labels
deno_core Changes in "deno_core" crate are needed feat new feature (which has been agreed to/accepted)

Comments

@jbellerb
Copy link
Contributor

jbellerb commented Feb 5, 2021

I've been messing with deno_core a bit and I found it a bit strange that JSON ops only work on loosely typed serde_json::Values given serde is capable of directly converting between JSON and Rust types. This can be changed by simply updating the type signatures of json_op_sync/async. Since serde_json::Value implements Deserialize/Serialize, very little code needs to be changed, however as json_op_sync/async are now generic, type inference is broken in some cases (see cli/build.rs:146).

Since the changes required for this feature are so simple, I've made them on a fork of deno (which can be upgraded to a PR if there is interest) for testing. I've also updated the deno_core examples and found that this change removed a lot of boilerplate. Finally, since serde now has more information about the types it is working with, serialization and deserialization should be faster, but I don't have benchmarks to back up that claim. Is there any interest in making this change? Thanks.

@lucacasonato
Copy link
Member

Please open a PR to discuss this further. Sounds interesting - benchmarks would be nice.

@jbellerb
Copy link
Contributor Author

jbellerb commented Feb 6, 2021

I've created a PR (#9423) . I'll try to get some basic benchmarks based on http_bench_json_ops soon.

@kitsonk kitsonk added deno_core Changes in "deno_core" crate are needed feat new feature (which has been agreed to/accepted) labels Feb 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deno_core Changes in "deno_core" crate are needed feat new feature (which has been agreed to/accepted)
Projects
None yet
Development

No branches or pull requests

3 participants