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

Omit types, queries and mutations in production #2368

Closed
jaulz opened this issue Mar 26, 2023 · 9 comments · Fixed by #2392
Closed

Omit types, queries and mutations in production #2368

jaulz opened this issue Mar 26, 2023 · 9 comments · Fixed by #2392
Labels
discussion Requires input from multiple people

Comments

@jaulz
Copy link
Contributor

jaulz commented Mar 26, 2023

What problem does this feature proposal attempt to solve?

Sometimes it's helpful to have certain queries or types in development that are not supposed to be in production. Though we can handle it via field middleware, they will still be visible in production. It would be even better if we could remove queries or types completely depending on the environment or other flags.

Which possible solutions should be considered?

I think something like @env( name: "local" ) could be helpful to make queries, manipulations and types only available in certain environments.

@spawnia
Copy link
Collaborator

spawnia commented Mar 27, 2023

It is unclear to me what you are actually asking. Please reformulate your issue in terms of an actual problem and reflect that in the title.

@k0ka
Copy link
Collaborator

k0ka commented Mar 27, 2023

I guess he wanted to expose some fields/queries only for local tests.
I have something similar in my codebase but not sure if it should be included in the main library tree.

class DebugDirective extends BaseDirective implements FieldMiddleware
{
    private bool $debug;

    public function __construct(Repository $config)
    {
        $this->debug = $config->get('app.debug');
    }

    public static function definition(): string
    {
        return <<<GRAPHQL
"""
Limit field access to debug server
"""
directive @debug on FIELD_DEFINITION
GRAPHQL;
    }

    public function handleField(FieldValue $fieldValue, Closure $next): FieldValue
    {
        $originalResolver = $fieldValue->getResolver();

        return $next(
            $fieldValue->setResolver(
                function ($root, array $args, GraphQLContext $context, ResolveInfo $resolveInfo) use ($originalResolver) {
                    if(!$this->debug)
                        return null;

                    return $originalResolver($root, $args, $context, $resolveInfo);
                }
            )
        );
    }
}

@jaulz
Copy link
Contributor Author

jaulz commented Mar 27, 2023

@k0ka yeah, that's almost what I had in mind 😊 I actually even thought about hiding types or fields completely if that's possible?

@spawnia
Copy link
Collaborator

spawnia commented Mar 27, 2023

I think the issue title and description are insufficiently descriptive of what the intent is. Please update them.

@jaulz jaulz changed the title Add @env directive Omit types, queries and mutations in production Mar 27, 2023
@k0ka
Copy link
Collaborator

k0ka commented Apr 7, 2023

Removing fields from AST will be a little harder, but I can do it.

@spawnia do you think this issue make sense?

@LastDragon-ru
Copy link
Contributor

LastDragon-ru commented Apr 7, 2023

Sometimes it's helpful to have certain queries or types in development that are not supposed to be in production.

Why not use different location of the schema for specific env?

  • config/lighthouse.php
    // ...
    'schema_path' => env('LIGHTHOUSE_SCHEMA`, base_path('graphql/schema.graphql'))
    // ...
    
  • graphql/schema.graphql
    type Query {
      a: A!
    }
  • graphql/schema.dev.graphql
    #import schema.graphql
    
    extend type Query {
      b: B!
    }
  • .env.dev
    LIGHTHOUSE_SCHEMA=graphql/schema.dev.graphql
    LIGHTHOUSE_SCHEMA_CACHE_PATH=bootstrap/cache/lighthouse-schema.dev.php

Only one pitfall here - it is not possible to add directives via extend.

@spawnia
Copy link
Collaborator

spawnia commented Apr 8, 2023

I think we can widen the scope of this issue even further and make it about the conditional inclusion/exclusion of any schema element. For example, I could see this being useful for any field, not just those on the root types - as well as arguments, enum values, etc.

Depending on the environment (via APP_ENV or app()->environment()) would probably be the most common use case, but I could see other use cases too. Examples include but are not limited to:

  • the tenant of a multi-tenancy deployment
  • the point in time to control a release
  • the deployed version of the application
  • a feature toggle

Clients already have access to a feature that allows conditional inclusion or exclusion of fields controlled through variables via the client directives @skip and @include. We could reuse the idea of having symmetric directives for inclusion and exclusion, perhaps we could call the equivalent schema directives @show and @hide? Both would have the same set of possible arguments and just differ in that @show requires some condition to be true whereas @hide requires it to be false in order to include the schema element.

We could have a simple way for the most common use case of depending on the environment, perhaps like this: @show(env: ["local", "dev", "testing"]) or @hide(env: ["production"]).

For the most complex use cases, we could allow the user to define a function that will be called to determine inclusion: @show(if: "App\\Providers\\GraphQLServiceProvider@allowExperimental").

@LastDragon-ru
Copy link
Contributor

@spawnia, performance can be a problem - eg to hide Type, all types/fields/arguments should be iterated, for big schema it can be slow + schema cache seems will not help here.

@k0ka
Copy link
Collaborator

k0ka commented Apr 8, 2023

I guess hiding must be done during schema creation (via TypeManipulator interface) so it shouldn't affect base performance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Requires input from multiple people
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants