Skip to content
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

Prefetch refactor #588

Merged
merged 17 commits into from
Aug 3, 2023
Merged

Conversation

oprypkhantc
Copy link
Contributor

@oprypkhantc oprypkhantc commented Mar 28, 2023

This PR targets two things:

  • cleanup of #[Field], QueryFieldDescriptor and QueryField itself, separating prefetch functionality from them
  • allow userland implementation of prefetch-like features, i.e. deferring the resolution of a field the same way prefetch does

This includes changes from #584, so that would need to be merged first; then this can be reviewed properly.

On the changes:

  • prefetchMethod parameter of #[Field] now trigger a deprecation error
  • new #[Prefetch] attribute supports callables, both through the container and through static methods: #[Prefetch([SomeService::class, 'method'])], #[Prefetch([SomeHelper::class, 'staticMethod'])], #[Prefetch('staticMethodInSourceClass')]
  • non static calls on the source object are no longer supported: #[Prefetch('methodOnSource')]. There's an exception for the deprecated #[Field(prefetchMethod: 'method')] - it continues to work
  • as a side effect of moving it to parameter level, multiple #[Prefetch] annotations per field are now allowed; order of input parameters in the scheme is determined by order of parameters on the field itself (see TestTypeWithPrefetchMethods)

@oprypkhantc oprypkhantc marked this pull request as draft March 28, 2023 21:37
@oojacoboo
Copy link
Collaborator

@oprypkhantc thanks for this. I'll get #584 reviewed and hopefully merged soon!

# Conflicts:
#	src/FieldsBuilder.php
#	src/Parameters/PrefetchDataParameter.php
#	src/QueryField.php
#	src/QueryFieldDescriptor.php
#	website/docs/field-middlewares.md
@oprypkhantc
Copy link
Contributor Author

oprypkhantc commented Aug 2, 2023

Hey @oojacoboo, this is ready for review.

@oprypkhantc oprypkhantc marked this pull request as ready for review August 2, 2023 18:05
@oojacoboo oojacoboo merged commit d093249 into thecodingmachine:master Aug 3, 2023
@oojacoboo
Copy link
Collaborator

Thanks @oprypkhantc, looks good - merged

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