fix(core): read past lazy-loading placeholders in Layer C - #16
Merged
Conversation
A learned profile records `src` for a product image, because that is the attribute the element the user clicked appeared to hold. On a lazy-loading theme `src` holds a 1x1 `data:` placeholder and the real URL sits in a data attribute, so `readRule()` returned the same grey pixel — or nothing — for every row in the export. Layer B already handled this, in a private `imageUrl()` inside microdata.ts. Move it to extract/html.ts as an exported `readImageUrl()` and have both layers call it. A theme does not lazy-load differently depending on whether the page also published JSON-LD, so any layer reading `src` on its own would be the one returning placeholders. Resolving this at apply time rather than at learn time repairs profiles that were already taught and saved, which is most of them. When no real URL exists anywhere on the element the field stays empty: an empty cell is honest, a grey pixel imported into a shop is not. Also fold the lazy attribute names into one place. inspect/images.ts had its own copy — it wants every URL an element references rather than the first usable one, so the logic genuinely differs, but the attribute names do not. It now imports LAZY_IMAGE_ATTRS and SRCSET_ATTRS from html.ts, which also gains it `data-lazy` and gains readImageUrl a `data-lazy-srcset` fallback. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
A learned profile (Layer C) records
srcfor a product image, because that is the attribute the element the user clicked appeared to hold. On a lazy-loading theme,srcholds a 1x1data:placeholder and the real URL sits in a data attribute.readRule()inpackages/core/src/learn/apply.tsreadfound.attr(attribute)raw, so every product row in the export got the same grey pixel — or an empty cell.Layer B (microdata) already handled this correctly, via a private
imageUrl()helper insidemicrodata.ts. Layer C did not, because the helper was not shared.The fix
packages/core/src/extract/html.tsas an exportedreadImageUrl(node).microdata.tscalls it instead of its private copy — no behaviour change there.apply.tscalls it when the profile's recorded attribute issrc.Resolving this at apply time rather than at learn time repairs profiles that were already taught and saved, which is most of them.
When no real URL exists anywhere on the element, the field stays empty. An empty cell is honest; a grey pixel imported into a shop is not. That path is covered by a test.
A theme does not lazy-load differently depending on whether the page also published JSON-LD, so any layer reading
srcon its own would be the one returning placeholders. Routing every<img>read through one function is what keeps that from recurring.Attribute names now live in one place
packages/core/src/inspect/images.ts(phase 13) kept its own copy of the lazy attribute names. Its job is genuinely different — it wants every URL an element references, not the first usable one — but the attribute names do not depend on who is asking. It now importsLAZY_IMAGE_ATTRSandSRCSET_ATTRSfromhtml.ts.Two small additive consequences, both improvements:
images.tsgainsdata-lazy, which its local list was missing.readImageUrlgains adata-lazy-srcsetfallback, which only fires when nothing else matched.Tests
Two new cases in
packages/core/test/learn/learn.test.tscover the lazy fallbacks (data-src,data-lazy-src,srcset) and the placeholder-only case.npm run checkis green: format, lint, typecheck, and 802 tests (800 before, plus the 2 new).🤖 Generated with Claude Code