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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

DefaultValue Code Generation for InputObject When DefaultValue is given from Schema #2997

Closed
hkjlee109 opened this issue May 8, 2023 · 4 comments 路 Fixed by apollographql/apollo-ios-dev#358
Labels
codegen Issues related to or arising from code generation enhancement Issues outlining new things we want to do or things that will make our lives as devs easier good first issue Issues that are suitable for first-time contributors. planned-next Slated to be included in the next release

Comments

@hkjlee109
Copy link

hkjlee109 commented May 8, 2023

Question

This is neither a bug or feature request but rather a small question/suggestion. 馃檪

Backend team added a new field for an input object. In the schema, it is shown as below.

          {
            "name": "sorting",
            "description": null,
            "type": {
              "kind": "ENUM",
              "name": "SortingEnum",
              "ofType": null
            },
            "defaultValue": "ASCENDING"
          },

In this case, Apollo Code Gen doesn't set any default value. Looking at the test code, looks like it's an intended behavior.

https://github.com/apollographql/apollo-ios/blob/1.1.3/Tests/ApolloCodegenTests/CodeGeneration/Templates/InputObjectTemplateTests.swift#L466

  func test__render__given_NullableField_WithDefault__generates_NullableParameter_NoInitializerDefault() throws {
    // given
    buildSubject(fields: [
      GraphQLInputField.mock("nullableWithDefault", type: .scalar(.integer()), defaultValue: .int(3))
    ])

    let expected = """
      public init(
        nullableWithDefault: GraphQLNullable<Int>   ### No default value set.
      ) {
        __data = InputDict([
          "nullableWithDefault": nullableWithDefault
        ])
      }

      public var nullableWithDefault: GraphQLNullable<Int> {
    """

It's not a big deal. We can just manually set the field when we call this InputObject. eg. nullableWithDefault = nil
But the thing is, for this incident, IOS was the only platform having a build break after fetching GraphQL schema.

Web and Android, looks like they are setting the default value as null in this case and had no issue.
If field is Optional(Nullable) and defaultValue is given in the schema, they ignore the given defaultValue in the schema and defaults it to null.

Wouldn't it be better if we change the expected behavior as below? 馃 Just for a consistency across platforms.

  func test__render__given_NullableField_WithDefault__generates_NullableParameter_NoInitializerDefault() throws {
    // given
    buildSubject(fields: [
      GraphQLInputField.mock("nullableWithDefault", type: .scalar(.integer()), defaultValue: .int(3))
    ])

    let expected = """
      public init(
        nullableWithDefault: GraphQLNullable<Int> = nil  ### <--- Can this be like this?
      ) {
        __data = InputDict([
          "nullableWithDefault": nullableWithDefault
        ])
      }

      public var nullableWithDefault: GraphQLNullable<Int> {
    """
@hkjlee109 hkjlee109 added the question Issues that have a question which should be addressed label May 8, 2023
@BobaFetters
Copy link
Member

Hey @hkjlee109 thanks for calling this out! This is something we should update for consistency with the other client platforms and will look to do in an upcoming patch release.

@BobaFetters BobaFetters added this to the Patch Releases (1.1.x) milestone May 8, 2023
@BobaFetters BobaFetters added the enhancement Issues outlining new things we want to do or things that will make our lives as devs easier label May 8, 2023
@calvincestari calvincestari added codegen Issues related to or arising from code generation and removed question Issues that have a question which should be addressed labels May 8, 2023
@hkjlee109
Copy link
Author

Nice! Thanks, guys. 馃檪

Copy link

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo iOS usage and allow us to serve you better.

@AnthonyMDev
Copy link
Contributor

This will be included in the next release!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
codegen Issues related to or arising from code generation enhancement Issues outlining new things we want to do or things that will make our lives as devs easier good first issue Issues that are suitable for first-time contributors. planned-next Slated to be included in the next release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants