-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Forward directive arguments params to context type #9669
base: master
Are you sure you want to change the base?
Forward directive arguments params to context type #9669
Conversation
🦋 Changeset detectedLatest commit: fac1505 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 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 |
packages/plugins/other/visitor-plugin-common/src/base-resolvers-visitor.ts
Show resolved
Hide resolved
@dotansimha Any chance of getting a test run (and review) on this? Thx :) |
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.
nice tests
@saihaj @dotansimha What are we missing to get this merged? Is there anything more I can do? I see some checks are failing - but not sure it has to do with this PR (I see other PRs seems to fail similarly). |
@saihaj @dotansimha Any chance of getting this merged? Thanks :) |
Description
This adds any arguments passed to directives to the directiveContextTypes, allowing more for specific context types.
Here is an example:
With this PR we can customize the ContextType of each resolver to adapt to whatever arguments where passed to the @auth directive.
In our example we can respect both the
skip: true
(i.e. no token in context) as well ascmsCanModifyEntities: true
(our auth middleware has verified that the token contains this):Related # (issue)
#9668
Type of change
Please delete options that are not relevant.
Not 100% sure if we should consider this a breaking change, it expects the directiveContextTypes to accept 2 type parameters instead of 1.
Screenshots/Sandbox (if appropriate/relevant):
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
bar
) demonstrating the functionality in the current test suite.Test Environment:
@graphql-codegen/...
:Checklist: