[PWA-1110] Investigate render/blocking in Venia (Performance)

[tjwiebell]

Description

As a developer, I want review and address issues with blockimg/response time (LCP/TBT) to achieve higher Google Lighthouse v6 performance scores as they relate to the shopping experience. This could affect the core code, Webpack and other areas. Analysis from PWA-775 can be found here: https://wiki.corp.magento.com/display/COREENG/Lighthouse+Performance+Audit+-+v8

In-scope

Deep analysis and review of blocking/response using DevTools and other resources. Recommend and remediate issues where needed.

Requirements

Investigate render/blocking with Venia in current state
Identify and remediate (fix) issues as appropriate; possible spike or docs
Testing and validation with Google Lighthouse

Implementation Note

This PR covers a single render blocking point at the very top of the tree added with i18n support. Instead of waiting on the network, the request flow will now cascade in this order:

1. Immediately render before messages are even loaded. This means defaultMessage props will be rendered at this point.
2. While network request for current store locale is in flight, use DEFAULT_LOCALE global that is queried for at build time and pre-load that language pack.
3. Once network request comes back, load language pack for that locale

For the happy path of an {{en_US}} store, this means we now immediately render at step 1, and even when step 2/3 completes, nothing in the application needs to re-render. For alternative paths that return a different locale, the app immediately renders with placeholders, and then re-renders with correctly translated strings at step 3. The flash of english strings happens in the application today, this just renders everything much faster.

Followup tickets have been added to the backlog to address the already existing flash of default locale strings; a simple solution being, always rely on build time storeConfig and preload messages while the network request is in flight. If the build time config becomes stale, the app would still recover, while still rendering immediately. We've also discussed rendering actual placeholders (grey boxes like skeleton rendering) instead of defaultMessages.

Related Issue

• [PWA-1110] Investigate render/blocking in Venia (Performance)

Acceptance

Verification Stakeholders

Specification

Verification Steps

1. Go to category page and clear all cache data
2. Run Lighthouse Performance audit
3. In the performance trace, verify that the getLocale graphql request is no longer render blocking (requests don't wait to fire until it returns)
4. Not required, but you can also compare Performance metrics with develop.pwa-venia.com. Network latency can cause these to be difficult to compare, but TTI and TBT should be lower, and the performance score should gain a few points.
5. Verify translation still works by switching to French store

Screenshots / Screen Captures (if appropriate)

Checklist

• I have added tests to cover my changes, if necessary.
• I have added translations for new strings, if necessary.
• I have updated the documentation accordingly, if necessary.

[PWAStudioBot]

	Messages
📖	Associated JIRA tickets: PWA-775.
📖	DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).
📖	Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.

Generated by 🚫 dangerJS against aca1523

[fooman]

Suggest to include an update to workbox. It includes an update to how manifests are loaded GoogleChrome/workbox#2528 which to my reading made the trace more in line with what I expected to load early (in some traces I for example had root components load that were not on the current page before actual images on the page were loaded).

[tjwiebell]

Suggest to include an update to workbox 5. It includes an update to how manifests are loaded GoogleChrome/workbox#2528 which to my reading made the trace more in line with what I expected to load early (in some traces I for example had root components load that were not on the current page before actual images on the page were loaded).

@fooman
I was just reviewing profiling results with the team, and we were all baffled why the service worker would do a flood of precache requests before the app has even finished rendering; I am very interested in this upgrade. It looks like we'd have to upgrade webpack as well, but I'll do some digging and if this won't have a huge impact I'll do it. Thanks for the heads up 👍

[fooman]

indeed I am on "webpack": "~4.40.0", but don't recollect that being an issue either.

[sirugh]

I wonder which is better:

a) Showing a loading indicator until fetchLocale resolves or
b) Not showing a loading indicator, ever, opting to have a potential "flash" when the translated strings come back from fetchLocale.

It would be really cool if we could read defaultMessage strings from some bundled file that agencies could customize depending on their own default language...

[tjwiebell]

That's comparing develop.pwa-venia.com to this PR, with this aiming to remove the render blocking of the loading indicator. Rendering the loading indicator means the context isn't getting rendered, including its children (the rest of the app).

The second part you outlined is exactly what this change does; while Apollo is still waiting on its response, we default to english and render the app immediately. If the response comes back and it's the same as the default, nothing re-renders.

I think the ideal solution here is to apply this logic to all stores; we can precache storeConfig at build time about all stores and immediately load the {StoreConfig.locale} language pack. If we don't want to require rebuilds when config changes, we can leave the query in place that will true up with the network to validate the precached config. I think we've been hinting at needing this solution for awhile, and would be another application to prevent unnecessary re-renders.

[fooman]

@tjwiebell a setting to have storeConfig pre-loaded would be great.

[sirugh]

Short and sweet. 👍

[dpatil-magento]

QA Approved.