Skip to content

Make root query operation type optional - #920

Draft
spawnia wants to merge 1 commit into
masterfrom
optional-root-query-operation-type
Draft

Make root query operation type optional#920
spawnia wants to merge 1 commit into
masterfrom
optional-root-query-operation-type

Conversation

@spawnia

@spawnia spawnia commented Aug 23, 2021

Copy link
Copy Markdown
Collaborator

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Updates the library to support GraphQL schemas that omit the root query operation type (enabling mutation-only / subscription-only schemas), aligning behavior with the referenced GraphQL spec discussion.

Changes:

  • Relax schema validation to no longer require a query root type.
  • Update tests to reflect mutation-only schemas and (attempted) introspection behavior without a query root.
  • Minor internal refactors/visibility tweaks in type definitions and object type construction.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/Type/ValidationTest.php Updates validation expectations for schemas without a query root type.
tests/Type/IntrospectionTest.php Adjusts introspection test setup/expected results for a schema without a query root.
tests/Type/DefinitionTest.php Updates schema construction to cover mutation-only schemas and renames the test accordingly.
src/Type/SchemaValidationContext.php Removes validation error requiring a query root type.
src/Type/Schema.php Removes an outdated TODO about assuming a query root type always exists.
src/Type/Definition/TypeWithFields.php Widens visibility of field storage/initialization for subclasses.
src/Type/Definition/ObjectType.php Refactors name inference/validation and tweaks doc wording.
src/Executor/ReferenceExecutor.php Removes special-casing for __schema/__type meta fields (currently breaking introspection).
Comments suppressed due to low confidence (2)

src/Executor/ReferenceExecutor.php:615

  • getFieldDef() no longer special-cases the introspection meta fields __schema and __type. Since these fields are not actually defined on the query root type, removing this logic breaks standard introspection queries (they will fail field lookup). If the query root type can be absent, the special-casing should remain but be guarded when $schema->getQueryType() is null.
        if ($fieldName === $typeNameMetaFieldDef->name) {
            return $typeNameMetaFieldDef;
        }

        return $parentType->findField($fieldName);

tests/Type/IntrospectionTest.php:45

  • The expected introspection result now asserts queryType => null, but Introspection::_schema() currently defines queryType as a NonNull field (new NonNull(self::_type())) and resolves it from $schema->getQueryType(). With a schema that has no query type, execution should error due to returning null for a non-null field. If the intent is to keep introspection executable here, this expectation should be reverted to QueryRoot (or removed) until the introspection schema is updated accordingly.
                            'queryType'        => null,

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

'fields' => ['a' => Type::string()],
]),
]);
$emptySchema = new Schema([]);
Comment on lines 300 to 304
/**
* @see it('defines a mutation schema')
*/
public function testDefinesAMutationSchema(): void
public function testDefinesAMutationOnlySchema(): void
{
Comment on lines 316 to 319
$this->assertMatchesValidationMessage(
$schema->validate(),
[['message' => 'Query root type must be provided.']]
[]
);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants