Allow override of federation directives #1126
Conversation
🦋 Changeset detectedLatest commit: 69ae78c The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| '@authenticated', | ||
| '@policy', | ||
| '@requiresScopes', | ||
| ...federationDirectives, | ||
| options.composeDirectives ? '@composeDirective' : null, | ||
| hasInterfaceObjects ? '@interfaceObject' : null, |
There was a problem hiding this comment.
I think potentially it might make more sense to have authenticated, requiresScopes, and policy, and maybe even all the directives work like composeDirective and interfaceObject where they are only imported when used. I think being able to explicitly add your own directives is good, but I think we should potentially just auto-import any directive that gets used through a plugin feature
There was a problem hiding this comment.
Sure, makes sense - is there a simple way to derive at this point which of these directives have been used, either via (e.g.) the requiresScopes option or the directives array on each query/ mutation/ enum/ etc...?
There was a problem hiding this comment.
Not yet, I can try to find some time in the next few days to add something like that. I think it would probably be easiest to track that as the directives are added into the schema in the plugin class
|
Added some logic to track what directives are used, and defaulted the imported directives to only import what is used |
| @@ -265,7 +264,7 @@ builder.asEntity(Media, { | |||
|
|
|||
| See federation documentation for more details on `interfaceObject`s | |||
|
|
|||
| ### composeDirective | |||
| ### composeDirective = | |||
There was a problem hiding this comment.
Is this = a typo?
There was a problem hiding this comment.
Yup, not sure where that come from
There was a problem hiding this comment.
Looks like it was already in the Readme, which I copied here to keep it in sync
Awesome! Going to be using this straight away. Thanks. |
See issue 1125.