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

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

Open
jtdLab opened this issue Aug 23, 2023 · 9 comments
Open

Comments

@jtdLab
Copy link

jtdLab commented Aug 23, 2023

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

BarteeX-Q commented Aug 25, 2023

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

jtdLab commented Aug 25, 2023

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

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

jtdLab commented Aug 26, 2023

Exactly but why is it done this way?

@kevmoo
Copy link
Collaborator

kevmoo commented Aug 26, 2023

Exactly but why is it done this way?

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

@jtdLab
Copy link
Author

jtdLab commented Aug 27, 2023

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

c15yi commented Nov 14, 2023

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

tremp-m commented Oct 29, 2024

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

@kunjaarogyatech
Copy link

@kevmoo I too feel having different core behaviour for one param createFactory is confusing. It should be consistent across.

It looks like bug to me as well.

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

No branches or pull requests

6 participants