Skip to content

feat: implement first version of sealed class serialization#1483

Open
O-Hannonen wants to merge 4 commits intogoogle:masterfrom
O-Hannonen:feat/1342-implement-sealed-class-serialization
Open

feat: implement first version of sealed class serialization#1483
O-Hannonen wants to merge 4 commits intogoogle:masterfrom
O-Hannonen:feat/1342-implement-sealed-class-serialization

Conversation

@O-Hannonen
Copy link
Copy Markdown

Whats included

This PR is a proposal related to #1342. Since the issue asked for a PR only to evaluate if adding this functionality is worth the added complexity, this PR DOES NOT (yet) include:

  • tests for the new functionality
  • documentation for the new functionality
  • and consideration for all the different edge cases (eg. error handling for unsupported config combos)

I'd be happy to contribute towards those too if the maintainers give green light.

Things to discuss / consider

  • Should genericArgumentFactories be supported and how? Or should it warn / throw as invalid config combo?
  • Should there be a UnionKey annotation similar to JsonKey allowing naming change per sealed implementation?
  • Should the generator warn / throw if discriminator is also present as sealed class implementations field? Would lead to conflict in generated toJson method where class fields are added to the output with spread operator

Comment thread json_serializable/lib/src/decode_helper.dart
Copy link
Copy Markdown
Collaborator

@kevmoo kevmoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm impressed, yo. A lot of folks have asked for this.

I think I like your approach. You're absolutely right, the devil is in the details here...lots of overlapping issues.

I'd avoid the generic factories for now. It'll likely be a HUGE mess that won't affect most folks.

If you look at many of my previous PRs, there is usually more code in tests than there is in the actual implementation.

If you're game to run with what you have, I'm happy to support you. Very cool!

Comment thread json_serializable/lib/src/decode_helper.dart Outdated
Comment thread json_serializable/lib/src/decode_helper.dart Outdated
Comment thread json_serializable/lib/src/decode_helper.dart Outdated
Comment thread json_serializable/lib/src/utils.dart Outdated
@O-Hannonen
Copy link
Copy Markdown
Author

Superb! I'll continue working on this over the weekend.

Haven't checked the testing practices of this project at all yet so might need some guidance on those, but let's see

@kevmoo
Copy link
Copy Markdown
Collaborator

kevmoo commented Apr 3, 2025

I highly recommend looking through other PRs that change something substantial. You'll get inspiration!

Comment thread json_annotation/pubspec.yaml Outdated
Comment thread json_serializable/README.md Outdated
Comment on lines +113 to +127
if (element.isSealed) {
sealedSubClasses(element).forEach((sub) {
final annotationConfig = jsonSerializableConfig(sub, _generator);

if (annotationConfig == null) {
throw InvalidGenerationSourceError(
'The class `${element.displayName}` is sealed but its '
'subclass `${sub.displayName}` is not annotated with '
'`JsonSerializable`.',
todo: 'Add `@JsonSerializable` annotation to ${sub.displayName}.',
element: sub,
);
}
});
}
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the other checks we can do when generating subclass but not this one as the generator will never get there for the subclass if its not annotated. IMO would be cleaner if all the checks would be done with lookups from subclass to superclass (would not need to use the sealedSubClasses method here yet).

Should sealedSubClasses be passed to decode/encode helpers if its already evaluated here? Now its called again in the helpers...

Comment thread json_serializable/test/json_serializable_test.dart
@O-Hannonen
Copy link
Copy Markdown
Author

@kevmoo Did I miss any type of tests that would need to be added? Or combos of unsupported configurations?

@O-Hannonen O-Hannonen requested a review from kevmoo April 7, 2025 18:55
@kevmoo
Copy link
Copy Markdown
Collaborator

kevmoo commented Jun 2, 2025

you'll need to rebase here. I'd wait until my Dart 3.8 change lands, though.

@kevmoo
Copy link
Copy Markdown
Collaborator

kevmoo commented Jun 11, 2025

rebase, please!

@O-Hannonen O-Hannonen force-pushed the feat/1342-implement-sealed-class-serialization branch from 167aad6 to 55a821b Compare June 13, 2025 15:22
Comment thread json_serializable/pubspec.yaml Outdated
@O-Hannonen
Copy link
Copy Markdown
Author

Sorry for the delay, rebase is now done! @kevmoo

Comment thread json_annotation/lib/src/json_serializable.dart Outdated
Comment thread json_annotation/lib/src/json_serializable.dart Outdated
@kevmoo
Copy link
Copy Markdown
Collaborator

kevmoo commented Jul 24, 2025

willing to do a rebase here?

@O-Hannonen O-Hannonen force-pushed the feat/1342-implement-sealed-class-serialization branch from 21972ed to b96ed81 Compare August 7, 2025 11:04
@O-Hannonen
Copy link
Copy Markdown
Author

@kevmoo sorry for the delay due to holidays, rebase is now done!

@O-Hannonen O-Hannonen force-pushed the feat/1342-implement-sealed-class-serialization branch 2 times, most recently from 00e16ce to 90bbeab Compare December 21, 2025 11:41
Comment thread json_annotation/lib/src/json_serializable.dart Outdated
Comment thread json_serializable/lib/src/decode_helper.dart Outdated
Comment thread json_serializable/lib/src/utils.dart Outdated
@hai-trung-le
Copy link
Copy Markdown

Any update?

@O-Hannonen O-Hannonen force-pushed the feat/1342-implement-sealed-class-serialization branch from 5e93427 to d449416 Compare April 17, 2026 10:48
Copy link
Copy Markdown
Collaborator

@kevmoo kevmoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The amount of code churn here is HUGE!

(1) Did you do other, unrelated refactoring? Please minimize this so it's easier to review
(2) Could you write up a 1-page review of what you're proposing

  • What does this enable
  • What does a user of json_serializable have to do to make this work
  • What is changed in how json_serial is implemented to make it happen

I'm not against these changes, but I need some front-loading!

@O-Hannonen
Copy link
Copy Markdown
Author

The large diff is primarily due to extensive integration tests and examples for the new sealed class support. I did refactor decode_helper.dart and encoder_helper.dart to ensure the code is readable even after this "bigger" new feature. I can totally revert the refactoring if that's preferred, but I think the sealed class implementation would become a bit messy then.

Sealed Class Support Overview

What it enables:
Native support for Dart 3 sealed classes. json_serializable can now generate a central fromJson factory on a base sealed class that dispatches to subclasses using a discriminator field (defaulting to type), and a toJson that includes this discriminator.

User Requirements:

  1. Annotate the base sealed class and all subclasses with @JsonSerializable().
  2. Add fromJson and toJson to the base class that call the generated _$BaseClassFromJson and _$BaseClassToJson.
  3. Subclasses maintain their own standard fromJson/toJson factories.
  4. Customization is available via unionDiscriminator and unionRename options.

Implementation Changes:

  1. Annotation Update: Added unionDiscriminator and unionRename to JsonSerializable.
  2. Validation: GeneratorHelper now validates sealed hierarchies for consistent annotations and configurations.
  3. Refactored Logic: DecodeHelper and EncodeHelper generate switch statements for polymorphic dispatching of sealed classes
  4. Analyzer Utilities: Added helpers in utils.dart to navigate sealed structures using the analyzer API.

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

Successfully merging this pull request may close these issues.

3 participants