Skip to content

fix: dedupe preload links and stop double-processing category collections#11

Merged
SamJUK merged 3 commits into
masterfrom
fix/duplicate-preload-links
Jul 16, 2026
Merged

fix: dedupe preload links and stop double-processing category collections#11
SamJUK merged 3 commits into
masterfrom
fix/duplicate-preload-links

Conversation

@SamJUK

@SamJUK SamJUK commented Jul 15, 2026

Copy link
Copy Markdown
Owner

Summary

QA on category pages found <link rel="preload"> tags doubled (8 tags for 4 images) on
every category listing. Root cause: Magento\Catalog\Block\Product\ProductList\Toolbar::setCollection()
fires twice per render (confirmed via instrumentation — same toolbar/collection instance
both times, a pre-existing Magento/Luma template quirk, not something this module introduced),
so SetCollection::afterSetCollection ran its preload work twice.

Two fixes:

  • SetCollection: tracks processed collections by spl_object_id(), skips a collection
    it's already handled. Holds a reference to the collection object itself (not just the id)
    so the id can't be recycled by an unrelated object later in the same request — spl_object_id()
    values are only unique while the object is alive.
  • LinkStore: keys its internal store by md5(serialize($link->getAttrs())) instead of
    appending unconditionally, so any other duplicate-invocation case (present or future, in any
    observer/plugin) collapses to one entry as a backstop — O(1) per add(), not a scan.

Test plan

  • Added testAddingLinkWithIdenticalAttrsIsDeduped / testAddingLinkWithDifferentAttrsIsNotDeduped
    to LinkStoreTest.
  • Added testDoesNotReprocessTheSameCollectionInstanceTwice /
    testProcessesDifferentCollectionInstancesIndependently to SetCollectionTest.
  • Full unit suite passes (51/51) against Magento 2.4.8-p3/PHP 8.3.
  • Live-verified against a real category page (148 products, ?product_list_order=price&product_list_dir=desc):
    exactly 4 <link rel="preload" as="image"> tags now (was 8), sort-order fix (SAM-6) still holds,
    no-gallery-image fix (SAM-44) still holds, product-page preload still emits exactly 1 link.

…ions

Toolbar::setCollection() fires twice per category page render (a
pre-existing Magento/Luma quirk, same toolbar/collection instance both
times), so SetCollection::afterSetCollection ran the same preload work
twice, doubling every category-grid <link rel="preload"> tag.

Two fixes:
- SetCollection now tracks processed collections by spl_object_id() and
  skips a collection it's already handled. Holds a reference to the
  collection itself (not just the id) so the id can't be recycled by
  an unrelated object for the rest of the request.
- LinkStore now keys its internal store by a hash of each link's
  getAttrs(), so any other duplicate-invocation case (present or
  future, in any observer/plugin) collapses to one entry as a backstop.
@SamJUK
SamJUK force-pushed the fix/duplicate-preload-links branch from 5430e60 to 996d42d Compare July 15, 2026 23:14
SamJUK added 2 commits July 16, 2026 08:58
phpcs (Magento2 standard) forbids md5() and serialize() outright.
Attrs is a flat scalar array, so json_encode() + sha256 is a drop-in
replacement with no new dependency needed.
@SamJUK
SamJUK force-pushed the fix/duplicate-preload-links branch from c90f9a3 to 03bec23 Compare July 16, 2026 07:58
@SamJUK
SamJUK merged commit b76264e into master Jul 16, 2026
12 checks passed
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.

1 participant