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

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

Open
rrousselGit opened this issue Apr 10, 2021 · 7 comments

Comments

@rrousselGit
Copy link
Contributor

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
Copy link
Collaborator

kevmoo commented Apr 10, 2021

PRs welcome! 😄

@rrousselGit
Copy link
Contributor Author

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
Copy link
Collaborator

kevmoo commented Apr 14, 2021

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

@Saxorman
Copy link

Saxorman commented Sep 6, 2023

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

borgoat added a commit to borgoat/dart_redux_entity that referenced this issue Nov 1, 2023
This keeps json_serializable happy, as it checks the name
of additional methods provided to the `fromJson` factory[^1].

[^1]: google/json_serializable.dart#870
borgoat added a commit to borgoat/dart_redux_entity that referenced this issue Dec 30, 2023
This keeps json_serializable happy, as it checks the name
of additional methods provided to the `fromJson` factory[^1].

[^1]: google/json_serializable.dart#870
@dleurs
Copy link

dleurs commented Oct 24, 2024

Any news on this ?

@TekExplorer
Copy link

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

@TekExplorer
Copy link

The range error thing is a problem though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants