Skip to content

Ironing JSONSerialization quirks#1478

Merged
designatednerd merged 2 commits intoapollographql:mainfrom
TizianoCoroneo:tiziano/null-jsonencodable
Oct 28, 2020
Merged

Ironing JSONSerialization quirks#1478
designatednerd merged 2 commits intoapollographql:mainfrom
TizianoCoroneo:tiziano/null-jsonencodable

Conversation

@TizianoCoroneo
Copy link
Copy Markdown
Contributor

Removed a workaround in the jsonValue property of Optional: the solution was to make NSDictionary conform to JSONEncodable.

Also fixed an issue where trying to serialize .some(NSNull()) would result in a crash.

BREAKING CHANGE (?): This slightly changes the result of the JSON serialization, because instead of taking a NSDictionary, casting to Dictionary and then calling jsonValue on it, it directly uses the NSDictionary as the JSONValue itself.
Previously, this resulted in having Bool values encoded as NSNumbers, while now they are encoded as a true or false JSON constants. IMHO, this is slightly more correct.

…ution was to make NSDictionary conform to JSONEncodable.

Also fixed an issue where trying to serialize `.some(NSNull())` would result in a crash.
Copy link
Copy Markdown
Contributor

@designatednerd designatednerd left a comment

Choose a reason for hiding this comment

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

I agree having true/false is more correct, so I'm fine with that change.

In theory this shouldn't be breaking, since it shouldn't produce different results under the hood with the exception of true/false vs 1/0.

Comment thread Tests/ApolloTests/JSONTests.swift
@designatednerd designatednerd merged commit 4b4084e into apollographql:main Oct 28, 2020
@designatednerd designatednerd added this to the Next Release milestone Oct 28, 2020
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.

2 participants