Wrap calls to search results and collection counts in paginate to reduce additional requests #2421
Conversation
ludoboludo
requested review from
ludoboludo
and removed request for
tyleralsbury
| {%- if search.filters != empty -%} | ||
| {%- if section.settings.enable_filtering or section.settings.enable_sorting -%} | ||
| <aside aria-labelledby="verticalTitle" class="facets-wrapper{% unless section.settings.enable_filtering %} facets-wrapper--no-filters{% endunless %}{% if section.settings.filter_type != 'vertical' %} page-width{% endif %}" id="main-search-filters" data-id="{{ section.id }}"> | ||
| {% render 'facets', results: search, enable_filtering: section.settings.enable_filtering, enable_sorting: section.settings.enable_sorting, filter_type: section.settings.filter_type, paginate: paginate %} |
There was a problem hiding this comment.
Is it needed here to pass in the paginate object here 🤔 I don't see it referenced in the snippet.
If we an attribute to the render we want to add it to the snippet itself in the description at the top of the file:
Screenshot
There was a problem hiding this comment.
result.results_count benefits from having a paginate context within the facets render to reduce unnecessary data retrieval. Even though the paginate block wraps where the snippet is rendered in main-search.liquid and main-collection-product-grid.liquid, it is not in the context inside of the rendered liquid partial by default.
https://shopify.dev/docs/api/liquid/tags/render
Inside snippets and app blocks, you can't directly access variables that are created outside of the snippet or app block. However, you can specify variables as parameters to pass outside variables to snippets.
…uce additional requests
zacharyreynochiasson
force-pushed
the
optimize_search_paginate
branch
from
b973bb2
to
28394b6
Compare
zacharyreynochiasson
changed the title
* shopify/main: Gift cards/add recipient (Shopify#2412) Update 1 translation file (Shopify#2453) [Slideshow] Add ambient movement to background (Shopify#2383) Wrap calls to search results ind collection counts in paginate to reduce additional requests (Shopify#2421) [Header] Add app block in the header section (Shopify#2238) [Image with Text] Add ambient movement to image (Shopify#2385) Update 1 translation file (Shopify#2450) [Blog post section] Fix slides size on mobile (Shopify#2368) Duplicated scrollbar in drawer menu (Shopify#2400) [Header locales] Support gradients (Shopify#2386) [Image banner] Add ambient movement to background (Shopify#2342)
PR Summary:
Performance optimization by ensuring any calls to search results and collection counts are within the proper pagination context. Also passes the paginate down to the filter render for the same reason.
Note the diff is large cause of indenting but this is a 3 line change for each file (moved the wrapping paginate block and added passing paginate to the
facetsrender)suggest hiding whitespace to view changes:
Why are these changes introduced?
Improves performance of page render
What approach did you take?
Wrap any calls to
search.resultsandsearch.results_countin the paginate block inmain-search.liquid, also passes thepaginateto thefacetsrender.Wrap any calls to
collection.results_count,collection.products_countandcollection.all_products_countin the paginate block inmain-collection-product-grid.liquid, also passes thepaginateto thefacetsrender.Visual impact on existing themes
No visual impact
Testing steps/scenarios
Ensure search & collections render the same
Checklist