-
-
Notifications
You must be signed in to change notification settings - Fork 161
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
Support Client Directives in Prisma plugin (@skip and @include) #1017
Support Client Directives in Prisma plugin (@skip and @include) #1017
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: 1fc5b60 The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me! thanks for adding this ❤️
It would be great to add a test for these directives. The prisma tests all verify the queries that are executed, so adding a test that skips some relation field should be pretty straight forward |
me { | ||
id | ||
name @skip(if: true) | ||
email @include(if: false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you need to add these to a relation instead to see any difference, unless you query a type that is using select mode. This change only changes the generated query, and not what revolvers get run.
query {
me {
id
profile @skip(if: true) {
bio
}
posts(limit: 1) @include(if: false) {
id
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I just updated the test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome, thanks!
bccfd8c
to
66a62ac
Compare
@@ -65,6 +65,54 @@ describe('prisma', () => { | |||
`); | |||
}); | |||
|
|||
it('skips fields based on @skip and @include directives', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry to keep asking for more changes one at a time, but would also be nice to check the inverted condition, so that if the conditions is reversed the selection includes the expected relations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no worries! Just pushed one more commit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
Related to #723