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

Use the extension methods feature to remove need of most boilerplate #558

Closed
filiph opened this issue Nov 4, 2019 · 27 comments
Closed

Use the extension methods feature to remove need of most boilerplate #558

filiph opened this issue Nov 4, 2019 · 27 comments

Comments

@filiph
Copy link

filiph commented Nov 4, 2019

Starting with Dart 2.6, json_serializable has the option to create the toJson() method for the user. Although extension methods don't support constructors, json_serializable could create a static generative method instead. This means that user code would look like this:

import 'package:json_annotation/json_annotation.dart';

part 'example.g.dart';

@JsonSerializable(nullable: false)
class Person {
  final String firstName;
  final String lastName;
  final DateTime dateOfBirth;
  Person({this.firstName, this.lastName, this.dateOfBirth});
}

The generated extension might look like this:

// in example.g.dart

extension on Person {
  static Person fromJson(Map<String, dynamic> json) => _$PersonFromJson(json);

  Map<String, dynamic> toJson() => _$PersonToJson(this);
}

// ...

Rest of the code can keep the same.

@natebosch
Copy link
Member

The extension would need to be named, and the invocation of fromJson would need to reference that name. This would be a breaking change. Instead of Person.fromJson the usage would look like PersonExtension.fromJson.

@kevmoo
Copy link
Collaborator

kevmoo commented Nov 4, 2019 via email

@bsutton
Copy link

bsutton commented Nov 28, 2019

I like the general idea as the current method of having to create the methods in the source class is messy.

What about using a with statement?

It doesn't remove the need to update the source class but does make the update more elegant.
It would also be non-breaking as the old technique would still work.
In team.dart

class Team with  _TeamJson {
  Customer owner;
}

In team.g.dart

class _TeamJson {
  factory _TeamJson.fromJson(Map<String, dynamic> json) => _$TeamFromJson(json);
  Map<String, dynamic> toJson() => _$TeamToJson(this as Team);
}

Team _$TeamFromJson(Map<String, dynamic> json) {
...

The 'this as Team' expression is a little dodgy but given its generated code it will always be correct.
class _TeamJson can be auto generated in the .g.dart file.

So now the implementation is two words with _TeamJson.

Elegant and non-breaking.

@natebosch
Copy link
Member

A with or extends does not bring constructors. The toJson() may be solvable with this approach, the fromJson would not be.

@mikuhl-dev
Copy link

Why are we concerned about breaking changes? This simplifies everything by a bunch. Don't make the solution extra complicated to keep from breaking. Next we will just need native struct support and get rid of this all together. 😍

@k-paxian
Copy link

At the moment, there is only one full-blown solution with zero boilerplate in the model class, based on
@eernstg reflectable library

Almost two years I've looked for a clean serialization/deserialization solution, ended up creating it on my own 😄

@kevmoo
Copy link
Collaborator

kevmoo commented Dec 22, 2019

Awesome, @k-paxian

This package is meant to be straight-forward and meet the needs of the Dart team.

Glad you're running with something that handles more cases in a different way!

@kevmoo kevmoo closed this as completed Dec 25, 2019
@PiN73
Copy link

PiN73 commented Dec 25, 2019

@natebosch extensions allow to write exactly Person.fromJson, aren't they?

@kevmoo
Copy link
Collaborator

kevmoo commented Dec 26, 2019

@natebosch extensions allow to write exactly Person.fromJson, aren't they?

Hrm...not sure...

@mikuhl-dev
Copy link

Sad that this is closed.

@natebosch
Copy link
Member

@natebosch extensions allow to write exactly Person.fromJson, aren't they?

No, They'd allow us to write PersonExtension.fromJson. Extension methods add method to instances. They do not add static methods to a class. Adding members to the static interface of a class is a separate feature requested here: dart-lang/language#723 You can add a thumbs up to the original comment to express support.

Sad that this is closed.

It is closed because it is not actionable. If the language adds new features like the ability to add static interface extensions we can certainly revisit.

Why are we concerned about breaking changes?

That is not the only concern. The PersonExtension.fromJson call site is also a worse and less discoverable UX.

@jakemac53
Copy link
Collaborator

We should consider re-opening this issue given a recent idea that @DaveShuckerow implemented for a hackathon, see the cereal package.

This adds extension methods to the JsonDecoder class which are named based on the class, so for instance json.decodeUser and json.encodeUser.

We could similarly just add top level methods jsonEncodeUser and jsonDecodeUser.

Both of these approaches remove all the boilerplate except for the generated part file reference, and result in something very similar to my previous jsonAutoDecode<T> proposal.

@jakemac53
Copy link
Collaborator

cc @kevmoo @natebosch

@jakemac53
Copy link
Collaborator

Another alternative would be to create a separate package which also depends on json_annotation but provides a different builder and uses this same strategy. It could then share all the same configuration but that wouldn't require a breaking change and the current approach would still be an option for those who like it.

@jakemac53 jakemac53 reopened this Jan 3, 2020
@DaveShuckerow
Copy link

DaveShuckerow commented Jan 3, 2020

We should consider re-opening this issue given a recent idea that @DaveShuckerow implemented for a hackathon, see the cereal package.

Thanks for the shout-out, @jakemac53!

All, if you're interested in seeing what JSON serialization in Dart looks like with no-boilerplate extension methods, please do try package:cereal. It's not production-ready, but it is a proof-of-concept.

If we find that this approach works well, that can help us justify the work to port the functionality back to the full library.

@kevmoo
Copy link
Collaborator

kevmoo commented Jan 3, 2020

@jakemac53 – let's talk about this when I'm back in the office.

@mnordine
Copy link

mnordine commented Mar 5, 2020

Any update on this? I did take a look at cereal, and it is an exciting approach. However, it only handles simple cases (no inheritance or generics).

@mikuhl-dev
Copy link

Lets do this!

@kevmoo
Copy link
Collaborator

kevmoo commented Mar 9, 2020

I'm willing to entertain a PR here. It'd be a lot of work!

Also: what's the suggestion for handling factories?

@mikuhl-dev
Copy link

I'm willing to entertain a PR here. It'd be a lot of work!

Also: what's the suggestion for handling factories?

Couldn't you just use a static method? That way you can list.map(SomeStruct.fromJson) instead of list.map((json) => SomeStruct.fromJson(json))

@mikuhl-dev
Copy link

Excited to see this soon!

@kevmoo
Copy link
Collaborator

kevmoo commented Mar 17, 2020

Just to set expectations, NO ONE in the Dart org is working on this.

If we get a very clean PR, we'll consider it.

...we're very slammed working on null safety – while many of us are dealing with our wonderful children while we work from home. 😄

@bsutton
Copy link

bsutton commented Mar 17, 2020

It really felt like you meant to say ""wonderful children"".

@trevorwang
Copy link

I tried to implement this feature, but I encountered the below issue.

Here's the generated code.

extension PersonExt on Person {
  static Person fromJson(Map<String, dynamic> json) => _$PersonFromJson(json);
  Map<String, dynamic> toJson() => _$PersonToJson(this);
}

When json.encode(person) was invoked, the following error occurred.

NoSuchMethodError (NoSuchMethodError: Class 'Person' has no instance method 'toJson'.
Receiver: Instance of 'Person'
Tried calling: toJson())

I did some investigation, it seems that toJson() in extension cannot be found by convert library.
https://github.com/dart-lang/sdk/blob/b43714854e4d68907ab2dd033353e234a690cc6e/sdk/lib/convert/json.dart#L521

I posted test code #658 (comment) to demo this issue.

Any suggestion here?

@kevmoo
Copy link
Collaborator

kevmoo commented Jun 23, 2020

See #558 (comment) – I commented this in November.

We need dart-lang/language#252

@trevorwang
Copy link

Thanks for the reply. Clear get it.

@kevmoo
Copy link
Collaborator

kevmoo commented Sep 24, 2021

Closing this out. We'll likely get metaprogramming before we get partial classes.

Happy to re-open if language support comes along.

@kevmoo kevmoo closed this as completed Sep 24, 2021
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