google / json_serializable.dart

Generates utilities to aid in serializing to/from JSON.
https://pub.dev/packages/json_serializable
BSD 3-Clause "New" or "Revised" License
1.56k stars 400 forks source link

genericArgumentFactories: support any variable name for fromJsonT/toJsonT #870

Open rrousselGit opened 3 years ago

rrousselGit commented 3 years ago

Currently, it appears that genericArgumentFactories works only if the mappers use a very specific name format

Consider the following example:

@JsonSerializable(genericArgumentFactories: true)
class Either<First, Second> {
  Either(this.first, this.second);

  factory Either.fromJson(
    Map<String, Object?> json,
    First Function(Object?) fromJsonFirst,
    Second Function(Object?) fromJsonSecond,
  ) {
    throw UnimplementedError();
  }

  First first;
  Second second;
}

@JsonSerializable()
class Another {
  Another(this.value);

  factory Another.fromJson(Map<String, Object?> json) =>
      _$AnotherFromJson(json);

  final Either<int, String> value;
}

If we renamed fromJsonFirst to anything else, then json_serializable would fail with:

Expecting a `fromJson` constructor with exactly one positional parameter. The only extra parameters allowed are functions of the form `T Function(Object?) fromJsonT` where `T` is a type parameter of the target type.
package:generic_argument_factory_integration/models.dart:46:18
   â•·
46 │   factory Either.fromJson(

It would be great if we could use any other variable name instead, such as:

  factory Either.fromJson(
    Map<String, Object?> json,
    First Function(Object?) first,
    Second Function(Object?) second,
  ) {
    throw UnimplementedError();
  }
kevmoo commented 3 years ago

PRs welcome! 😄

rrousselGit commented 3 years ago

I may raise a PR, as the current implementation throws a RangeError if the toJson is defined in an interface (which it will with Freezed)

What do you think about simply checking the number of arguments, without checking what they are? Worse case scenario, we'll have a compilation error.

This may improve the developer experience too, because the compilation error will show what the problem is.

For example at some point I made a mistake in the syntax for "toJson". And instead of Object Function(T) I did T Function(Object), but the error message wasn't helpful. On the other hand, a compilation error will clearly indicate the issue.

kevmoo commented 3 years ago

I don't like guessing. I guess i could just look for one function that matches what is needed here.

Saxorman commented 1 year ago

Hello! I have the same warning during code generation.

@JsonSerializable(genericArgumentFactories: true, createToJson: false)
class InheritWrapperField<T> {
  final T value;
  final bool inherit;

  InheritWrapperField(this.value, this.inherit);

  factory InheritWrapperField.fromJson(
          Map<String, dynamic> json, T Function(Object? json) fromJson) =>
      _$InheritWrapperFieldFromJson(json, fromJson);
}
Expecting a `fromJson` constructor with exactly one positional parameter. The only extra parameters allowed are functions of the form `T Function(Object?) fromJsonT` where `T` is a type parameter of the target type.
*/options_field_wrapper.dart:27:31
   â•·
27 │   factory InheritWrapperField.fromJson(
   │                               ^^^^^^^^

If change class name to InheritWrapper - all ok

dleurs commented 2 weeks ago

Any news on this ?

TekExplorer commented 2 weeks ago

Why... Does this matter? It doesn't really affect how you use it, and it keeps to a known, consistent pattern.

TekExplorer commented 2 weeks ago

The range error thing is a problem though