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

[5.x]: Eager loading nested Matrix fields with similar names not working as expected #15796

Closed
rob-c-baker opened this issue Sep 26, 2024 · 4 comments
Labels

Comments

@rob-c-baker
Copy link

rob-c-baker commented Sep 26, 2024

What happened?

First noticed this when I crated this: craftcms/docs#661 (comment)

Description

With this eager loading map:

{% set eager_load_map = [
	[ 'navigation', { with: [
                [ 'topLevel' ],
		[ 'topLevel.detail:entry', { limit: 1 }],
		[ 'elements' ],
		[ 'elements.heading3:entry', { limit: 1 }],
		[ 'elements.link:entry', { limit: 1 }],
	]}]
] %}
{# example follow on call #}
[entryQuery].with(eager_load_map).all()

navigation is an Matrix field in an Entry type on a section dedicated to being the site's main navigation
navigation.topLevel is a Matrix field holding a single block within navigation
navigation.elements is a Matrix field holding many blocks within navigation
The entry within the topLevel & elements Matrix blocks is an Entry Field that in each appearance holds a single entry representing the target of the navigation item.

Note that there are (at least) 2 different Entry types attached to the elements Matrix block but they all have an entry Entry Field.

The issue is that depending how you adjust the map (these bits: navigation.elements:link.entry & navigation.elements:heading3.entry) only 1 of them is ever actually eager loaded. I have tried many different iterations of how to lay this map out but they never all get eager loaded.

My guess here is that navigation.elements:link.entry & navigation.elements:heading3.entry are internally being treated as the same thing, effectively resolving to navigation.elements.entry but only ever applying to one the entry types on the Matrix block - it looks like the order they are defined in the map make no difference.

For reference, the same map, doing a working eager load on Craft 4 is this:

{% set eager_load_map = [
	[ 'navigation' ],
	[ 'navigation.topLevel' ],
	[ 'navigation.topLevel.entry', { limit: 1 } ],
	[ 'navigation.elements' ],
	[ 'navigation.elements.heading:entry', { limit: 1 } ],
	[ 'navigation.elements.link:entry', { limit: 1 } ]
] %}

Trying this on v5 doesn't work in the same way as the first snippet doesn't!

Steps to reproduce

  1. Matrix on an entry type in a section called navigation containing:
  2. Another Matrix with a single block topLevel containing an entry field called entry
  3. Another Matrix with multiple blocks 2 or more of which should contain an entry field called entry.
  4. Attempt to eager load navigation, topLevel.entry and navigation.[eitherEntryType].entry

Expected behavior

All the things gets eager loaded.

Actual behavior

Some of the things get eager loaded.

Craft CMS version

5.4.4

PHP version

8.3.7

@rob-c-baker rob-c-baker changed the title [5.x]: [5.x]: Eager loading nested Matrix fields with similar names not working as expected Sep 26, 2024
@brandonkelly
Copy link
Member

See my comment here: craftcms/docs#661 (comment)

If you’re still having issues, let me know and I can reopen.

@rob-c-baker
Copy link
Author

Thanks for looking at this @brandonkelly. Adding in all the EntryType handles to the v5 example above, gives:

{% set eager_load_map = [
	[ 'navigation' ],
	[ 'navigation.navigationBlock:topLevel' ],
	[ 'navigation.navigationBlock:topLevel.detail:entry', { limit: 1} ],
	[ 'navigation.navigationBlock:elements' ],
	[ 'navigation.navigationBlock:elements.heading3:entry', { limit: 1 } ],
	[ 'navigation.navigationBlock:elements.link:entry', { limit: 1 } ],
] %}

But the entry in the line [ 'navigation.navigationBlock:elements.heading3:entry', { limit: 1 } ] does not load. Dumping out what should contain the entry out reveals an empty ElementCollection. This is coupled with the debug toolbar showing zero eager loading opportunities. Removing the limit criteria does not change things.

If I specifically remove this part of the eager loading map, the code that renders the nav realises this and loads the entry via an N+1 query (as a fallback). This works - the entry loads, and the nav renders, but the debug toolbar then shows 79 eager loading opportunities, goes from 80 queries to 350 and spends well over a second in the database.

I'm considering re-structuring the data as a separate release / migration in v4 prior to coming back to the upgrade but I would rather not if possible. It maybe a case of juggling content manager's ease of use vs the technicalities of implementation.

@brandonkelly
Copy link
Member

Thanks for the additional details! I was able to reproduce this when each entry relation field had the same handle, but was based on a different custom field (as you’d end up with after upgrading to Craft 5, with a Matrix field that had multiple entry relation fields within different block types).

Fixed now for the next release.

@brandonkelly
Copy link
Member

Craft 5.4.7 is out with that fix. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants