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.

Sign up for GitHub

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

Introduce StandardTypeRegistry #1426

Closed
wants to merge 9 commits into from
Closed

Introduce StandardTypeRegistry #1426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
47530c7
Introduce StandardTypeRegistry
ruudk Aug 14, 2023
287e1e9
Apply feedback
ruudk Aug 14, 2023
0d3ff24
Extract BuiltInDirectiveRegistry
ruudk Aug 14, 2023
fb12480
Deprecate all static getters on Type
ruudk Aug 14, 2023
1494762
Apply php-cs-fixer changes
ruudk Aug 14, 2023
193a8ef
Fix type
ruudk Aug 15, 2023
12252ac
Prettify docs
ruudk Aug 15, 2023
432cc04
Remove Introspection from StandardTypeRegistry
ruudk Aug 15, 2023
9d7d6e0
Prettify docs
ruudk Aug 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions docs/class-reference.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,8 @@ Registry of standard GraphQL types and base class for all other types.

```php
/**
* @deprecated use the `typeRegistry` on the `Schema` instead
*
* @api
*
* @throws InvariantViolation
Expand All @@ -181,6 +183,8 @@ static function int(): GraphQL\Type\Definition\ScalarType

```php
/**
* @deprecated use the `typeRegistry` on the `Schema` instead
*
* @api
*
* @throws InvariantViolation
Expand All @@ -190,6 +194,8 @@ static function float(): GraphQL\Type\Definition\ScalarType

```php
/**
* @deprecated use the `typeRegistry` on the `Schema` instead
*
* @api
*
* @throws InvariantViolation
Expand All @@ -199,6 +205,8 @@ static function string(): GraphQL\Type\Definition\ScalarType

```php
/**
* @deprecated use the `typeRegistry` on the `Schema` instead
*
* @api
*
* @throws InvariantViolation
Expand All @@ -208,6 +216,8 @@ static function boolean(): GraphQL\Type\Definition\ScalarType

```php
/**
* @deprecated use the `typeRegistry` on the `Schema` instead
*
* @api
*
* @throws InvariantViolation
Expand Down Expand Up @@ -548,6 +558,7 @@ typeLoader?: TypeLoader|null,
assumeValid?: bool|null,
astNode?: SchemaDefinitionNode|null,
extensionASTNodes?: array<SchemaExtensionNode>|null,
typeRegistry?: null|(StandardTypeRegistry&BuiltInDirectiveRegistry),
}

### GraphQL\Type\SchemaConfig Methods
Expand Down Expand Up @@ -2418,6 +2429,7 @@ assumeValidSDL?: bool
* @phpstan-param TypeConfigDecorator|null $typeConfigDecorator
*
* @param array<string, bool> $options
* @param (StandardTypeRegistry&BuiltInDirectiveRegistry)|null $typeRegistry
*
* @phpstan-param BuildSchemaOptions $options
*
Expand All @@ -2429,7 +2441,13 @@ assumeValidSDL?: bool
* @throws InvariantViolation
* @throws SyntaxError
*/
static function build($source, ?callable $typeConfigDecorator = null, array $options = []): GraphQL\Type\Schema
static function build(
$source,
?callable $typeConfigDecorator = null,
array $options = [],
$typeRegistry = null,
?GraphQL\Type\Introspection $introspection = null
): GraphQL\Type\Schema
```

```php
Expand All @@ -2444,6 +2462,7 @@ static function build($source, ?callable $typeConfigDecorator = null, array $opt
* @phpstan-param TypeConfigDecorator|null $typeConfigDecorator
*
* @param array<string, bool> $options
* @param (StandardTypeRegistry&BuiltInDirectiveRegistry)|null $typeRegistry
*
* @phpstan-param BuildSchemaOptions $options
*
Expand All @@ -2457,7 +2476,9 @@ static function build($source, ?callable $typeConfigDecorator = null, array $opt
static function buildAST(
GraphQL\Language\AST\DocumentNode $ast,
?callable $typeConfigDecorator = null,
array $options = []
array $options = [],
$typeRegistry = null,
?GraphQL\Type\Introspection $introspection = null
): GraphQL\Type\Schema
```

Expand Down
26 changes: 11 additions & 15 deletions src/Executor/ReferenceExecutor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
use GraphQL\Language\AST\SelectionNode;
use GraphQL\Language\AST\SelectionSetNode;
use GraphQL\Type\Definition\AbstractType;
use GraphQL\Type\Definition\Directive;
use GraphQL\Type\Definition\FieldDefinition;
use GraphQL\Type\Definition\InterfaceType;
use GraphQL\Type\Definition\LeafType;
Expand Down Expand Up @@ -467,7 +466,7 @@ protected function shouldIncludeNode(SelectionNode $node): bool
$variableValues = $this->exeContext->variableValues;

$skip = Values::getDirectiveValues(
Directive::skipDirective(),
$this->exeContext->schema->typeRegistry->skipDirective(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still feels off, why would the $typeRegistry deal with directives?

$node,
$variableValues
);
Expand All @@ -476,7 +475,7 @@ protected function shouldIncludeNode(SelectionNode $node): bool
}

$include = Values::getDirectiveValues(
Directive::includeDirective(),
$this->exeContext->schema->typeRegistry->includeDirective(),
$node,
$variableValues
);
Expand Down Expand Up @@ -661,23 +660,20 @@ protected function resolveField(ObjectType $parentType, $rootValue, \ArrayObject
*/
protected function getFieldDef(Schema $schema, ObjectType $parentType, string $fieldName): ?FieldDefinition
{
static $schemaMetaFieldDef, $typeMetaFieldDef, $typeNameMetaFieldDef;
$schemaMetaFieldDef ??= Introspection::schemaMetaFieldDef();
$typeMetaFieldDef ??= Introspection::typeMetaFieldDef();
$typeNameMetaFieldDef ??= Introspection::typeNameMetaFieldDef();

$queryType = $schema->getQueryType();

if ($fieldName === $schemaMetaFieldDef->name && $queryType === $parentType) {
return $schemaMetaFieldDef;
$schemaMeta = $schema->introspection->schemaMetaFieldDef();
if ($fieldName === $schemaMeta->name && $queryType === $parentType) {
return $schemaMeta;
}

if ($fieldName === $typeMetaFieldDef->name && $queryType === $parentType) {
return $typeMetaFieldDef;
$typeMeta = $schema->introspection->typeMetaFieldDef();
if ($fieldName === $typeMeta->name && $queryType === $parentType) {
return $typeMeta;
}

if ($fieldName === $typeNameMetaFieldDef->name) {
return $typeNameMetaFieldDef;
$typeNameMeta = $schema->introspection->typeNameMetaFieldDef();
if ($fieldName === $typeNameMeta->name) {
return $typeNameMeta;
}

return $parentType->findField($fieldName);
Expand Down
90 changes: 8 additions & 82 deletions src/Type/Definition/Directive.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

use GraphQL\Error\InvariantViolation;
use GraphQL\Language\AST\DirectiveDefinitionNode;
use GraphQL\Language\DirectiveLocation;
use GraphQL\Type\Registry\DefaultStandardTypeRegistry;

/**
* @phpstan-import-type ArgumentListConfig from Argument
Expand All @@ -28,12 +28,11 @@ class Directive
public const DEPRECATED_NAME = 'deprecated';
public const REASON_ARGUMENT_NAME = 'reason';

/**
* Lazily initialized.
*
* @var array<string, Directive>
*/
protected static array $internalDirectives;
public const INTERNAL_DIRECTIVE_NAMES = [
self::INCLUDE_NAME,
self::SKIP_NAME,
self::DEPRECATED_NAME,
];

public string $name;

Expand Down Expand Up @@ -75,91 +74,18 @@ public function __construct(array $config)
$this->config = $config;
}

/** @throws InvariantViolation */
public static function includeDirective(): Directive
{
$internal = self::getInternalDirectives();

return $internal['include'];
}

/**
* @throws InvariantViolation
*
* @return array<string, Directive>
*/
public static function getInternalDirectives(): array
{
return self::$internalDirectives ??= [
'include' => new self([
'name' => self::INCLUDE_NAME,
'description' => 'Directs the executor to include this field or fragment only when the `if` argument is true.',
'locations' => [
DirectiveLocation::FIELD,
DirectiveLocation::FRAGMENT_SPREAD,
DirectiveLocation::INLINE_FRAGMENT,
],
'args' => [
self::IF_ARGUMENT_NAME => [
'type' => Type::nonNull(Type::boolean()),
'description' => 'Included when true.',
],
],
]),
'skip' => new self([
'name' => self::SKIP_NAME,
'description' => 'Directs the executor to skip this field or fragment when the `if` argument is true.',
'locations' => [
DirectiveLocation::FIELD,
DirectiveLocation::FRAGMENT_SPREAD,
DirectiveLocation::INLINE_FRAGMENT,
],
'args' => [
self::IF_ARGUMENT_NAME => [
'type' => Type::nonNull(Type::boolean()),
'description' => 'Skipped when true.',
],
],
]),
'deprecated' => new self([
'name' => self::DEPRECATED_NAME,
'description' => 'Marks an element of a GraphQL schema as no longer supported.',
'locations' => [
DirectiveLocation::FIELD_DEFINITION,
DirectiveLocation::ENUM_VALUE,
DirectiveLocation::ARGUMENT_DEFINITION,
DirectiveLocation::INPUT_FIELD_DEFINITION,
],
'args' => [
self::REASON_ARGUMENT_NAME => [
'type' => Type::string(),
'description' => 'Explains why this element was deprecated, usually also including a suggestion for how to access supported similar data. Formatted using the Markdown syntax, as specified by [CommonMark](https://commonmark.org/).',
'defaultValue' => self::DEFAULT_DEPRECATION_REASON,
],
],
]),
];
}

/** @throws InvariantViolation */
public static function skipDirective(): Directive
{
$internal = self::getInternalDirectives();

return $internal['skip'];
}

/** @throws InvariantViolation */
public static function deprecatedDirective(): Directive
{
$internal = self::getInternalDirectives();

return $internal['deprecated'];
return DefaultStandardTypeRegistry::instance()->internalDirectives();
}

/** @throws InvariantViolation */
public static function isSpecifiedDirective(Directive $directive): bool
{
return \array_key_exists($directive->name, self::getInternalDirectives());
return \in_array($directive->name, self::INTERNAL_DIRECTIVE_NAMES, true);
}
}
Loading