Skip to content

Lazy loading for arguments #429

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

Closed
wants to merge 3 commits into from
Closed

Lazy loading for arguments #429

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4dfb2d0
Field arguments lazy loading
mad-ice Jan 7, 2019
0b1f0c2
Missed throws in docblock
mad-ice Jan 7, 2019
88b62c4
Removed .idea from gitignore
mad-ice Jan 7, 2019
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
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ composer.phar
phpcs.xml
phpstan.neon
vendor/
.idea/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi, for this is not project related you should add it to your global gitignore.

Copy link
Author

Choose a reason for hiding this comment

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

Didn't know that existed, thanks!

14 changes: 12 additions & 2 deletions src/Type/Definition/FieldArgument.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use GraphQL\Language\AST\InputValueDefinitionNode;
use GraphQL\Utils\Utils;
use function is_array;
use function is_callable;
use function is_string;
use function sprintf;

Expand Down Expand Up @@ -66,12 +67,21 @@ public function __construct(array $def)
}

/**
* @param mixed[] $config
* @param mixed[]|callable $config
*
* @return FieldArgument[]
* @throws \InvalidArgumentException
*/
public static function createMap(array $config)
public static function createMap($config)
{
if (is_callable($config)) {
Copy link
Contributor

@shmax shmax Oct 16, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry for the long delay, but as promised I tried out your changes. So far I don't quite understand how this changes things; createMap is called by the FieldDefinition constructor, and then you just check to see if $config is callable, and if it is you immediately call it? Where do the savings come in?

I tested this by converting a few of the args on my top-level Query fields to functions, and stuck breakpoints inside them; they get called immediately.

What am I missing, here? Where does the "lazy" happen?

Copy link
Contributor

Choose a reason for hiding this comment

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

I just POC'ed my own full lazy loading concept; so far seems to work pretty well. I'll try to put together a PR by the end of the week.

$config = $config();
}

if (! is_array($config)) {
throw new \InvalidArgumentException('$config must be an array or a callable which returns such an array.');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
throw new \InvalidArgumentException('$config must be an array or a callable which returns such an array.');
throw new \InvalidArgumentException('"$config" must be an array or a callable which returns such an array.');

}

$map = [];
foreach ($config as $name => $argConfig) {
if (! is_array($argConfig)) {
Expand Down
4 changes: 2 additions & 2 deletions src/Type/Definition/FieldDefinition.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,9 @@ public static function defineFieldMap(Type $type, $fields)

$field['name'] = $name;
}
if (isset($field['args']) && ! is_array($field['args'])) {
if (isset($field['args']) && ! is_array($field['args']) && ! is_callable($field['args'])) {
throw new InvariantViolation(
sprintf('%s.%s args must be an array.', $type->name, $name)
sprintf('%s.%s args must be an array or a callable which returns such an array.', $type->name, $name)
);
}
$fieldDef = self::create($field);
Expand Down