-
-
Notifications
You must be signed in to change notification settings - Fork 569
Lazy loading of types #557
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
Merged
Changes from all commits
Commits
Show all changes
61 commits
Select commit
Hold shift + click to select a range
f45ac75
lazy load types
shmax 6ae8227
fix introspection resolve
shmax eeec7a7
tests, linting, stanning
shmax 84d6514
fix annotation
shmax 8aa3770
remove annotation, debug stuff
shmax 9f11df8
cleanup
shmax 5d98f7e
fix annotations
shmax 5acedf8
start on stanning
shmax 889083c
rename to Schema::resolveType
shmax a540519
remove some more useless resolve calls
shmax 2609b27
remove another unneeded resolve call
shmax b704d54
remove unused
shmax 9a2486a
workaround for Scrutinizer
shmax c85a7d5
ws
shmax f38d9b3
make assertType 0 cost
shmax 935cab2
use getType in assertValid
shmax 5e1bb4a
add lazy load to blog sample
shmax d2dedf7
fix scrutinizer
shmax 9c354e3
use InvariantViolation
shmax 1646126
resolve abstract type
shmax cf2b0da
remove whoopsie
shmax 6fde27a
remove annotation
shmax 3b88752
lint
shmax 92bf81c
linting and stanning
shmax 922991a
fix completeAbstractValue
shmax 32fadac
fix tests
shmax 35c2fef
lint
shmax 8316564
cast
shmax 0be4525
fix stan issues
shmax 4e04cad
use trait
shmax 81a924a
remove assert stuff
shmax 2342766
lock sniffer
shmax 6af0e9b
stanning
shmax fdcce6e
remove unused
shmax f89c842
remove unused
shmax 730e117
remove unused
shmax 48106ac
remove assert
shmax e1b363e
remove cast
shmax 1e48d6d
seal class
shmax 51caeef
add typehint
shmax c085169
rename, change visibility
shmax 9f2cf34
add typehint
shmax a8b6ce3
Merge branch 'master' into lazy-load-type
shmax 4e6e442
Merge branch 'master' into lazy-load-type
shmax f6839dd
stanning
shmax a8f870d
expand type
shmax e73353c
expand type
shmax d214aa5
remove unused
shmax 07b9297
reorganize
shmax 88db0b3
simplify
shmax b02d8ce
use isset
shmax cfc5dff
touch
shmax ae79c77
revert
shmax cfb3d3a
disable specific rule
shmax 7aba47f
cast type
shmax 3551138
broaden type
shmax 72ccaa3
annotate return type
shmax 375fd91
remove redundant code
shmax 6a1fad4
skip method call
shmax 484d2dd
move execution of lazy type to typeloader
shmax b2a50d6
remove pointless test
shmax File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ | |
| use GraphQL\Error\Error; | ||
| use GraphQL\Error\InvariantViolation; | ||
| use GraphQL\Language\AST\FieldDefinitionNode; | ||
| use GraphQL\Type\Schema; | ||
| use GraphQL\Utils\Utils; | ||
| use function is_array; | ||
| use function is_callable; | ||
|
|
@@ -58,7 +59,7 @@ class FieldDefinition | |
| */ | ||
| public $config; | ||
|
|
||
| /** @var OutputType&Type */ | ||
| /** @var callable|(OutputType&Type) */ | ||
| public $type; | ||
|
|
||
| /** @var callable|string */ | ||
|
|
@@ -174,12 +175,9 @@ public function getArg($name) | |
| return null; | ||
| } | ||
|
|
||
| /** | ||
| * @return OutputType&Type | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should be kept in order not to lower type coverage
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As discussed previously, nothing I can do here without asserts. |
||
| */ | ||
| public function getType() : Type | ||
| { | ||
| return $this->type; | ||
| return Schema::resolveType($this->type); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -217,7 +215,7 @@ public function assertValid(Type $parentType) | |
| ) | ||
| ); | ||
|
|
||
| $type = $this->type; | ||
| $type = $this->getType(); | ||
| if ($type instanceof WrappingType) { | ||
| $type = $type->getWrappedType(true); | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should stay, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like that, but without some kind of assert my only other option is a cast, which sort of defeats the point of checking.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so can we assert / ignore in static analysis for now? This can be IMO resolved better through templates later. This changes would unnecessarily lower type coverage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My next PR will be a full native assert overhaul across the board. You can consider that the price of merging this PR.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you need to remove this, though? As a user of this library + PHPStan, i would appreciate the precise type, even if there is a slim chance that it might be wrong (not sure how that might even happen).
Adding an
assert()later seems like a great idea, too - i think we can have both.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because if I restore it, it conflicts with the known return type of
Schema::resolveType, which isType. I believe the best way to make everybody happy is going to be with native asserts, but setting those up properly will involve more work and more discussion, and I really think it warrants a dedicated PR.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TBH the type doc can be IMO kept as is and the type incompatibility within this method ignored until the next PR