Lazy Load AuthModal

[dani97]

Description

The AuthModal component can be lazy loaded, as guest users may never use it.

This saves us ~6.5 KB on initial load of the application.

Related Issue

Closes PWA-113.

Acceptance

@zetlen

Verification Stakeholders

Specification

Verification Steps

1. yarn watch:venia
2. Open Venia in the browser
3. Open the left (navigation) drawer
4. View Browser's "Network" tab
5. Click the "Sign In" button
6. Verify that new entries appeared in the Network tab (downloading AuthModal chunk)
7. Verify that the AuthModal chunk loads and the app works as expected

You can also verify that the loading spinner appears by artificially slowing the network speed in the Network tab before clicking the Sign In button.

Screenshots / Screen Captures (if appropriate)

Checklist

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

[PWAStudioBot]

	Messages
📖	Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.
📖	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).
📖	Associated JIRA tickets: PWA-113.

Generated by 🚫 dangerJS against 741a54d

[zetlen]

@dani97 This seems like too granular of a place for dynamic imports. Can you post a short screen capture session showing us how quickly this responds to user input? It might be that we want to combine all those possible AuthModal screens into a single dynamic chunk, rather than separate ones.

[dani97]

I can see a bit of lag on 3g connections but user experience is still good. I visited Sign in form, Forgot Password form and Create Account form

At no throttling, with cache

At 3g connection, without cache

[zetlen]

Can you confirm that all three of these new dynamic imports are combining into a single chunk? If chunks get too small, then the overhead of Webpack boilerplate starts to be too much.

Don't get me wrong, I appreciate the contribution! I just need to be sure that we benefit from asynchronous imports at this level of granularity.

[dani97]

Lazy loaded AuthModal and got this result. I thought of this approach but this bundle is fetched before user opens authModal as AuthModal is mounted initially, regardless not present in client bundle

[dani97]

Can you confirm that all three of these new dynamic imports are combining into a single chunk? If chunks get too small, then the overhead of Webpack boilerplate starts to be too much.

Don't get me wrong, I appreciate the contribution! I just need to be sure that we benefit from asynchronous imports at this level of granularity.

• I agree, when I lazy loaded all of these individually I get components as separate chunks and not a single bundle. In a ideal case a registered customer won't go for forgot password or create account or login component. The person will stick with customer account component.
• If a registered user installs the PWA , will login and may use sign in and forgot password. He may never use create account component.
• A person who wants to perform a guest checkout may never use any of these components
  Your thoughts?

[zetlen]

Seems like login -> forgot password -> create account is a common enough use case. I'd worry about bad network latency in those very bounce-prone workflows.

Did you try changing the place where AuthModal is important to be a dynamic import, rather than changing all of its downstream dependencies?

[dani97]

Seems like login -> forgot password -> create account is a common enough use case. I'd worry about bad network latency in those very bounce-prone workflows.

Did you try changing the place where AuthModal is important to be a dynamic import, rather than changing all of its downstream dependencies?
#1955 (comment) added the results here, just a new bundle that is called at initial page load

[jimbo]

@zetlen Do you recall how we had to change renderRoutes.js so that <Suspense> was above the <Switch> element instead of below it in order to fix some strange behavior? Do you think we're likely to encounter that same behavior with these <Suspense> elements being conditionally rendered?

[zetlen]

@jimbo I don't know; we'd need a test case to be sure. That bug with Suspense and Switch was easily reproducible because it was completely broken. THis isn't.

[supernova-at]

Did you try changing the place where AuthModal is important to be a dynamic import, rather than changing all of its downstream dependencies?

PR updated to show what this would look like.

AuthModal and all of its dependencies are not loaded until the "Sign In" button in the left drawer is clicked.

Unit tests updated in 234ae42.

[supernova-at]

Looks like this saves us ~6.5 KB:

[sirugh]

Great job and amazing effort on the talon tests.

[dpatil-magento]

@dani97 @supernova-at Can you please resolve the conflict?