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.

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

Stepper: Performance fine-tuning #99352

Open
wants to merge 10 commits into
base: trunk
Choose a base branch
from
Original file line number Diff line number Diff line change
@@ -1,17 +1,27 @@
import { HelpCenter, User as UserStore } from '@automattic/data-stores';
import { HelpCenter } from '@automattic/data-stores';
import { useDispatch } from '@wordpress/data';
import { useCallback } from 'react';
import AsyncLoad from 'calypso/components/async-load';
import { useSelector } from 'calypso/state';
import { isUserLoggedIn } from 'calypso/state/current-user/selectors';
import type { User as UserStore } from '@automattic/data-stores';

const HELP_CENTER_STORE = HelpCenter.register();

const AsyncHelpCenter: React.FC< { user: UserStore.CurrentUser | undefined } > = ( { user } ) => {
const isLoggedIn = useSelector( isUserLoggedIn );

const { setShowHelpCenter } = useDispatch( HELP_CENTER_STORE );

const handleClose = useCallback( () => {
setShowHelpCenter( false );
}, [ setShowHelpCenter ] );

// The Help Center only works if you're logged in. Don't waste time loading it if you're not.
if ( ! isLoggedIn ) {
return null;
}

/**
* The stepper query parameter ensures Webpack treats this Help Center as separate from the one in the main client app.
* Without it, Webpack would create one shared chunk, loaded in both apps. Since Stepper is smaller, more CSS would
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { SiteDetails } from '@automattic/data-stores';
import debugFactory from 'debug';
import { useEffect } from 'react';
import { Flow, StepperStep } from '../types';

const debug = debugFactory( 'calypso:stepper:preloading' );

async function tryPreload( step?: StepperStep, followingStep?: StepperStep ) {
if ( step && 'asyncComponent' in step ) {
debug( 'Preloading next step:', step.slug );

await step.asyncComponent();

// Flows are indeterminate, they often pick one of the two next steps based on user input, so load two steps ahead.
tryPreload( followingStep );
}
}

/**
* Preload the next step in the flow, if it's an async component.
*
* @param siteSlugOrId The site slug or ID.
* @param selectedSite The selected site.
* @param currentStepRoute The current step route.
* @param flowSteps The flow steps.
* @param flow The flow.
*/
export function usePreloadSteps(
siteSlugOrId: string | number,
selectedSite: SiteDetails | undefined | null,
currentStepRoute: string,
flowSteps: readonly StepperStep[],
flow: Flow
) {
useEffect( () => {
if ( siteSlugOrId && ! selectedSite ) {
// If this step depends on a selected site, only preload after we have the data.
// Otherwise, we're still waiting to render something meaningful, and we don't want to
// potentially slow that down by having the CPU busy initialising future steps.
return;
}
if ( currentStepRoute ) {
// The user step is a special case, as it's not part of the flow steps. It always comes in the end of the steps array.
if ( currentStepRoute === 'user' ) {
const nextStep = flowSteps[ 0 ];
const nextNextStep = flowSteps[ 1 ];
tryPreload( nextStep, nextNextStep );
} else {
const nextStepIndex = flowSteps.findIndex( ( step ) => step.slug === currentStepRoute ) + 1;
const nextNextStepIndex = nextStepIndex + 1;

const nextStep = flowSteps[ nextStepIndex ];
const nextNextStep = flowSteps[ nextNextStepIndex ];

tryPreload( nextStep, nextNextStep );
}
}
// Most flows sadly instantiate a new steps array on every call to `flow.useSteps()`,
// which means that we don't want to depend on `flowSteps` here, or this would end up
// running on every render. We thus depend on `flow` instead.
//
// This should be safe, because flows shouldn't return different lists of steps at
// different points. But even if they do, worst case scenario we only fail to preload
// some steps, and they'll simply be loaded later.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [ siteSlugOrId, selectedSite, currentStepRoute, flow ] );
}
44 changes: 4 additions & 40 deletions client/landing/stepper/declarative-flow/internals/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { Boot } from './components/boot';
import { RedirectToStep } from './components/redirect-to-step';
import { useFlowAnalytics } from './hooks/use-flow-analytics';
import { useFlowNavigation } from './hooks/use-flow-navigation';
import { usePreloadSteps } from './hooks/use-preload-steps';
import { useSignUpStartTracking } from './hooks/use-sign-up-start-tracking';
import { useStepNavigationWithTracking } from './hooks/use-step-navigation-with-tracking';
import { PRIVATE_STEPS } from './steps';
Expand Down Expand Up @@ -97,37 +98,7 @@ export const FlowRenderer: React.FC< { flow: Flow; steps: readonly StepperStep[]
const selectedSite = useSelector( ( state ) => site && getSite( state, siteSlugOrId ) );

// this pre-loads the next step in the flow.
useEffect( () => {
const nextStepIndex = flowSteps.findIndex( ( step ) => step.slug === currentStepRoute ) + 1;
const nextStep = flowSteps[ nextStepIndex ];

// 0 implies the findIndex returned -1.
if ( nextStepIndex === 0 || ! nextStep ) {
return;
}

if ( siteSlugOrId && ! selectedSite ) {
// If this step depends on a selected site, only preload after we have the data.
// Otherwise, we're still waiting to render something meaningful, and we don't want to
// potentially slow that down by having the CPU busy initialising future steps.
return;
}
if (
// Don't load anything on user step because the user step will hard-navigate anyways.
currentStepRoute !== 'user' &&
'asyncComponent' in nextStep
) {
nextStep.asyncComponent();
}
// Most flows sadly instantiate a new steps array on every call to `flow.useSteps()`,
// which means that we don't want to depend on `flowSteps` here, or this would end up
// running on every render. We thus depend on `flow` instead.
//
// This should be safe, because flows shouldn't return different lists of steps at
// different points. But even if they do, worst case scenario we only fail to preload
// some steps, and they'll simply be loaded later.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [ siteSlugOrId, selectedSite, currentStepRoute, flow ] );
usePreloadSteps( siteSlugOrId, selectedSite, currentStepRoute, flowSteps, flow );

const stepNavigation = useStepNavigationWithTracking( {
flow,
Expand Down Expand Up @@ -168,8 +139,8 @@ export const FlowRenderer: React.FC< { flow: Flow; steps: readonly StepperStep[]
}

// The `nextStep` is available only when logged-out users go to the step that requires auth
// and are redirected to the user step.
const postAuthStepSlug = stepData?.nextStep ?? '';
// and are redirected to the user step. When it's not available, go to the first step.
const postAuthStepSlug = stepData?.nextStep ?? stepPaths[ 0 ];
if ( step.slug === PRIVATE_STEPS.USER.slug && postAuthStepSlug ) {
const previousAuthStepSlug = stepData?.previousStep;
const postAuthStepPath = generatePath( '/setup/:flow/:step/:lang?', {
Expand Down Expand Up @@ -205,13 +176,6 @@ export const FlowRenderer: React.FC< { flow: Flow; steps: readonly StepperStep[]
);
}

if ( step.slug === PRIVATE_STEPS.USER.slug ) {
// eslint-disable-next-line no-console
console.warn(
'Please define the next step after auth explicitly as we cannot find the user step automatically.'
);
}

return (
<StepComponent
navigation={ stepNavigation }
Expand Down
Loading