apollographql / apollo-ios

📱  A strongly-typed, caching GraphQL client for iOS, written in Swift.
https://www.apollographql.com/docs/ios/
MIT License
3.88k stars 722 forks source link

1.14.0 using introspection creates duplicate defer directive #3417

Closed loganblevins closed 2 months ago

loganblevins commented 2 months ago

Summary

See https://github.com/apollographql/apollo-ios/issues/2395#issuecomment-2246074517

Version

1.14.0

Steps to reproduce the behavior

I cannot share our private schema, but our schema line 1 contains directive defer.

Then I run introspection on 1.14.0 and it makes yet another defer directive and the json schema validation fails.

Logs

No response

Anything else?

No response

calvincestari commented 2 months ago

@loganblevins, I'm not able to replicate the error you describe when the schema only has a single defer directive definition. Here is a sample project I put together that contains three schemas:

We also have a test on our GraphQLCompiler module to detect this scenario and fail CI if our logic for this behaviour is changed. All tests passed on the release PR for 1.14.0.

If you're able to change the sample project to fail in the same way as you're seeing it would help in debugging this further. Otherwise if your schema is large it may be worth doing another search for directive @defer to ensure it's not buried somewhere deeper in the schema.

Then I run code gen on 1.14.0 and it makes yet another defer directive and the json schema validation fails.

What leads to you to believe that codegen is adding another directive definition? I don't think we output the schema, or store it locally for inspection, at any stage of code generation.

calvincestari commented 2 months ago

@calvincestari Thanks for quick reply! Is there a way I can disable this experimental feature in meantime? I couldn't find that key for my config json

No, there is no way to disable defer support. If there are no operations that use the defer directive then codegen outputs models as previous versions.

loganblevins commented 2 months ago

@calvincestari Here is my json config for context:

The endpoint is not public so you won't be able to introspect.

{
  "schemaNamespace" : "Federation",
  "input" : {
    "operationSearchPaths" : [
      "./Schema/**/*.graphql"
    ],
    "schemaSearchPaths" : [
      "./Schema/**/*.graphqls"
    ]
  },
  "output" : {
    "testMocks" : {
      "none" : {
      }
    },
    "schemaTypes" : {
    "path" : "./Sources/FederationAPI/Generated/Federation",
      "moduleType" : {
        "embeddedInTarget" : {
          "name" : "FederationAPI"
        }
      }
    },
    "operations" : {
      "inSchemaModule" : {
      }
    }
  },
  "schemaDownloadConfiguration": {
      "downloadMethod": {
          "introspection": {
              "endpointURL": "https://api.angelstudios.io/graphql",
              "httpMethod": {
                  "POST": {}
              },
              "includeDeprecatedInputValues": false,
              "outputFormat": "SDL"
          }
      },
      "downloadTimeout": 60,
      "headers": [],
      "outputPath": "./Schema/schema.graphqls"
  }
}
loganblevins commented 2 months ago

@calvincestari Here is my script I run to introspect and run code gen.

This command on 1.14.0 is creating the duplicate defer directive in my schema file.

if ! [ -x "$(command -v ./apollo-ios-cli)" ]; then
  echo 'apollo-ios-cli is not installed..installing now.'
  swift package --allow-writing-to-package-directory apollo-cli-install
fi

./apollo-ios-cli generate --verbose --path .././apollo-codegen-config.json --fetch-schema
loganblevins commented 2 months ago

If you're able to change the sample project to fail in the same way as you're seeing it would help in debugging this further. Otherwise if your schema is large it may be worth doing another search for directive @defer to ensure it's not buried somewhere deeper in the schema.

I mentioned in the other GH thread that our schema file already contains a defer directive prior to updating to 1.14.0 and the above command I run is adding another one.

loganblevins commented 2 months ago

Sorry if I said code gen... I run the plugin CLI tool with introspect and code gen at once so it may just be the introspect step that is the issue

calvincestari commented 2 months ago

OK, so is it the schema file that's output from the --fetch-schema stage that contains two defer directive definitions?

That schema file is simply the result of introspection and output. There is no defer-related manipulation done to the schema before output.

loganblevins commented 2 months ago

@calvincestari that is correct but only on 1.14.0

If I revert back to my prior tag 1.12.0 and introspect the diff is unchanged. If I introspect on 1.14.0 my diff shows

Screenshot 2024-07-23 at 16 56 49
loganblevins commented 2 months ago
Screenshot 2024-07-23 at 16 57 47

This directive is already in my schema file prior to introspecting on 1.14.0....but 1.14.0 adds on a duplicate

calvincestari commented 2 months ago

This is certainly weird. I'm confused how fetch-schema would be adding the directive.

Are you able to validate this with a separate tool like Apollo Studio or GraphiQL that will let you view the schema off your server too? Either of those should let you view, and download, the schema.

loganblevins commented 2 months ago

@calvincestari I don't know how to use the sample project you sent to replicate if it's the --fetch-schema that is the problem.

It definitely is a blocker bug because I am unable to update the package on my end and am blocked. I've been using this Apollo tool for a long time and never had this issue.

I just introspected now on 1.12.0 and have no issues.

loganblevins commented 2 months ago

@calvincestari While it is possible I could use Apollo Studio that is a very manual process and leaves the CLI in a broken state and unusable

calvincestari commented 2 months ago

@calvincestari I don't know how to use the sample project you sent to replicate if it's the --fetch-schema that is the problem.

No, if --fetch-schema is the issue that sample project won't help. It demonstrates that the code generation build phase does not add a duplicate directive - but to an already existing schema file with only a single defer directive definition.

While it is possible I could use Apollo Studio that is a very manual process and leaves the CLI in a broken state and unusable

The CLI will get fixed if there is an issue there. I'm still trying to figure out the root cause.

calvincestari commented 2 months ago

OK, I've gotten to a state where I can replicate the diff via introspection. Give me some time to try figure out how this is happening.

loganblevins commented 2 months ago

@calvincestari Awesome thanks

calvincestari commented 2 months ago

~I think the problem is in graphql-js~ which we use for validating a schema, compiling a schema, and converting introspection JSON into GraphQL SDL. The downloaded introspection JSON only contains a single defer directive definition but after the call to printSchemaToSDL the output then contains two instances of the defer directive definition.

calvincestari commented 2 months ago

I've got a ~draft~ PR up at https://github.com/apollographql/apollo-ios-dev/pull/440. ~with one outstanding JS test to get working then I think we'll be good to go~.

loganblevins commented 2 months ago

@calvincestari Cool thanks!

github-actions[bot] commented 2 months ago

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.

calvincestari commented 2 months ago

The PR to resolve this has been merged and the fix will go out in the next release, we're aiming to publish that this week still.

loganblevins commented 2 months ago

@calvincestari super grateful - thank you!

loganblevins commented 2 months ago

Working!! Thank you!