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

test @inaccessible in response formatting #1033

Merged
merged 6 commits into from
May 13, 2022
Merged

test @inaccessible in response formatting #1033

merged 6 commits into from
May 13, 2022

Conversation

Geal
Copy link
Contributor

@Geal Geal commented May 13, 2022

Fix #1030

@netlify
Copy link

netlify bot commented May 13, 2022

Deploy Preview for apollo-router-docs ready!

Name Link
🔨 Latest commit fc9ee35
🔍 Latest deploy log https://app.netlify.com/sites/apollo-router-docs/deploys/627e3cae4b37ce0009c96810
😎 Deploy Preview https://deploy-preview-1033--apollo-router-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions

This comment has been minimized.

@Geal Geal self-assigned this May 13, 2022
if @inaccessible is set on an object type, a client should not be able
to receive a value of that type. Instead, it should be replaced with a
null, without an error.

An inaccessible type will not appear in the API schema that we use for
response formatting, so when we get an object with a __typename field,
we check if that typ ename is known in the schema's object types
@Geal Geal marked this pull request as ready for review May 13, 2022 10:12
@Geal Geal requested review from abernix, bnjjj and BrynCooke May 13, 2022 10:12
Copy link
Contributor

@BrynCooke BrynCooke left a comment

Choose a reason for hiding this comment

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

LGTM,

Just to replay my understanding:

If the concrete type is not in the schema because it was inaccessible then we just zap the contents.
Otherwise if the field is inaccessible then the existing logic will take care of things as the field will be absent from the API schema.

"test_union": null,
"test_union2": {
"__typename": "B",
"common": "hello",
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, how come b is not output for union2?

Copy link
Contributor

Choose a reason for hiding this comment

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

Could be my mistake, or it could be a separate issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because the query does not call for this field:

test_union2: test_union {
    ...on B {
        __typename
        common
    }
}

@Geal
Copy link
Contributor Author

Geal commented May 13, 2022

If the concrete type is not in the schema because it was inaccessible then we just zap the contents.
Otherwise if the field is inaccessible then the existing logic will take care of things as the field will be absent from the API schema.

Both cases are taken care of by the API schema: an inaccessible object type won't appear in the API schema, in the same way an inaccessible field won't be in the schema

@Geal Geal enabled auto-merge (squash) May 13, 2022 11:11
@Geal Geal merged commit 534f744 into main May 13, 2022
@Geal Geal deleted the geal/test-inaccessible branch May 13, 2022 11:22
@Geal Geal added this to the v0.9.0 milestone May 13, 2022
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.

Inaccessible on object types (Contracts correctness)
4 participants