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

Extending the Root Query Type #1656

Closed
shellscape opened this issue Jul 30, 2022 · 7 comments
Closed

Extending the Root Query Type #1656

shellscape opened this issue Jul 30, 2022 · 7 comments

Comments

@shellscape
Copy link

Summary

I'm doing what is probably a silly thing. I've defined a directive and am attempting to extend Query with that directive. The probably silly thing is within TagQueryPlugin where I'm attempting to use that plugin to add the directive - reason being that I'm seeking a smooth way to enable other devs in my org to add that directive. Nothing is throwing an error, but I'm unable to see the Symbol I've created in scope for the context, within my LogScopePlugin plugin.

Any pointers on where I've gone off the rails?

Additional context

Here's the code I'm playing with at the moment:

export const ALLOWEDIF = Symbol('allowed-if');

export const AllowedDirective: Plugin = (builder) => {
  builder.hook('GraphQLSchema', (schemaConfig, build) => {
    return {
      ...schemaConfig,
      directives: [
        ...(schemaConfig.directives || []),
        build.newWithHooks(
          GraphQLDirective,
          {
            name: 'allowed',
            locations: ['OBJECT', 'FIELD_DEFINITION'],
            args: { if: { type: GraphQLString } }
          } as GraphQLDirectiveConfig,
          { [ALLOWEDIF]: true }
        )
      ]
    };
  });
};

export const LogScopePlugin = makeWrapResolversPlugin(
  (context: any) => context,
  ({ scope }) => {
    console.log(scope[ALLOWEDIF]);
    return async (resolver, user, args, context, info) => resolver(user, args, context, info);
  }
);

export const TagQueryPlugin = makeExtendSchemaPlugin({
  typeDefs: gql`
    extend type Query @allowed(if: "foo")
  `,
  resolvers: {}
});
@benjie
Copy link
Member

benjie commented Jul 30, 2022

Directives that the client can apply (those valid against operations, fields, fragments, selection sets, etc) become part of the schema. The other directive locations (field definition, object, etc) only become part of the schema if they are in the GraphQL spec, such as @deprecated, @skip, @include, etc. There's a long-running GitHub thread on this topic: graphql/graphql-spec#300

Adding a server-side directive to PostGraphile is currently meaningless. If you're doing this so that people may use the directive in makeExtendSchemaPlugin then it's unnecessary - makeExtendSchemaPlugin currently just reads any arbitrary directive application and makes the information available to the plugin system via scope.directives, it does not validate that it's a know directive, that it has the right arguments, or anything like that. Here's a method that looks for the @pgSubscription directive from makeExtendSchemaPlugin in order to add subscriptions; there's no definition of that directive anywhere:

https://github.com/graphile/graphile-engine/blob/1bc8cfefdab7a61fd7ad287bcdff66298352e308/packages/pg-pubsub/src/PgSubscriptionResolverPlugin.ts#L64-L69

I'm unable to see the Symbol I've created in scope for the context, within my LogScopePlugin plugin.

Graphile Build doesn't currently have a hook system around directives; you can only hook certain GraphQL types: https://github.com/graphile/graphile-engine/blob/1bc8cfefdab7a61fd7ad287bcdff66298352e308/packages/graphile-build/src/makeNewBuild.js#L336 so your LogScopePlugin won't be being called for the directive.

@benjie
Copy link
Member

benjie commented Jul 30, 2022

I hadn't noticed that the code block had a scrollbar 🤦‍♂️

extend type Query @allowed(if: "foo") is meaningless in makeExtendSchemaPlugin right now, as far as I know. It does nothing.

@shellscape
Copy link
Author

Understood. What's the method to get that serverside directive added to scope.directives for Query if not with makeExtendSchemaPlugin?

@benjie
Copy link
Member

benjie commented Jul 30, 2022

scope.directives is currently only populated when the type is defined (not when it is extended). I guess you could retro-fit it in afterwards, but there'll be an ordering issue where some plugins might run before the directives are added (and thus not be affected by them). PostGraphile generally doesn't care about server-side directives (because most directives just change the schema, and you can just change the schema directly with Graphile Build anyway) so I'm afraid there's no particular flow for it.

@shellscape
Copy link
Author

OK thanks, that gives me an idea about how to accomplish the goal. I can see what you mean about plugin ordering, that will be tricky. You're welcome to close this, I'll update this issue if/as I make progress.

@shellscape
Copy link
Author

Merging typeDefs seems like a possibility:

import { inspect } from 'util';

import { mergeTypeDefs } from '@graphql-tools/merge';

const result = mergeTypeDefs([`type Query`, `type Query @deprecated`]);

console.log(inspect(result, { depth: 10, colors: true }));

results in:

{
  kind: 'Document',
  definitions: [
    {
      name: { kind: 'Name', value: 'Query', loc: { start: 5, end: 10 } },
      description: undefined,
      kind: 'ObjectTypeDefinition',
      loc: { start: 0, end: 22 },
      fields: [],
      directives: [
        {
          kind: 'Directive',
          name: {
            kind: 'Name',
            value: 'deprecated',
            loc: { start: 12, end: 22 }
          },
          arguments: [],
          loc: { start: 11, end: 22 }
        }
      ],
      interfaces: []
    },
    {
      kind: 'SchemaDefinition',
      operationTypes: [
        {
          kind: 'OperationTypeDefinition',
          type: {
            kind: 'NamedType',
            name: {
              kind: 'Name',
              value: 'Query',
              loc: { start: 5, end: 10 }
            }
          },
          operation: 'query'
        }
      ]
    }
  ]
}

The directive is there, and that's clutch. The idea being a dev could perhaps specify a simplified schema annotated with directives (I'm sure that's turning up a nose but this is all an exercise)

@benjie
Copy link
Member

benjie commented Jan 3, 2023

I'm going to close this because it doesn't seem actionable. We'd need to come up with a plan for defining exactly what directives mean and how they're used in PostGraphile and then set about implementing that. Feel free to open a discussion about that, but it's a very low priority for me.

@benjie benjie closed this as completed Jan 3, 2023
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

No branches or pull requests

2 participants