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

Allow plugins to implement custom eager loading #4220

Closed
wants to merge 1 commit into from
Closed

Allow plugins to implement custom eager loading #4220

wants to merge 1 commit into from

Conversation

sebastian-lenz
Copy link
Contributor

@sebastian-lenz sebastian-lenz commented May 6, 2019

Eager loading is a great feature of Craft, basically the user marks relations / fields he is going to access on queries and the CMS can preload those fields in oder to speed up things. However, the current implementation is very strict on how this works, basically a field can only eager load one element type, the field will be completely replaced by the eager loaded elements and the field must supply the element ids in a certain fashion; all this magic happens in one big method.

Suggestion
I guess it would be quite difficult to extend the current technique so it fits multiple purposes, so I would like to suggest to open up a way for fields to take care of eager loading stuff on their behalf and bypass the default behavior.

Simple approach
Currently fields can implement the interface EagerLoadingFieldInterface whose method getEagerLoadingMap can either return an array if elements should be eager loaded or false if for some reason the field is sure all values will be empty. No matter what it returns, the core will set the eager loaded property here on all elements and we have no way to prevent this. As a simple solution, allow getEagerLoadingMap to return something (like null or true) to signal that it will take care of eager loading by itself.

Usage example
I'm the author a a link field plugin and I've noticed the field can produce an immense amount of additional database hits if a lot of element links must be fetched (e.g. when building a navigation that contains redirect pages). Unfortunately I cannot use the default eager loading path as a) the field can contain different element links (e.g. links to assets and elements) and b) would replace the link model with the eager loaded elements. I've highjacked the with property (by looking for my fields handle in Field::modifyElementsQuery and filtering it out) and was able to greatly improve the performance, but the current implementation feels more than wacky.

brandonkelly added a commit that referenced this pull request May 8, 2019
@brandonkelly brandonkelly added this to the 3.2 milestone May 8, 2019
@brandonkelly
Copy link
Member

Just implemented this for 3.2.

@sebastian-lenz sebastian-lenz deleted the patch-1 branch May 11, 2019 14:04
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