Conversation
|
|
||
| config.mepXlgTagsURL = xlgUrl; | ||
| resolve(xlgUrl); | ||
| }); |
There was a problem hiding this comment.
I tested this locally and wanted to share a few things I noticed — mostly around how the caching and fetch flow interact. I think the intent makes sense, but a couple of edge cases seem to cause unexpected behavior:
-
Undefined fetch case
On the second call to getXLGListURL (from getEntitlementMap), the function returns undefined because config.mepXlgTagsURL is already truthy from the first call. That ends up calling fetchData(undefined), which results in a fetch("undefined") request (you can see it fail in the network/perf waterfall). So instead of skipping the duplicate download, we end up with an erroneous request, and the preloaded XLG data never actually gets consumed since getEntitlementMap never receives a valid URL. -
Cache getting reset
After that second call, config.mepXlgTagsURL is set back to undefined, which means the “already fetched” guard wouldn’t hold if a third call were to happen. -
Promise wrapper
isSignedInUser() looks to be synchronous, so wrapping it in new Promise(resolve => …) adds a microtask delay without really buying us anything.
One alternative that seems to avoid those issues is: kick off the fetch early and cache the Promise itself, so getEntitlementMap can reuse the same in‑flight/resolved response rather than re-deriving the URL each time:
export const getXLGListURL = (config) => {
if (config.mepXlgTagsURL) return config.mepXlgTagsURL;
if (!window.adobeIMS?.isSignedInUser()) return undefined;
const sheet = config.env?.name === 'prod' ? 'prod' : 'stage';
const rawUrl = `https://www.adobe.com/federal/assets/data/mep-xlg-tags.json?sheet=${sheet}`;
config.mepXlgTagsURL = normalizePath(rawUrl);
return config.mepXlgTagsURL;
};
export function prefetchXLGTags(config) {
const url = getXLGListURL(config);
if (!url) return;
config.mepXlgTagsPromise = fetchData(url, DATA_TYPE.JSON);
}
Then in getEntitlementMap, reuse the cached promise:
const fetchedData = config.mepXlgTagsPromise ? await config.mepXlgTagsPromise : await fetchData(getXLGListURL(config), DATA_TYPE.JSON);
And in init, replace the loadLink preload with:
if (pzn || pznroc) prefetchXLGTags(config);
That should give us one fetch, no wasted preload, no undefined request, and the data actually gets consumed. And again — the isSignedInUser() guard to skip signed‑out users is great; I’d definitely keep that.
There was a problem hiding this comment.
Thanks. I haven't really used promises since Angular. It was something Viv suggested. Updated to work as per your specifications.
|
This pull request is not passing all required checks. Please see this discussion for information on how to get all checks passing. Inconsistent checks can be manually retried. If a test absolutely can not pass for a good reason, please add a comment with an explanation to the PR. |
|
This PR has not been updated recently and will be closed in 7 days if no action is taken. Please ensure all checks are passing, https://github.com/orgs/adobecom/discussions/997 provides instructions. If the PR is ready to be merged, please mark it with the "Ready for Stage" label. |

Adds a check to prevent getXLGListURL() from firing multiple times by utilizing promises. It should only perform the call once.
Behavior
Instructions
Before:

After:

Resolves: OPT-39999
Test URLs: