Allow Object and Object?/dynamic is models, functions and exceptions

[marcglasberg]

@dnys1 said (in Discord):

Our intent was only to support Object/dynamic as the value type of a Map, not directly as a parameter.
This is because we need to know all the types statically so we can generate serializers for them.

I already have a use case for this, as one of my exception types needs to have a few Object? fields:

/// This will be turned into a `UserException` on the client, and its message
/// will be translated to the user language, and shown in a dialog.
///
/// Usage:
/// ```
/// throw TranslatableUserException('Cannot sell %d shares of stock you do not own', 3);
/// throw TranslatableUserException('Stock %s not found.', 'IBM');
/// ```
class TranslatableUserException implements Exception {
  const TranslatableUserException(this.message,
      [this.value1, this.value2, this.value3, this.value4, this.value5]);

  final String message;
  final Object? value1, value2, value3, value4, value5;
}

I'm not sure you do need to know all the types statically. Suppose your function is doSomething(Object? value) and you get one of these JSON values from the request: { 123 } or { "abc" } or { true }. Why can't you simply desserialize this as doSomething(123), doSomething(true) respectively?

I understand you can't use any object there, but maybe allow for the common values accepted in JSON (plus Date?), and throw an error at runtime if an invalid value is used. In the future you can use an IDE plugin (#34) to show an error in the code editor if an invalid value type is directly used.

[dnys1]

Hey @marcglasberg, great feedback and thanks for writing this up.

My Discord comment probably should have said:

we need to know all the types statically so we can ensure runtime safety

It's true that we could allow Object?/dynamic for JSON primitives and simply throw when a non-primitive value was passed. Ultimately, though, this feels like a major footgun, especially for new developers. I had even debated removing Map<String, Object?> support initially for the same reason.

I do think there is a model which can satisfy all uses cases coming with extension types, which would allow an explicit opt-in to this behavior without any runtime overhead. For example, you could have the following:

extension type JsonValue(Object? value) {}

class TranslatableUserException implements Exception {
  factory TranslatableUserException(String message,
          [Object? value1,
          Object? value2,
          Object? value3,
          Object? value4,
          Object? value5]) =>
      TranslatableUserException._(message, JsonValue(value1), JsonValue(value2),
          JsonValue(value3), JsonValue(value4), JsonValue(value5));

  const TranslatableUserException._(this.message,
      this.value1, this.value2, this.value3, this.value4, this.value5);

  final String message;
  final JsonValue value1, value2, value3, value4, value5;
}

While this is more verbose, it provides a level of type-safety that cannot be guaranteed with Object?/dynamic. It disincentivizes using those types at the client-server boundary but would enable a "I know what I'm doing" mechanism as well.

Thoughts?

[marcglasberg]

This solution is great, I love it.

I was just reading this file: https://github.com/celest-dev/celest/blob/main/packages/celest_core/test/json_value_test.dart which uses JsonValue

JsonValue currently has no unnamed constructor. Would you add it, so that you can write JsonValue(value1)?

final class JsonObject extends JsonValue {
  /// Creates a [JsonObject] from [wrapped].
  const JsonObject(this.wrapped)
      : _path = const [],
        assert(
            wrapped == null ||
                wrapped is String ||
                wrapped is int ||
                wrapped is double ||
                wrapped is bool ||
                wrapped is List<Object?> ||
                wrapped is Map<String, Object?>,
            'Unsupported JSON value: $wrapped');

  const JsonObject._(this.wrapped, [this._path = const []]);

  @override
  final Object? wrapped;

  @override
  final List<String> _path;
}

[dnys1]

Yes! My goal was to make JsonValue part of the public interface when extension types are released 😄 Currently, I don't feel like it's worth the overhead with the class-based version.

And I'll make sure to add a constructor like that for easy usage 💯

[marcglasberg]

You mean dart-lang/language#2727 ?

I personally don't mind the overhead, though.

[dnys1]

Yes, that should be released in Dart 3.3 soon 👍

The overhead is really two-fold with classes vs. extension types

1. With classes, you pay a runtime penalty for boxing/unboxing every value. Extension types are free.
2. Classes would complicate my serialization logic considerably, since String and JsonString have entirely different identities.

What I mean is:

// With classes
class JsonString extends JsonValue {
  const JsonString(String value): super(value);
}

const String s = 'abc';
const JsonString js = JsonString(s);
print(js == s); // false
print(js is String); // false

// With extension types
extension type const JsonString(String value) implements JsonValue {}

const String s = 'abc';
const JsonString js = JsonString(s);
print(js == s); // true
print(js is String); // true

They are basically marker types which only exist at compile type, which means I can treat every JsonString as a String when it comes to serialization, equality checking, etc. Otherwise, I would need to implement a lot of boxing/unboxing in the code generator and have special paths carved out for these types.

It's doable, but since Dart 3.3 is right around the corner, I wanted to save myself the energy 😆

[marcglasberg]

Perfect!

[marcglasberg]

@dnys1 Dart 3.3 is out! :)

[dnys1]

On it! 😄

[dnys1]

The JsonValue family has been released in 0.2.0 🚀