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

Make JsonKey's defaultValue a Function? type #1185

Closed
erf opened this issue Aug 10, 2022 · 23 comments · Fixed by #1216
Closed

Make JsonKey's defaultValue a Function? type #1185

erf opened this issue Aug 10, 2022 · 23 comments · Fixed by #1216

Comments

@erf
Copy link

erf commented Aug 10, 2022

I would like to supply a DateTime.now() as a defaultValue to JsonKey, but as it's not a const it's not possible to add it, however if it was of type Function? i could pass it a function which would return a value in the same way as toJson and fromJson, e.g. static DateTime newDate() => DateTime.now(); This would be a breaking change so not sure if it's feasible, but it would make the API more flexible perhaps?

Dart SDK version: 2.17.6 (stable) (Tue Jul 12 12:54:37 2022 +0200) on "macos_arm64"

@kevmoo
Copy link
Collaborator

kevmoo commented Oct 4, 2022

You know what I COULD do. We could allow defaultValue (which is already typed as Object?) to accept a function of type T Function() where T is the field type. Not a bad idea!

@kevmoo
Copy link
Collaborator

kevmoo commented Oct 4, 2022

Not a priority for me, but I'd accept a reasonable PR.

@erf
Copy link
Author

erf commented Oct 4, 2022

I'm looking into the code and perhaps point me in the right direction if you have an idea already?

If i try to pass a function expression to the current defaultValue type of Object? it complains with the following:

Arguments of a constant creation must be constant expressions.
Try making the argument a valid constant, or use 'new' to call the constructor.dart[const_with_non_constant_argument](https://dart.dev/diagnostics/const_with_non_constant_argument)

The JsonKey is an annotation type so it's constructor needs to be const it seem and a function expression is apparently not that.

Were you thinking of changing the type of defaultValue from Object? to something else?

@kevmoo
Copy link
Collaborator

kevmoo commented Oct 4, 2022

@erf – you will ONLY be able to pass in top-level or static functions. No instance functions will be allowed.

@erf
Copy link
Author

erf commented Oct 4, 2022

@kevmoo that seem to work now - so not sure how to improve this per now. I'm closing this. Feel free to open this if you have any suggestions for further improvement.

@erf erf closed this as completed Oct 4, 2022
@kevmoo
Copy link
Collaborator

kevmoo commented Oct 4, 2022

@erf – like...the feature actually works? I don't remember adding that feature. (goes to look at code...)

@kevmoo
Copy link
Collaborator

kevmoo commented Oct 5, 2022

This is NOT implemented. Build will break if you use a function. I actually have a todo here for this!

@kevmoo kevmoo reopened this Oct 5, 2022
kevmoo added a commit that referenced this issue Oct 5, 2022
Update docs in json_annotation
Prepare to release json_serializable v6.5.0

Closes #1185
@kevmoo
Copy link
Collaborator

kevmoo commented Oct 5, 2022

@erf – see #1216

@erf
Copy link
Author

erf commented Oct 5, 2022

Sorry, i did not test it well, i just did not get any errors, i guess i misunderstood. It seems like you've fixed it now? Great stuff! I tried to pull down a local version now and link to it using dependency_overrides and path given a top function which returned a date as a defaultValue but it seem to be null. Must be doing something wrong? (i did pub upgrade and checked out the correct branch). Btw, maybe you could add an example on how to use it in json_annotation

kevmoo added a commit that referenced this issue Oct 5, 2022
Update docs in json_annotation
Prepare to release json_serializable v6.5.0

Closes #1185
@eximius313
Copy link

@kevmoo - when do you plan the release?

@kevmoo
Copy link
Collaborator

kevmoo commented Oct 7, 2022 via email

@kevmoo
Copy link
Collaborator

kevmoo commented Oct 7, 2022

Done! v6.5.0

@erf
Copy link
Author

erf commented Oct 7, 2022

Are you able to pass a function which returns a DateTime now in the defaultValue? I can't seem to make that work.

@kevmoo
Copy link
Collaborator

kevmoo commented Oct 7, 2022

You just need to use a top-level or static function.

@kevmoo
Copy link
Collaborator

kevmoo commented Oct 7, 2022

Always good to share source code...

@erf
Copy link
Author

erf commented Oct 7, 2022

I tried the following:

@JsonSerializable()
class Task {
  @JsonKey(defaultValue: '')
  String task;

  @JsonKey(defaultValue: DateTime.now)
  DateTime? date0;
}

I also tried:

DateTime getDateTime() => DateTime.now();


@JsonSerializable()
class Task {
  @JsonKey(defaultValue: '')
  String task;

  @JsonKey(defaultValue: getDateTime)
  DateTime? date0;
}

The last one produces:

Task _$TaskFromJson(Map<String, dynamic> json) => Task(
      json['task'] as String? ?? '',
      date0: json['date0'] == null
          ? null
          : DateTime.parse(json['date0'] as String) ?? getDateTime(),
      date1: json['date1'] == null
          ? null
          : DateTime.parse(json['date1'] as String),
    );

Here the getDateTime() has a squiggly yellow line indicating that:

"The left operand can't be null, so the right operand is never executed.
Try removing the operator and the right operand."

It also seem strange that getDateTime() is not called if json['date0'] == null ..?

@kevmoo
Copy link
Collaborator

kevmoo commented Oct 7, 2022

Remove the ? from date0.

@kevmoo
Copy link
Collaborator

kevmoo commented Oct 7, 2022

Hrm...I'm digging now and seeing the problem. Will have to investigate.

@kevmoo
Copy link
Collaborator

kevmoo commented Oct 7, 2022

Will need to follow-up here: #1219

@erf
Copy link
Author

erf commented Oct 7, 2022

Ok. I was thinking if i kept the date values optional / nullable, i did not have to set them at construction time and then if i write them to json or parse them and the value is null, then the default value is used, not sure if this is possible. When i think about it i could just make them non-nullable, and just make a custom constructor for these cases and set the date values there.

@kevmoo
Copy link
Collaborator

kevmoo commented Oct 7, 2022

@erf you actually FOUND A BUG!! It affects BigInt, DateTime and Uri. Completely unrelated to this feature.

Well done!

@kevmoo
Copy link
Collaborator

kevmoo commented Oct 7, 2022

@erf – well, it's weird. Turns out the type helpers for certain types NEED to explicitly support default values. I'd never done "the work" for three types, because they weren't const-able. That's now fixed! See #1220

@kevmoo
Copy link
Collaborator

kevmoo commented Oct 10, 2022

Fixed now and published @erf

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

Successfully merging a pull request may close this issue.

3 participants