MWPW-206311: address #6643 review feedback - #6662
Merged
mokimo merged 2 commits intoSep 7, 2026
Merged
Conversation
- add missing crossorigin to the placeholders.json preload link so the browser reuses it instead of double-fetching (overmyheadandbody) - decouple preloadLcpCodeFiles() from ?mep=off - it preloads ordinary block/icon/placeholder resources, not MEP-specific ones, so only its own kill switch should gate it (mokimo, zagi25) - anchor isCommerceBlock's merch alternative so a future block name containing "merch" as a substring isn't misclassified (mokimo) - drop the unused catch binding to match the file's own convention (overmyheadandbody) - note that the auto-block detector is intentionally approximate vs decorateAutoBlock (mokimo)
Contributor
- no comments in libs/utils/utils.js, per repo convention - revert the preloadLcpCodeFiles()/mepParam==='off' ordering: when MEP is off there's no MEP/Target await to hide the preload behind, and the normal block-loading path already loads everything fast enough on its own, so this only needs to run when MEP is present
This was referenced Sep 7, 2026
milo-pr-merge Bot
pushed a commit
that referenced
this pull request
Sep 9, 2026
* MWPW-206183: preload LCP-section blocks before MEP Warm the first (LCP) section's assets — authored blocks, link-autoblocks, placeholders, and icons.js — in loadArea immediately before the MEP await in checkForPageMods() holds up section processing, so their fetches are already in flight when processing resumes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * MWPW-206311: preload first-section LCP content before MEP await Warm the first section's block code, non-merch/mas autoblocks, placeholder sheets and icons during checkForPageMods() instead of after it, so the LCP element can paint sooner. fetchpriority=high; commerce (merch/mas) autoblocks excluded (heavy dep chain hurts non-store heroes). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * MWPW-206311: add disable-mep-perf-optimization metadata kill switch Emergency opt-out: <meta name="disable-mep-perf-optimization" content="on"> skips the pre-MEP preload (page-level or site-wide via the global metadata sheet). Default = enabled. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * MWPW-206311: drop redundant fetchpriority; call preload from checkForPageMods high == browser default for these preload types, so the explicit fetchpriority plumbing is removed (preloadBlockResources reverts to its original signature). The preload now fires inside checkForPageMods right after the mep=off early return, co-locating it with MEP logic while still running before the MEP await. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * MWPW-206311: unit test preloadLcpCodeFiles (kill switch, merch/mas exclusion) Export preloadLcpCodeFiles and add focused tests: kill-switch metadata, authored-block preload, non-commerce autoblock warming with merch/mas excluded, media_*.mp4 video guard (anchor + img alt), placeholder and icon gating, and the no-first-section early return. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * MWPW-206311: hoist mas-dep/preload chain above checkForPageMods Move getValidatedMasLibsUrl, getMasDepUrl, STATIC_BLOCK_DEPS, blockDeps, preloadBlockResources and preloadLcpCodeFiles above checkForPageMods so the pre-MEP preloadLcpCodeFiles() call no longer needs an eslint-disable for no-use-before-define. Pure reorder, no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * MWPW-206313: warm geo-ip placeholder sheet in preloadLcpCodeFiles (pre-MEP) #6589 hoisted geo-ip sheet warming into loadArea, but it runs after `await checkForPageMods()` — i.e. behind the MEP/Target wait, the exact latency this LCP work removes. Fold the same warmGeoIpSheet(config) call into preloadLcpCodeFiles alongside the placeholder warms so a {{...-geo-ip}} LCP token (or geo-ip-lcp MEP flag) is warmed before the await. Idempotent via geoIpWarm[url] ??=, so loadArea's later call dedupes. Geo-ip helpers hoisted above preloadLcpCodeFiles to keep it in scope. Adds two tests (warms when lingo active + token; no-op otherwise). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * MWPW-206313: fold geo-ip lingo/token gate into warmGeoIpSheet Both callers (preloadLcpCodeFiles and loadArea) repeated the same lingoActive + -geo-ip-token + geo-ip-lcp gate. Move it inside warmGeoIpSheet(config, section, isDoc) so each caller is a single line. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * MWPW-206311: also preload icons.css (rel=preload as=style) pre-MEP loadLink's dedup selector is rel-aware (link[href][rel=stylesheet] for loadStyle vs link[href] for preload), so the preload does not shadow the rel=stylesheet that decorateIcons later appends -- both coexist and the browser consumes the preload (single fetch). Warm the render-affecting sheet in parallel alongside icons.js. Tighten two decorateIcons tests to query the stylesheet specifically now that a preload link with the same href also exists. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * MWPW-206311: exclude authored merch/mas blocks from pre-MEP preload too The /merch|^mas-/ filter was applied only to link-derived autoblocks, so an authored <div class="merch"> in the first section still pulled the heavy commerce dep chain (commerce.js + autoblock.js) before MEP -- the opposite of the change's intent. Apply the same exclusion to authored blocks. Adds a test covering the authored-block path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * MWPW-206311: warm (don't apply) block css pre-MEP; fix useBlockCode nala preloadLcpCodeFiles ran preloadBlockResources before MEP, which applied the DEFAULT block stylesheet via loadStyle. For a block MEP redirects with useBlockCode (e.g. marquee -> libs/mep/sample-block-code/marquee, whose css is `body { color: purple }`), the default blocks/marquee/marquee.css (`.marquee { color: var(--color-white) }`, higher specificity) then overrode the custom styling -> text rendered white, failing the cross-browser mep-actions useBlockCode test. Add a warmStyles option to preloadBlockResources: when set, css is warmed as `rel=preload as=style` instead of applied via loadStyle. preloadLcpCodeFiles now passes it, so the pre-MEP path only warms resources (nothing applied) and decoration applies the correct (MEP-redirected) sheet post-MEP. Unit test asserts the css is preloaded, not stylesheet-applied. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * MWPW-206311: drop explanatory comments from preload code/tests Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * MWPW-206311: address #6643 review feedback (#6662) * fix(perf): address review feedback on MWPW-206311 pre-MEP LCP preload - add missing crossorigin to the placeholders.json preload link so the browser reuses it instead of double-fetching (overmyheadandbody) - decouple preloadLcpCodeFiles() from ?mep=off - it preloads ordinary block/icon/placeholder resources, not MEP-specific ones, so only its own kill switch should gate it (mokimo, zagi25) - anchor isCommerceBlock's merch alternative so a future block name containing "merch" as a substring isn't misclassified (mokimo) - drop the unused catch binding to match the file's own convention (overmyheadandbody) - note that the auto-block detector is intentionally approximate vs decorateAutoBlock (mokimo) * fix(perf): drop added comments; keep mep=off short-circuit as-is - no comments in libs/utils/utils.js, per repo convention - revert the preloadLcpCodeFiles()/mepParam==='off' ordering: when MEP is off there's no MEP/Target await to hide the preload behind, and the normal block-loading path already loads everything fast enough on its own, so this only needs to run when MEP is present * MWPW-206311: drop getValidatedMasLibsUrl JSDoc, no comments in utils (#6663) fix(perf): drop JSDoc on getValidatedMasLibsUrl, no replacement comment zagi25 flagged this JSDoc as inconsistent with the rest of the PR (#6643). It was pre-existing code being relocated, not new authorship, but per repo convention libs/utils/utils.js carries no comments - dropping it outright rather than converting to a plain comment. --------- Co-authored-by: Victor Hargrave <hargrave@adobe.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Okan Sahin <39759830+mokimo@users.noreply.github.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
1 participant
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.
Addresses review feedback from #6643, into that branch:
crossoriginon the placeholders.json preload(
as="fetch"preloads only get reused by a later same-URLfetch()call whencrossoriginmatches - otherwise the browser treats it as a mismatchedresource and double-fetches, which is the "new additional request" the
before/after comparison in MWPW-206311: preload first-section LCP content before MEP #6643 surfaced). Added, plus a test pinning it.
catch (e)binding to match theno-op-catch convention used elsewhere in this same PR (
getValidatedMasLibsUrl'sown
catch { ... }).isCommerceBlock'smerchalternative(
/merch|^mas-/->/^merch|^mas-/) so a block whose name merely contains"merch" isn't misclassified as commerce. Harmless today, added a regression test.
Not changed (kept as-is, confirmed intentional - replied on both threads in #6643):
?mep=offordering question:preloadLcpCodeFiles()staysgated behind the existing
if (mepParam === 'off') return;. When MEP is off there'sno MEP/Target await to hide the preload behind, and the normal block-loading path
already loads everything fast enough on its own - so this only needs to run when
MEP is present, and adding the coupling isn't worth it.
getValidatedMasLibsUrl- that JSDoc isn't newauthorship, it's the pre-existing block (incl. its VULN-36379 security rationale)
being relocated verbatim as part of this PR's reordering, and the neighboring
getValidatedRepoOwnerOrigin-style validator elsewhere in the file keeps the sameJSDoc convention.
in
libs/utils/utils.js, per this repo's convention for that file.Verification: full
Utilssuite locally (215/215 passing, incl. 2 new tests forthe crossorigin fix and the
isCommerceBlockanchor), ESLint clean. I intended to alsodrive a live before/after comparison on the PR's own
aem.pagelinks via Playwright,but couldn't get exclusive access to the browser instance in my environment - flagging
so nobody assumes that pass happened. Worth a manual before/after look before merge
given this touches a perf-sensitive path.
🤖 Generated with Claude Code