putyourlightson / craft-blitz-recommendations

Provides templating performance recommendations for Craft CMS.
MIT License
18 stars 1 forks source link

Falsely reporting globals? #2

Closed MattWilcox closed 4 years ago

MattWilcox commented 4 years ago

Unsure if this is a genuine issue or something I just aren't understanding.

Blitz Recommendations is flagging a field which is loaded from a Global as not being Eager loaded, when... pretty sure it is.

Use the with parameter to eager-load sub-elements of the Navigation - Footer field.
{% set entries = craft.entries.with(['navigationFooter']).all() %}

But here's the code pertaining to that field in the template:

{% cache globally %}
    {% set
        navItems = navigation.navigationFooter.with([
            'internalPage:page',
            'file:file'
        ]).all()
    %}

    {% for navItem in navItems %}
        {% if loop.first %}
            <div class="footerNav">
                <h3>Footer Navigation</h3>
                <ul>
        {% endif %}

        {% if navItem.type == "internalPage" and navItem.page | length %}
            <li
                {% if entry is defined and entry.slug == navItem.page[0].slug %}
                    class="current"
                {% endif %}
            >
                <a href="{{ navItem.page[0].url }}">
                    {% if navItem.linkTitle | length %}
                        {{ navItem.linkTitle }}
                    {% else %}
                        {{ navItem.page[0].title }}
                    {% endif %}
                </a>
            </li>
        {% endif %}

        {% if navItem.type == "externalPage" %}
            <li><a href="{{ navItem.linkUrl }}">{{ navItem.linkTitle }}</a></li>
        {% endif %}

        {% if navItem.type == "file" %}
            <li>
                <a href="{{ navItem.file[0].url }}">
                    {% if navItem.label | length %}
                        {{ navItem.label }}
                    {% else %}
                        {{ navItem.file[0].title }}
                    {% endif %}
                </a>
            </li>
        {% endif %}

        {% if loop.last %}
                </ul>
            </div><!-- .footerNav -->
        {% endif %}
    {% endfor %}
{% endcache %}
bencroker commented 4 years ago

What exactly are navigation and navigationFooter in your code?

    {% set
        navItems = navigation.navigationFooter.with([
            'internalPage:page',
            'file:file'
        ]).all()
    %}
MattWilcox commented 4 years ago

navigation is a Global group, and navigationFooter is a Matrix field.

bencroker commented 4 years ago

Ok so it seems that the element query used to detect eager-loading opportunities is being executed here as well. If you want to eliminate the warning then you will probably need to eager-load the global set first.

{% set navigation = craft.globalSets().handle('navigation').with([
    'navigationFooter.internalPage:page',
    'navigationFooter.file:file',
]).one() %}
MattWilcox commented 4 years ago

Ok, so use that rather than my existing Eager Loading of navItems... I'll try and wrap my head around that, cheers.

bencroker commented 4 years ago

I'm not saying it's the right way (or even recommended way), but it should eliminate that recommendation if that's what you're after :)