Skip to content

removed loadAlloy function; deferred instruments.js#371

Open
maxn-adobe wants to merge 1 commit intostagefrom
MWPW-192838
Open

removed loadAlloy function; deferred instruments.js#371
maxn-adobe wants to merge 1 commit intostagefrom
MWPW-192838

Conversation

@maxn-adobe
Copy link
Copy Markdown
Contributor

@maxn-adobe maxn-adobe commented Apr 16, 2026

Summary

Modified scripts.js, removed loadAlloy function; deferred loading of instruments.js.

Here are explanations of both changes that were made:

Fix 1: Defer instrument.js import until after loadArea()

Previously, instrument.js was imported as a fire-and-forget dynamic import immediately before await loadArea() — meaning the browser was simultaneously trying to download, parse, and execute the analytics module at the same time it was painting the LCP element. This created two forms of contention during the most performance-sensitive window on the page: network bandwidth competition (the Demdex audience segment fetch in instrument.js races against the LCP image fetch) and main thread pressure (the alloy_all data layer setup and event listener registration add JS execution cost while the browser is trying to complete layout and paint).

The fix moves the instrument.js import to after await loadArea() completes, placing it alongside express-delayed.js where deferred work already lands. This is also semantically more correct: instrument.js's default export (martechLoadedCB) calls decorateAnalyticsEvents() which sets data-analytics attributes on DOM elements — work that requires the full DOM to be present anyway. No analytics fidelity is lost by this change; click tracking listeners attached after LCP miss no meaningful interactions during the page-load phase, and the audience segment data for the ratings block arrives only marginally later with no user-visible effect.

Fix 2: Remove listenAlloy()

listenAlloy() was a function introduced to expose window.alloyLoader — a Promise that was intended to resolve when alloy dispatched a pageView event. The function had a // TODO this method should be removed about two weeks after going live comment indicating it was always meant to be temporary. Investigation confirms it is now dead code for two independent reasons: first, the event listener inside it checks e.detail.type === 'pageView', but the alloy_sendEvent custom event dispatched by milo's helpers.js has a detail object of { destinations, propositions, inferences, decisions } with no type field — so the condition is never true and the promise is never resolved via the event. Second, a codebase-wide search found zero consumers of window.alloyLoader in either da-express-milo or milo, confirming nothing awaits it.

The practical consequence was that the setTimeout(..., 3000) fallback — the only code path that ever resolved the promise — fired unconditionally on every page load at exactly 3 seconds, creating a guaranteed JS task right in the middle of the LCP and TTI measurement windows. Removing the function and its call site eliminates this timer entirely with no effect on analytics, click tracking (which operates through Launch rules and the safelyFireAnalyticsEvent pattern in instrument.js independently), or any other runtime behavior.


Jira Ticket

Resolves: MWPW-192838


Test URLs

Env URL
Before https://main--da-express-milo--adobecom.aem.page/express/
After https://mwpw-192838--da-express-milo--adobecom.aem.page/express/

Verification Steps

These updates should have no visible or functional effect, other than increasing the pageload speed and improving the CWV's of our pages.


Potential Regressions

N/A


Additional Notes

stage_trace mwpw-192838_trace

@aem-code-sync
Copy link
Copy Markdown

aem-code-sync Bot commented Apr 16, 2026

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

  • Re-run PSI checks
  • Re-sync branch
Commits

@github-actions github-actions Bot added the Ready for Review Ready for peer review. label Apr 16, 2026
@aem-code-sync
Copy link
Copy Markdown

aem-code-sync Bot commented Apr 16, 2026

Page Scores Audits
📱 /express/ PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS
🖥️ /express/ PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS

@maxn-adobe
Copy link
Copy Markdown
Contributor Author

Ran the PR-review tool, including it's output here:
2026-04-17-PR371.md

Copy link
Copy Markdown
Contributor

@fullcolorcoder fullcolorcoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move that to delayed?

const { fixIcons } = await import('./utils.js');
document.querySelectorAll('.section>.text').forEach((block) => fixIcons(block));

const urlParams = new URLSearchParams(window.location.search);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR gets instrument.js out of the LCP window which is the main win, but it still fires immediately after loadArea() rather than inside express-delayed.js where true Phase D work lands. Since you've already shown the DOM needs to be complete anyway and no fidelity is lost by deferring, would it make sense to move this entirely into express-delayed.js? Not a blocker — the current improvement is substantial — just curious if there's a reason to keep it in the middle.

@nateyolles nateyolles added this to the Express-26.19 milestone Apr 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants