Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MWPW-154899 - Pilot preloading marquee dependencies #383

Closed
wants to merge 4 commits into from
Closed

MWPW-154899 - Pilot preloading marquee dependencies #383

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
710d09f
MWPW-154899 - Pilot preloading marquee dependencies
auniverseaway Aug 6, 2024
8f20a75
Merge branch 'stage' into marqpreload
salonijain3 Aug 7, 2024
d45cdd1
Merge branch 'stage' into marqpreload
auniverseaway Aug 20, 2024
0032103
Merge branch 'stage' into marqpreload
auniverseaway Aug 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion creativecloud/scripts/scripts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,14 @@ const CONFIG = {
],
};

function loadLCPPreloads(miloLibs, loadLink) {
const marqueeLcp = document.body.querySelector('main > div:nth-child(1) > .marquee');
if (!marqueeLcp) return;
loadLink(`${miloLibs}/blocks/marquee/marquee.css`, { rel: 'stylesheet' });
loadLink(`${miloLibs}/blocks/marquee/marquee.js`, { as: 'script', rel: 'modulepreload' });
loadLink(`${miloLibs}/utils/decorate.js`, { as: 'script', rel: 'modulepreload' });
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

@auniverseaway
The changes currently rely on the assumption that the default authoring of the first block on the page is marquee and it will always be marquee
In a case if there is a marquee on page with mep personalization that changes it to hero-marquee..
marquee js and css would get loaded
but then mep will change it
milo will run as usual and hero-marquee js and css will get loaded
So in this scenario extra call was made to a js or css that was not needed

Copy link
Member Author

Choose a reason for hiding this comment

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

The changes currently rely on the assumption that the default authoring of the first block on the page is marquee and it will always be marquee

This isn't quite accurate, but I think I know where you're going with your question. FWIW, this does not assume marquee is the first block on a page. Only that there is a marquee somewhere in the first section. If marquee is not in the first section, this does nothing and that is the intention.

In a case if there is a marquee on page with mep personalization that changes it to hero-marquee..
marquee js and css would get loaded
but then mep will change it
milo will run as usual and hero-marquee js and css will get loaded
So in this scenario extra call was made to a js or css that was not needed

Yes. This is expected. Hence, "pilot" and "out of scope: mep awareness" as by the time MEP fires, this would not provide any value. It's too late in the order of operations. The ideal use case is that MEP replaces a same block for same block, but with different content. This is not meant to cover all edge cases, just enough to collect some data to see if this improves performance.

Copy link

@mokimo mokimo Aug 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@aishwaryamathuria we want to trial this as part of the performance tiger team, we are closely observing the numbers and want to see if this has any positive benefit.

As Chris mentioned, if this assumption (marquee in the first section) is not true, there isn't a lot of harm. Would be awesome if we could take it forward, if the concept works on CC we might want to adopt it more broadly in milo-core.

if its doing nothing, we can remove the code again.


/*
* ------------------------------------------------------------
* Edit below at your own risk
Expand All @@ -180,7 +188,7 @@ const CONFIG = {

const isSignedInHomepage = window.location.pathname.includes(CHINA_SIGNED_IN_HOME_PATH);
const miloLibs = setLibs(LIBS);
const { loadArea, setConfig, loadLana } = await import(`${miloLibs}/utils/utils.js`);
const { loadArea, setConfig, loadLana, loadLink } = await import(`${miloLibs}/utils/utils.js`);
setConfig({ ...CONFIG, miloLibs });
if (isSignedInHomepage) acomsisCookieHandler();

Expand All @@ -199,5 +207,6 @@ decorateArea();

(async function loadPage() {
loadLana({ clientId: 'cc' });
loadLCPPreloads(miloLibs, loadLink);
await loadArea();
}());
Loading