add add-graphql option includeSchemaDirecties - #1587
Conversation
|
|
@cometkim is attempting to deploy a commit to the Michael Hayes' projects Team on Vercel. A member of the Team first needs to authorize it. |
@pothos/core
@pothos/plugin-add-graphql
@pothos/plugin-complexity
@pothos/plugin-dataloader
@pothos/plugin-directives
@pothos/plugin-drizzle
@pothos/plugin-errors
@pothos/plugin-example
@pothos/plugin-federation
@pothos/plugin-grafast
@pothos/plugin-mocks
@pothos/plugin-prisma
@pothos/plugin-prisma-utils
@pothos/plugin-relay
@pothos/plugin-scope-auth
@pothos/plugin-simple-objects
@pothos/plugin-smart-subscriptions
@pothos/plugin-sub-graph
@pothos/plugin-tracing
@pothos/plugin-validation
@pothos/plugin-with-input
@pothos/plugin-zod
@pothos/tracing-newrelic
@pothos/tracing-opentelemetry
@pothos/tracing-sentry
@pothos/tracing-xray
commit: |
hayes
left a comment
There was a problem hiding this comment.
Thanks for the PR! I probably can't merge this in its current state, but I can look into supporting this properly.
What format are the directives in in your current schema that they don't show up in extensions with the current implementation?
The main issue with the current implementation is that I try to avoid dependencies in pothos wherever possible (with a few exceptions the only dep in pothos packages is a peer dependency on graphql)
|
I just pushed up a change that adds astNode support to core, as well as copying astNode info for types added with the add-graphQL plugin. This won't add the directive definitions to the schema, but it MAY remove the need for the extracting of directives and re-adding them as extensions |
| const { schema: addedSchema, includeSchemaDirectives = false } = this.builder.options.add ?? {}; | ||
|
|
||
| if (includeSchemaDirectives && addedSchema) { | ||
| return addTypes(schema, [...addedSchema.getDirectives()]); |
There was a problem hiding this comment.
I am curious if after the change I just published if this is the only part that is still needed
There was a problem hiding this comment.
This is the part for including the directive definition. And I noticed addTypes is required for rewiring duplicated definitions by the directive. But yeah, maybe manually redefining the directive is simpler.
It's worth noting that the add-graphql plugin doesn't handle this. I was confused.
|
|
Still working on something for this. Hoping to have something this weekend. Directives are kinda weird and complex. I got sucked down the rabbit hole a bit, but I think automatically adding referenced directives should make sense (as long as the directive plugin is also added) |
While I'm doing incremental migration to Pothos using the add-graphql plugin,
I noticed that directives from
originalSchemaare omitted. Even withprintSchemaWithDirectives()from@graphql-tools/utils, it doesn't generate directives in the SDL.This was important to me because I needed to verify that the new schema provided the same functionality (including the printer) as the existing schema.
I made this patch to fix it, but I'm not sure it can be merged upstream.