fix replacing collections in return types #751
Merged
Conversation
This file contains 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
szepeviktor
approved these changes
Jan 5, 2021
|
Please merge master to get #775 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
resolves #747
Changes
This PR restricts the methods that
ModelExtensionwill change the return type. Instead of replacing any return type containing a collection, it will now only replace the return type of the methods that can return a custom collection.Additionally,
GenericEloquentCollectionTypeNodeResolverExtensionwould inadvertently resolveCollection|static[]to a class with the namestatic. I have updated this to resolve the inner array type, such that static is now correctly interpreted as aStaticType.This is the first time I've looked into the internals, please let me know if this is not the correct approach (apologies if so).
The test failure is unrelated and is currently affecting master, the tests passed on my fork with the unrelated failure ignored.
Breaking changes
I believe this was the intended behaviour.