Skip to content

Update generated types #780

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

Merged

Conversation

murtukov
Copy link
Member

@murtukov murtukov commented Nov 8, 2020

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Documented? no
Fixed tickets webonyx/graphql-php#425, webonyx/graphql-php#557

As described here webonyx/graphql-php#425, instantiating schemas with many types can hit performance. It was suggested (and implemented) to wrap field type definitions into a closure for lazy loading.

This PR updates the TypeBuilder class to generate proper field types. It wraps only user-defined types into a closure.

Example:

$config = [
    'fields' => fn() => [
        'street' => Type::nonNull(Type::string()),
        'city' => Type::nonNull(Type::string()),
        'zipCode' => Type::nonNull(Type::int()),
        'period' => fn() => Type::nonNull($globalVariables->get('typeResolver')->resolve('Period')),
    ],
];

Also some unused methods were removed from TypeResolver, as TypeGenerator never generates references like this:

$globalVariables->get('typeResolver')->resolve('[Address]');

but generates this instead:

Type::listOf($globalVariables->get('typeResolver')->resolve('Address'));

- Change TypeBuilder to wrap field type into a closures if it's a reference to a user-defined type
- Remove unused TypeResolver methods
- Update PHPDocs
@murtukov murtukov requested a review from mcg-web November 8, 2020 02:20
@mcg-web mcg-web merged commit d7c2e25 into overblog:master Nov 9, 2020
@murtukov murtukov deleted the enhancement/type-fields-callbacks branch November 9, 2020 14:41
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

Successfully merging this pull request may close these issues.

2 participants