MWPW-168334 Load georoutingv2.json and georoutingv2.js in parallel #3720
Conversation
|
sharmrj
requested review from
salonijain3,
bandana147,
vhargrave and
nishantka
| @@ -1250,8 +1250,11 @@ async function loadPostLCP(config) { | |||
|
|
|||
| const georouting = getMetadata('georouting') || config.geoRouting; | |||
| if (georouting === 'on') { | |||
| const jsonPromise = fetch(`${config.contentRoot ?? ''}/georoutingv2.json`) | |||
There was a problem hiding this comment.
to keep this even leaner , you could probably only call the fetch right? (without any .then or .catch handling)
| .catch(() => null); | ||
|
|
||
| const json = (await v2jsonPromise) ?? (await oldGeorouting()) ?? (await federatedJSON()); | ||
| if (json === 'use-old-georouting') return; |
There was a problem hiding this comment.
i would really recommend avoiding things like magic strings - https://softwareengineering.stackexchange.com/questions/365339/what-is-wrong-with-magic-strings
| .then((r) => r.json()) | ||
| .catch(() => null); | ||
|
|
||
| const json = (await v2jsonPromise) ?? (await oldGeorouting()) ?? (await federatedJSON()); |
There was a problem hiding this comment.
You're kind of cramming multiple asynchronous operations and fallback logic into a single expression here. I'd separate this out for readability.
|
|
||
| const json = (await v2jsonPromise) ?? (await oldGeorouting()) ?? (await federatedJSON()); | ||
| if (json === 'use-old-georouting') return; | ||
|
|
There was a problem hiding this comment.
Based on my upper comments, I'd maybe even just edit to something simpler like so (this is untested btw). Your call on how you want to do it though:
let json;
const response = await v2jsonPromise;
if(response.ok) json = await response.json();
if (!json) {
// Try old georouting
const r = await fetch(`${config.contentRoot ?? ''}/georouting.json`);
if (r.ok) {
const data = await r.json();
const { default: loadGeoRoutingOld } = await import('../georouting/georouting.js');
await loadGeoRoutingOld(config, createTag, getMetadata, data);
return;
}
}
if(!json) {
const r = await fetch(`${getFederatedContentRoot()}/federal/georouting/georoutingv2.json`);
if (r.ok) {
json = await r.json();
}
}
if (!json) return;
| @@ -271,6 +271,7 @@ export default async function loadGeoRouting( | |||
| getMetadataFunc, | |||
| loadBlockFunc, | |||
| loadStyleFunc, | |||
| v2jsonPromise, | |||
There was a problem hiding this comment.
can you give this the default value of a promise fetch to the georoutingV2.json file?
Before:
After:
Measurement done on slow 4G.
Resolves: MWPW-168334
Test URLs:
https://main--cc--adobecom.hlx.page?milolibs=optimize-georouting