Sdk: JSON.decode is not very handy in strong mode

Created on 12 Jan 2018  路  10Comments  路  Source: dart-lang/sdk

This is an umbrella issue for unhandiness of using objects returned by JSON.decode in strong mode.

JSON.decode returns List<dynamic> and Map<String, dynamic>, while in strong mode people expect it to return the most precise type.

For example in Flutter code like this:

final Map<String, List<String>> parsedManifest = JSON.decode(json)

needs to be replaced by this

final Map<String, dynamic> parsedJson = JSON.decode(json);
final Iterable<String> keys = parsedJson.keys;
final Map<String, List<String>> parsedManifest =
      new Map<String, List<String>>.fromIterables(keys,
         keys.map((key) => new List<String>.from(parsedJson[key])));

to make it correctly typed.

We might be doing work post-Dart 2.0 on improving the usability, but right now the code simply needs to be fixed. /cc @leafpetersen @lrhn

area-library type-enhancement

Most helpful comment

There is still the issue of "using" JSON as structured data. This is quite popular, for example:

Imagine the following JSON blob:

{
  "names": [
    "Leaf Peterson",
    "Vyacheslav Egorov",
  ]
}
Future<void> getAndPrintNames() async {
  Map<String, dynamic> json = await _getJson();
  // Error: List<dynamic> is not a List<String>
  List<String> names = json['names'];
  names.forEach(print);
}

Once we have _cast_ we could do it, but, not easily for our users:

var names = (json['names'] as List).cast<String>().toList();

Let's break that down:

var /* dynamic */ namesDynamic = json['names'];
var /* List<dynamic> */ namesListOfDynamic = namesDynamic as List;
var /* Iterable<String> */ namesIterableOfString = namesListOfDynamic.cast<String>();
var /* List<String> */ namesListOfString = namesIterableOfString.toList();

That's a lot of work, and easy to get wrong, especially with implicit casting:

  1. You could forget to do "as List", and do dynamic calls instead.
  2. You could forget to use "cast", and fail at runtime with the original error.
  3. You could forget to use "toList", and fail at runtime with Iterable<String> is not List<String>.

All 10 comments

There's been internal discussion of this: quoting myself:

One possible interface to this that would require no compiler magic at all might end up looking something like this:

const decoder = const json.decodeMap<String, int>(json.decodeString, json.decodeInt);

String preference = read(preferenceKey) ?? '{}';
return decoder.decode(preference); // No cast required

@rakudrama and @munificent played around with this a bit, I'd love for us to follow up on it when we have more time.

Note that once the .cast methods being added to the core library have landed, you could use those to avoid re-allocating the lists at the expense of checking each access.

There is still the issue of "using" JSON as structured data. This is quite popular, for example:

Imagine the following JSON blob:

{
  "names": [
    "Leaf Peterson",
    "Vyacheslav Egorov",
  ]
}
Future<void> getAndPrintNames() async {
  Map<String, dynamic> json = await _getJson();
  // Error: List<dynamic> is not a List<String>
  List<String> names = json['names'];
  names.forEach(print);
}

Once we have _cast_ we could do it, but, not easily for our users:

var names = (json['names'] as List).cast<String>().toList();

Let's break that down:

var /* dynamic */ namesDynamic = json['names'];
var /* List<dynamic> */ namesListOfDynamic = namesDynamic as List;
var /* Iterable<String> */ namesIterableOfString = namesListOfDynamic.cast<String>();
var /* List<String> */ namesListOfString = namesIterableOfString.toList();

That's a lot of work, and easy to get wrong, especially with implicit casting:

  1. You could forget to do "as List", and do dynamic calls instead.
  2. You could forget to use "cast", and fail at runtime with the original error.
  3. You could forget to use "toList", and fail at runtime with Iterable<String> is not List<String>.

We have made this harder for ourselves by returning mutable collections.
If they were immutable, we could let the decoder pick the most specific type that actually match its contents. (Well, it would probably require extra copying or the ability to change the type of a list/map, but internally we should be able to do that).

When the returned collections are mutable, a user might expect a List<Object>, and try to add a number to it. We can't return a List<String> just because it happens to only contains strings.

Specifying the type of the decoded data is doable too, probably better using composable decoders rather than generics. Maybe something like:

  JsonMap(JsonList(JsonString)).decode(text);
  JsonList(JsonStruct({"name": JsonString, "address": JsonList(JsonString)})).decode(people)

(say, where JsonMap defines a uniform map and JsonStruct has a separate decoder per named entry in the map).
We do run the risk of inventing another schema language :)

Yeah, I would definitely love to sink my teeth into this one. One approach is to lazily deserialize the JSON. In most cases, the user does know what types they expect a given piece of the JSON to have. The problem is that by the time they tell us, we've already deserialized the JSON and built the collection.

If we could parse it and build the collections on demand as the user traverses into the JSON, we could ensure the sub-parts are built with the right reified types.

I toyed with a prototype of this a while back, and it seemed promising.

How would that work in environments such as JS where we don't control the parse impl?

In JS, there is (as far as I know, I only poked at it for a couple of hours or so) still a two-stage "parsing" process:

  1. Parse the JSON string to a JS JSON object.
  2. Go through and convert the JS collections to Dart ones.

Step 2 is done in Dart and would be where we could inject the types. Though, now that I look at DDC's implementation of dart:convert, it looks like step 2 only really happens for maps. For lists, we use the JS array as-is. :-/

So there would be overhead for what I propose, to go through and copy the JSArray to a new typed list. Though I imagine we might be able to do something more clever and actually jam the type argument into the existing JSArray, since we know it hasn't escaped anyway.

Note that JSON performance in Dart2JS is already 3x worse than using plain JavaScript:
https://github.com/matanlurey/dart-js-improvements#json

... so I'd be really hesitant to make this even worse if possible.

Yeah, understood. Performance was one of the things I was looking at when I was prototyping. I wouldn't consider any solution that had bad perf to be acceptable.

cc @rakudrama

Was this page helpful?
0 / 5 - 0 ratings