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

Provide a top-level function in dart:convert for decoding JSON from binary #55996

Open
osa1 opened this issue Jun 13, 2024 · 3 comments
Open
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-convert type-enhancement A request for a change that isn't a bug

Comments

@osa1
Copy link
Member

osa1 commented Jun 13, 2024

Currently there's no discoverable way to decode JSON from binary data, and users have to convert the binary data to a string first, then decode the string, which is inefficient.

I actually thought this is not possible to do because it's so well hidden. Only the backend implementers know how to do it. You have to fuse a Utf8Decoder with a JsonDecoder:

(const Utf8Decoder()).fuse(const JsonDecoder()).convert(...)

However if you look at the source code, it looks as if this does it in two steps:

T convert(S input) => _second.convert(_first.convert(input));

Which confuses even SDK devs: https://dart-review.googlesource.com/c/sdk/+/371080/comment/0e91626e_fa2b694f

The reason why this works is that backends can "patch" the fuse implementation to return a converter that does the conversion in one step:

  • VM:

    Converter<List<int>, T> fuse<T>(Converter<String, T> next) {
    if (next is JsonDecoder) {
    return new _JsonUtf8Decoder(
    (next as JsonDecoder)._reviver, this._allowMalformed)
    as dynamic/*=Converter<List<int>, T>*/;
    }

  • Wasm:

    Converter<List<int>, T> fuse<T>(Converter<String, T> next) {
    if (next is JsonDecoder) {
    return _JsonUtf8Decoder(
    (next as JsonDecoder)._reviver, this._allowMalformed)
    as dynamic/*=Converter<List<int>, T>*/;
    }

(dart2js doesn't patch it as there's no API to decode directly from a Uint8Array or similar in the browser)

There's no way for a user to discover this and use fuse instead of doing two conversions. E.g. https://dart-review.googlesource.com/c/sdk/+/371080.

The fact that this is possible is also not documented anywhere as far as I can see.

I think ideally we should have top-level function in dart:convert, similar to jsonDecode.

An easier fix might be just documenting (at the dart:convert top-level) that fusing decoders this way is possible and potentially more efficient, and should be preferred when decoding binary to JSON.

@mraleph mraleph added the area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. label Jun 13, 2024
@lrhn
Copy link
Member

lrhn commented Jun 13, 2024

Adding an external const Codec<List<int>, Object> utf8Json; seems possible and reasonable. It'll have to fall back on being a fuse of utf8 and json where we don't have a more efficient version.

@lrhn lrhn added library-convert type-enhancement A request for a change that isn't a bug labels Jun 13, 2024
@kevmoo
Copy link
Member

kevmoo commented Jun 27, 2024

We could also expose JsonUtf8Decoder to go along with JsonUtf8Encoder, then we have the bits for JsonUtf8Codec.

Right?

@lrhn
Copy link
Member

lrhn commented Jun 28, 2024

Probably, yes. If the _JsonUtf8Decoder exists and is designed well enough to be public, we could expose it directly.
Otherwise it might needs some tweaks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-convert type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants