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 JSON types #61

Merged
merged 11 commits into from
Nov 9, 2020
Merged

Add JSON types #61

merged 11 commits into from
Nov 9, 2020

Conversation

ms1111
Copy link
Collaborator

@ms1111 ms1111 commented Oct 31, 2020

Fixes

#59 - JsonNode type

Description

Define explicit types for legal plain JSON values to avoid double-deserialization shenanigans.

Fixes #59.

Proposed changes in this PR

Adds JsonValue and JsonObject types to make ToJsonStrategy and FromJsonStrategy more specific.

Things to look at

  • Test coverage
  • Code Style
  • Documentation (READMEs etc)

@ms1111 ms1111 requested a review from hardy925 October 31, 2020 21:59
from_json/as.ts Outdated Show resolved Hide resolved
serialize_property_test.ts Outdated Show resolved Hide resolved
Define explicit types for legal plain JSON values to avoid
double-deserialization shenanigans.

Fixes #59.
@hardy613 hardy613 added documentation Improvements or additions to documentation v1.0.0 roadmap labels Nov 1, 2020
Copy link
Collaborator

@hardy613 hardy613 left a comment

Choose a reason for hiding this comment

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

I will probably pull this down to look into some things, but this is a great start! thanks @ms1111

serializable.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@ChrisDufourMB ChrisDufourMB left a comment

Choose a reason for hiding this comment

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

Looks great! Some comments around JsonValue vs JsonObject

serialize_property_test.ts Outdated Show resolved Hide resolved
serializable.ts Outdated Show resolved Hide resolved
serializable.ts Outdated Show resolved Hide resolved
@hardy925 hardy925 changed the base branch from support-ts-isolated-modules to develop November 4, 2020 08:38
@ms1111 ms1111 marked this pull request as ready for review November 8, 2020 04:10
hardy925
hardy925 previously approved these changes Nov 9, 2020
Copy link
Contributor

@hardy925 hardy925 left a comment

Choose a reason for hiding this comment

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

Thanks @ms1111 LGTM

@hardy925 hardy925 merged commit f50ed71 into develop Nov 9, 2020
@hardy925 hardy925 deleted the json-type branch November 9, 2020 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFC] - JsonNode type
4 participants