Skip to content
This repository has been archived by the owner on Nov 14, 2018. It is now read-only.

Removed generation of double Optionals for properties of Input type #9

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Removed generation of double Optionals for properties of Input type #9

wants to merge 1 commit into from

Conversation

MarioBajr
Copy link

@MarioBajr MarioBajr commented Jul 4, 2018

For objects of type Input, the Swift code generation is creating unnecessary layers of Optionality for all Optional properties.

Given this GraphQL input type

input ReviewInput {
    stars: Int!
    commentary: String
    favoriteColor: ColorInput
}

The code generated has two layers of Optionality. eg: Optional<String?>

public struct ReviewInput: GraphQLMapConvertible {
    public init(stars: Int, 
                commentary: Optional<String?> = nil, 
                favoriteColor: Optional<ColorInput?> = nil) {
        ...
    }
}

This PR replaces to a single Optionality level, aligning with all other generated types.

public struct ReviewInput: GraphQLMapConvertible {
    public init(stars: Int, 
                commentary: String? = nil, 
                favoriteColor: ColorInput? = nil) {
        ...
    }
}

@minbi
Copy link
Contributor

minbi commented Jul 5, 2018

Thanks @MarioBajr ,

I will discuss with the team about the change and update you with the decision.

@frankmuellr
Copy link

Hello @MarioBajr

We will be retiring this repo soon as the new [AWS Amplify CLI](url
https://github.com/aws-amplify/amplify-cli) has the codegen features, improvements, and more, including automatic graphql generation as well as a GraphQL transformer:
https://github.com/aws-amplify/amplify-cli/blob/master/native_guide.md

Could you look to see if the new AWS Amplify CLI flow resolves your issue and if not open something in that repo?

@MarioBajr
Copy link
Author

MarioBajr commented Oct 4, 2018

Hi @muellerfr,
thanks for the updates, while evaluating amplify-cli, I found that this project (aws-appsync-codegen) is imported as a dependency, being the responsible for the Swift code generation.
https://github.com/aws-amplify/amplify-cli/blob/master/packages/amplify-codegen/package.json#L17
https://github.com/aws-amplify/amplify-cli/blob/master/packages/amplify-codegen/src/commands/types.js#L39

Is the new codegen feature that you guys are planning to use available anywhere else?
Cheers

@rohandubal rohandubal self-requested a review November 14, 2018 01:08
@rohandubal rohandubal self-assigned this Nov 14, 2018
@frankmuellr
Copy link

Hi @MarioBajr, we will merge this change into the https://github.com/aws-amplify/amplify-cli repo, where this code now lives.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants