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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Introspection-based schema download creates duplicate @defer directive #440

Merged
merged 10 commits into from
Jul 30, 2024

Conversation

calvincestari
Copy link
Member

@calvincestari calvincestari commented Jul 25, 2024

Fixes apollographql/apollo-ios#3417

This changeset splits the logic for adding the defer directive to the document between SDL and introspection sources. SDL sources can be inspected as a DocumentNode with all declared directives available but introspection sources, once converted to a DocumentNode, do not contain all declared directives and instead need to be inspected as the schema built from the introspection source; I do not know whether this is a bug in graphql-js or intended behaviour but I will take that up with them separately.

  • Adds JS tests for introspection sources with the same pattern as SDL source tests; valid directive, unsupported directive, and no directive.
  • Adds a Swift-based introspection test to the ApolloSchemaDownloaderInternalTests to ensure that even though the JS logic has been fixed there isn't some fetch-schema-workflow specific difference to creating the duplicate directive.
  • Splits the defer directive addition logic between SDL and introspection sources.

Copy link

netlify bot commented Jul 25, 2024

Deploy Preview for eclectic-pie-88a2ba canceled.

Name Link
🔨 Latest commit 43b2187
🔍 Latest deploy log https://app.netlify.com/sites/eclectic-pie-88a2ba/deploys/66a2ddff849e260008a60ba5

Copy link

netlify bot commented Jul 25, 2024

Deploy Preview for apollo-ios-docc canceled.

Name Link
🔨 Latest commit 43b2187
🔍 Latest deploy log https://app.netlify.com/sites/apollo-ios-docc/deploys/66a2ddff756b720008030456

@calvincestari calvincestari marked this pull request as ready for review July 25, 2024 23:17
new Source(documentString, "Test Query", { line: 1, column: 1 })
);

it("ensure the introspection JSON is correct for test", () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

Introspection JSON is difficult to parse and read so I've added tests like this one to ensure that the JSON doesn't get changed in a way that would invalidate what the test is meant to be checking for.

Copy link
Member

@BobaFetters BobaFetters left a comment

Choose a reason for hiding this comment

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

LGTM

@calvincestari calvincestari merged commit df28132 into main Jul 30, 2024
33 checks passed
@calvincestari calvincestari deleted the fix/introspection-duplicate-defer-directive branch July 30, 2024 17:05
BobaFetters pushed a commit to apollographql/apollo-ios-codegen that referenced this pull request Jul 30, 2024
BobaFetters pushed a commit that referenced this pull request Jul 30, 2024
83488f68 fix: Introspection-based schema download creates duplicate `@defer` directive (#440)

git-subtree-dir: apollo-ios-codegen
git-subtree-split: 83488f68370f0c7ec5bb3239218c737a1b7a9650
BobaFetters pushed a commit that referenced this pull request Jul 30, 2024
… download creates duplicate `@defer` directive

git-subtree-dir: apollo-ios-codegen
git-subtree-mainline: b14cab1
git-subtree-split: 83488f68370f0c7ec5bb3239218c737a1b7a9650
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.

1.14.0 using introspection creates duplicate defer directive
2 participants