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

Bug when using `@JsonSerializable(createFactory: false)` on an `Equatable` class. #1348

Open jtdLab opened 1 year ago

jtdLab commented 1 year ago

Using json_serializable: ^6.7.1 on a model class which extends Equatable leads to unwanted fields being included in generated code.

Given:

@JsonSerializable(createFactory: false) // <- notice createFactory being false
@immutable
class User extends Equatable {
  const User({
    required this.id,
    required this.name,
    required this.username,
    required this.password,
  });

  final String id;

  final String name;

  final String username;

  final String password;

  @override
  List<Object?> get props => [id, name, username, password];
}

the output is

Map<String, dynamic> _$UserToJson(User instance) {
  final val = <String, dynamic>{};

  void writeNotNull(String key, dynamic value) {
    if (value != null) {
      val[key] = value;
    }
  }

  writeNotNull('stringify', instance.stringify); // <- should not be included
  val['hashCode'] = instance.hashCode; // <- should not be included
  val['id'] = instance.id;
  val['name'] = instance.name;
  val['username'] = instance.username;
  val['password'] = instance.password;
  val['props'] = instance.props; // <- should not be included
  return val;
}

with multiple unwanted fields being included in the json output.

Hint: When using @JsonSerializable() or @JsonSerializable(createToJson: false) all works as expected.

BarteeX-Q commented 1 year ago

You can use @JsonKey(includeToJson: false) on props getter. And also you can create abstract class like bellow for farther implementation :)

abstract class EquatableJsonSerializable extends Equatable {
  const EquatableJsonSerializable() : super();

  @JsonKey(includeToJson: false)
  @override
  bool? get stringify => super.stringify;

  @JsonKey(includeToJson: false)
  @override
  int get hashCode => super.hashCode;
}
jtdLab commented 1 year ago

Yeah but thats just a workaround. When generating both fromJson and toJson the package already works correctly without the need for a workaround. So the package should also handle only generating toJson correctly.

BarteeX-Q commented 1 year ago

When using this package to generate fromJson and toJson methods, it ensures that when you serialize and then deserialize an object, you receive the same object as before. However, when you use only the toJson generator, it includes all getter calls and their values and also standard class fields in the JSON.

jtdLab commented 1 year ago

Exactly but why is it done this way?

kevmoo commented 1 year ago

Exactly but why is it done this way?

I had to pick a default when I started. Seemed reasonable 🤷

jtdLab commented 1 year ago

I had to pick a default when I started. Seemed reasonable 🤷

Makes sence to start this way, but would you agree that @JsonSerializable(createFactory: false) should include the same fields as @JsonSerializable()and @JsonSerializable(createToJson: false)?

c15yi commented 1 year ago

Reading the comment from @BarteeX-Q it sounds like this is expected behaviour, but having an option createFactory altering the output of the toJson method is very confusing and for me it is a bug.

I agree with @jtdLab, they should be the same, in my opinion.

Including the hashCode, stringify, etc. fields should be a separate option to avoid confusion.

tremp-m commented 2 weeks ago

I spent half a day trying to find out what the problem was.