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

Add JsonConverter(converters) option #1135

Merged
merged 13 commits into from May 24, 2022

Conversation

rrousselGit
Copy link
Contributor

@rrousselGit rrousselGit commented Apr 21, 2022

This implements what is described here #1072 (comment) (assuming this is desirable)

That should simplify a bit the work of having to reuse a few JsonConverter over and over by allowing folks to make a custom reusable annotation or write things like:

const myConverters = [DocumentReferenceConverter()];

@JsonSerializable(converters: myConverters);

fixes #1072

@rrousselGit
Copy link
Contributor Author

I had to separate ClassConfig and JsonSerializable for this (because ClassConfig receives DartObjects for the converters but JsonSerializable receives a JsonConverter).

Hopefully that's alright.

@kevmoo
Copy link
Collaborator

kevmoo commented Apr 26, 2022

I had to separate ClassConfig and JsonSerializable for this (because ClassConfig receives DartObjects for the converters but JsonSerializable receives a JsonConverter).

Hopefully that's alright.

Ha! We'll start by seeing if the tests pass. 😁

@rrousselGit
Copy link
Contributor Author

The CI just wanted a dependency_overrides for the annotation 😄

@rrousselGit
Copy link
Contributor Author

I'm not quite sure how to fix that last CI error

One test seems to complain if I don't update the readme with 4.6.0 links. But the markdown checked complains that 4.6.0 links are 404 (which is logical since it isn't published yet)

@kevmoo
Copy link
Collaborator

kevmoo commented May 24, 2022

ignore the markdown errors. That's expected when things are revved!

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

Sweet!

@kevmoo kevmoo merged commit 8aadb45 into google:master May 24, 2022
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.

Add json allowlist of specific types
2 participants