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

Error parsing javascript object to serde_json::Value #11502

Closed
augustocdias opened this issue Jul 23, 2021 · 3 comments · Fixed by #15946
Closed

Error parsing javascript object to serde_json::Value #11502

augustocdias opened this issue Jul 23, 2021 · 3 comments · Fixed by #15946
Labels
bug Something isn't working correctly deno_core Changes in "deno_core" crate are needed upstream Changes in upstream are required to solve these issues

Comments

@augustocdias
Copy link

I'm using deno embedded in a rust application and we're facing an issue with the serde_v8 with some objects.

At some point in our script we call await Deno.core.opAsync('success', message);

This message on the script side is an object, such as:

{
    lines: {
        100: {
           unit: "m"
        },
        200: {
           unit: "cm"
        }
    }
}

On the rust side, my success function expects a serde_json::Value object. This is failing to be serialised back to rust because it cannot handle the numbers as keys. Shouldn't deno convert them to string in this case?

Even if I set the object like this:

{
    lines: {
        "100": {
           unit: "m"
        },
        "200": {
           unit: "cm"
        }
    }
}

deno still treats it as a number and fails to parse it.

@bnoordhuis
Copy link
Contributor

@AaronO pointed out it's caused by rusty_v8's Object::get_own_property_names() using KeyConversionMode::kKeepNumbers (default inherited from V8's C++ API), whereas for this particular case you'd want kConvertToString or kNoNumbers.

To summarize: needs a change in rusty_v8 before it can be fixed in deno.

@bnoordhuis
Copy link
Contributor

I've opened denoland/rusty_v8#740 to track the necessary changes to rusty_v8.

@kitsonk kitsonk added deno_core Changes in "deno_core" crate are needed upstream Changes in upstream are required to solve these issues bug Something isn't working correctly labels Jul 29, 2021
RaisinTen added a commit to RaisinTen/deno that referenced this issue Sep 18, 2022
Fixes: denoland#11502
Signed-off-by: Darshan Sen <raisinten@gmail.com>
@RaisinTen
Copy link
Contributor

And finally, here's the PR that fixes the issue - #15946! :)

RaisinTen added a commit to RaisinTen/deno that referenced this issue Sep 25, 2022
Fixes: denoland#11502
Signed-off-by: Darshan Sen <raisinten@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly deno_core Changes in "deno_core" crate are needed upstream Changes in upstream are required to solve these issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants