Skip to content

Input object null vs undefined #87

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

Merged
merged 7 commits into from
Sep 21, 2021

Conversation

NeedleInAJayStack
Copy link
Member

This adds support to differentiate between explicitly providing the literal value null and implicitly not providing a value at all, as described in the spec here and here.

To do so, an undefined case is added to the Map enum, and argument parsing uses this value when expected arguments are not provided. Field default values now override only undefined maps.

The end result is that Map encoding works like JSON where a field with a literal null is contained by the decoding container, whereas an un-provided value is not. From here, custom object decodings can adjust functionality based on the two situations. For example:

struct Obj : Codable {
    let field1: String

    init(from decoder: Decoder) throws {
        let container = try decoder.container(keyedBy: CodingKeys.self)
        
        if (!container.contains(.field1)) {
            field1 = "I was not provided"
        } else {
            field1 = try container.decode(String?.self, forKey: .field1) ?? "I was provided, but null"
        }
    }
}

@paulofaria
Copy link
Member

@NeedleInAJayStack great work! We really needed this.

paulofaria
paulofaria previously approved these changes Sep 21, 2021
Copy link
Member

@paulofaria paulofaria left a comment

Choose a reason for hiding this comment

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

Everything looks good to me. Would be nice to check for other instances of Map? elsewhere in the codebase and reason if using undefined would make sense. I was reluctant about adding an undefined case before, but now I think we should've had it all along.

@NeedleInAJayStack
Copy link
Member Author

@paulofaria I removed the OrderedCollections import and removed the ubuntu-16.04 CI because it's been removed. Mind approving again after these changes?

@paulofaria
Copy link
Member

Done!

@NeedleInAJayStack
Copy link
Member Author

@paulofaria Argh, since the ubuntu 16-04 test started and never completed, I can't merge it (even though I removed it for the future). If you could merge, I'll tag and release!

@paulofaria paulofaria merged commit 834e2cf into GraphQLSwift:master Sep 21, 2021
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