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

Boost: Improve responsiveness during CCSS generation retry #40675

Merged
merged 10 commits into from
Dec 20, 2024
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import { __ } from '@wordpress/i18n';
import Status from '../status/status';
import { useCriticalCssState } from '../lib/stores/critical-css-state';
import { useRetryRegenerate } from '../lib/use-retry-regenerate';
import { isFatalError } from '../lib/critical-css-errors';

export default function CloudCssMetaProps() {
const [ cssState ] = useCriticalCssState();
const [ hasRetried, retry ] = useRetryRegenerate();

const isPending = cssState.status === 'pending';
const hasCompletedSome = cssState.providers.some( provider => provider.status !== 'pending' );
Expand All @@ -29,8 +27,6 @@ export default function CloudCssMetaProps() {
cssState={ cssState }
isCloud={ true }
showFatalError={ isFatalError( cssState ) }
hasRetried={ hasRetried }
retry={ retry }
extraText={ extraText || undefined }
overrideText={ overrideText || undefined }
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,48 +11,58 @@ import {
import { runLocalGenerator } from '../lib/generate-critical-css';
import { CriticalCssErrorDetails } from '../lib/stores/critical-css-state-types';

type LocalGeneratorContext = {
type CriticalCssContextValues = {
isGenerating: boolean;
setGenerating: ( generating: boolean ) => void;

providerProgress: number;
setProviderProgress: ( progress: number ) => void;

// Whether we've retried generating critical CSS after an error.
hasRetriedAfterError: boolean;
setHasRetriedAfterError: ( hasRetried: boolean ) => void;
};

type ProviderProps = {
children: ReactNode;
};

const CssGeneratorContext = createContext< LocalGeneratorContext | null >( null );
const CriticalCssContext = createContext< CriticalCssContextValues | null >( null );

/**
* Local Critical CSS Context Provider component - provides context for any descendants that want to
* either initiate the local Critical CSS generator, or check its status.
*
* @param {ProviderProps} props - Component props.
*/
export default function LocalCriticalCssGeneratorProvider( { children }: ProviderProps ) {
export default function CriticalCssProvider( { children }: ProviderProps ) {
const [ isGenerating, setGenerating ] = useState< boolean >( false );
const [ providerProgress, setProviderProgress ] = useState< number >( 0 );
const [ hasRetriedAfterError, setHasRetriedAfterError ] = useState< boolean >( false );

const value = {
// Local Generator status.
isGenerating,
setGenerating,
providerProgress,
setProviderProgress,

// Whether we've retried generating critical CSS after an error.
hasRetriedAfterError,
setHasRetriedAfterError,
};

return <CssGeneratorContext.Provider value={ value }>{ children }</CssGeneratorContext.Provider>;
return <CriticalCssContext.Provider value={ value }>{ children }</CriticalCssContext.Provider>;
}

/**
* Internal helper function: Use the raw Critical CSS Generator context, and verify it's inside a provider.
*/
function useLocalCriticalCssGeneratorContext() {
const status = useContext( CssGeneratorContext );
function useCriticalCssContext() {
const status = useContext( CriticalCssContext );

if ( ! status ) {
throw new Error( 'Local critical CSS generator status not available' );
throw new Error( 'Critical CSS status not available' );
}

return status;
Expand All @@ -62,18 +72,26 @@ function useLocalCriticalCssGeneratorContext() {
* For status consumers: Get an overview of the local critical CSS generator status. Is it running or not?
*/
export function useLocalCriticalCssGeneratorStatus() {
const { isGenerating, providerProgress } = useLocalCriticalCssGeneratorContext();
const { isGenerating, providerProgress } = useCriticalCssContext();

return { isGenerating, providerProgress };
}

/** The retried state of critical CSS. */
export function useCriticalCssRetriedAfterErrorState() {
const { hasRetriedAfterError: hasRetried, setHasRetriedAfterError: setHasRetried } =
useCriticalCssContext();

return [ hasRetried, setHasRetried ] as const;
}

/**
* For Critical CSS UI: Actually run the local generator and return its status.
*/
export function useLocalCriticalCssGenerator() {
// Local Generator status context.
const { isGenerating, setGenerating, providerProgress, setProviderProgress } =
useLocalCriticalCssGeneratorContext();
useCriticalCssContext();

// Critical CSS state and actions.
const [ cssState, setCssState ] = useCriticalCssState();
Expand All @@ -86,7 +104,7 @@ export function useLocalCriticalCssGenerator() {

useEffect(
() => {
if ( cssState.status === 'pending' ) {
if ( cssState.status === 'pending' && cssState.providers.length > 0 ) {
let abortController: AbortController | undefined;

setGenerating( true );
Expand Down Expand Up @@ -119,7 +137,7 @@ export function useLocalCriticalCssGenerator() {
// This effect triggers an actual process that is costly to start and stop, so we don't want to start/stop it
// every time an object ref like `cssState` is changed for a trivial reason.
// eslint-disable-next-line react-hooks/exhaustive-deps
[ cssState.status ]
[ cssState.status, cssState.providers.length ]
);

const progress =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ import ProgressBar from '$features/ui/progress-bar/progress-bar';
import styles from './critical-css-meta.module.scss';
import { useCriticalCssState } from '../lib/stores/critical-css-state';
import { RegenerateCriticalCssSuggestion, useRegenerationReason } from '..';
import { useLocalCriticalCssGenerator } from '../local-generator/local-generator-provider';
import { useRetryRegenerate } from '../lib/use-retry-regenerate';
import { useLocalCriticalCssGenerator } from '../critical-css-context/critical-css-context-provider';
import { isFatalError } from '../lib/critical-css-errors';

/**
Expand All @@ -14,12 +13,11 @@ import { isFatalError } from '../lib/critical-css-errors';
*/
export default function CriticalCssMeta() {
const [ cssState ] = useCriticalCssState();
const [ hasRetried, retry ] = useRetryRegenerate();
const [ { data: regenerateReason } ] = useRegenerationReason();
const { progress } = useLocalCriticalCssGenerator();
const showFatalError = isFatalError( cssState );

if ( cssState.status === 'pending' ) {
if ( cssState.status === 'pending' || cssState.status === 'not_generated' ) {
return (
<div className="jb-critical-css-progress">
<div className={ styles[ 'progress-label' ] }>
Expand All @@ -39,8 +37,6 @@ export default function CriticalCssMeta() {
cssState={ cssState }
isCloud={ false }
showFatalError={ showFatalError }
hasRetried={ hasRetried }
retry={ retry }
highlightRegenerateButton={ !! regenerateReason }
extraText={ __(
'Remember to regenerate each time you make changes that affect your HTML or CSS structure.',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ export function isFatalError( cssState: CriticalCssState ): boolean {
return false;
}

// If there are no providers, the state is being re-initialized. So dismiss any show-stopper errors.
if ( cssState.providers.length === 0 ) {
return false;
}

const hasActiveProvider = cssState.providers.some(
provider => provider.status === 'success' || provider.status === 'pending'
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,20 @@ export function criticalCssErrorState( message: string ): CriticalCssState {
* All Critical CSS State actions return a success flag and the new state. This hook wraps the
* common logic for handling the result of these actions.
*
* @param {string} action - The name of the action.
* @param {z.ZodSchema} schema - The schema for the action request.
* @param {Function} onSuccess - Optional callback for handling the new state.
* @param {string} action - The name of the action.
* @param {z.ZodSchema} schema - The schema for the action request.
* @param {CriticalCssState} optimisticState - The state to use for optimistic updates.
* @param {Function} onSuccess - Optional callback for handling the new state.
*/
function useCriticalCssAction<
ActionSchema extends z.ZodSchema,
ActionRequestData extends z.infer< ActionSchema >,
>( action: string, schema: ActionRequestData, onSuccess?: ( state: CriticalCssState ) => void ) {
>(
action: string,
schema: ActionRequestData,
optimisticState?: CriticalCssState,
onSuccess?: ( state: CriticalCssState ) => void
) {
const responseSchema = z.object( {
success: z.boolean(),
state: CriticalCssStateSchema,
Expand All @@ -90,6 +96,13 @@ function useCriticalCssAction<
action_response: responseSchema,
},
callbacks: {
optimisticUpdate: ( _requestData, state: CriticalCssState ) => {
if ( optimisticState ) {
return optimisticState;
}

return state;
},
onResult: ( result, _state ): CriticalCssState => {
if ( result.success ) {
if ( onSuccess ) {
Expand Down Expand Up @@ -150,10 +163,23 @@ export function useSetProviderErrorsAction() {

/**
* Hook which creates a callable action for regenerating Critical CSS.
*
* @param {Function} callback - Optional callback to call when a regeneration starts successfully.
*/
export function useRegenerateCriticalCssAction() {
export function useRegenerateCriticalCssAction( callback?: () => void ) {
const [ , resetReason ] = useRegenerationReason();
return useCriticalCssAction( 'request-regenerate', z.void(), resetReason );

const onSuccess = () => {
if ( callback ) {
callback();
}

resetReason();
};

// Optimistically update the state to hide any errors and immediately show the pending state.
const optimisticState: CriticalCssState = { status: 'pending', providers: [] };
return useCriticalCssAction( 'request-regenerate', z.void(), optimisticState, onSuccess );
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { useState } from 'react';
import { useRegenerateCriticalCssAction } from './stores/critical-css-state';
import { useCriticalCssRetriedAfterErrorState } from '../critical-css-context/critical-css-context-provider';

/**
* Helper for "Retry" buttons for Critical CSS which need to track whether they have been clicked
Expand All @@ -8,13 +8,12 @@ import { useRegenerateCriticalCssAction } from './stores/critical-css-state';
* Returns a boolean indicating whether retrying has been attempted, and a function to call to retry.
*/
export function useRetryRegenerate(): [ boolean, () => void ] {
const [ retried, setRetried ] = useState( false );
const regenerateAction = useRegenerateCriticalCssAction();
const [ retriedAfterError, setRetriedAfterError ] = useCriticalCssRetriedAfterErrorState();
const regenerateAction = useRegenerateCriticalCssAction( () => setRetriedAfterError( true ) );

function retry() {
setRetried( true );
regenerateAction.mutate();
}

return [ retried, retry ];
return [ retriedAfterError, retry ];
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,16 @@ import getCriticalCssErrorSetInterpolateVars from '$lib/utils/get-critical-css-e
import formatErrorSetUrls from '$lib/utils/format-error-set-urls';
import actionLinkInterpolateVar from '$lib/utils/action-link-interpolate-var';
import { recordBoostEvent } from '$lib/utils/analytics';
import { useRetryRegenerate } from '../lib/use-retry-regenerate';

type ShowStopperErrorTypes = {
supportLink?: string;
cssState: CriticalCssState;
retry: () => void;
showRetry?: boolean;
};

const ShowStopperError: React.FC< ShowStopperErrorTypes > = ( {
supportLink = 'https://wordpress.org/support/plugin/jetpack-boost/',
cssState,
retry,
showRetry,
} ) => {
const primaryErrorSet = getPrimaryErrorSet( cssState );
const showLearnSection = primaryErrorSet && cssState.status === 'generated';
Expand Down Expand Up @@ -55,12 +52,7 @@ const ShowStopperError: React.FC< ShowStopperErrorTypes > = ( {
</FoldingElement>
</>
) : (
<OtherErrors
cssState={ cssState }
retry={ retry }
showRetry={ showRetry }
supportLink={ supportLink }
/>
<OtherErrors cssState={ cssState } supportLink={ supportLink } />
) }
</Notice>
</>
Expand Down Expand Up @@ -136,7 +128,9 @@ const DocumentationSection = ( {
);
};

const OtherErrors = ( { cssState, retry, showRetry, supportLink }: ShowStopperErrorTypes ) => {
const OtherErrors = ( { cssState, supportLink }: ShowStopperErrorTypes ) => {
const [ hasRetried, retry ] = useRetryRegenerate();

const firstTimeError = __(
'An unexpected error has occurred. As this error may be temporary, please try and refresh the Critical CSS.',
'jetpack-boost'
Expand Down Expand Up @@ -184,15 +178,15 @@ const OtherErrors = ( { cssState, retry, showRetry, supportLink }: ShowStopperEr
</>
) : (
<>
<p>{ showRetry ? firstTimeError : secondTimeError }</p>
<p>{ ! hasRetried ? firstTimeError : secondTimeError }</p>
<p>
{ sprintf(
/* translators: %s: error message */
__( `Error: %s`, 'jetpack-boost' ),
cssState.status_error
) }
</p>
{ showRetry ? (
{ ! hasRetried ? (
<button
className="secondary"
onClick={ () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ type StatusTypes = {
cssState: CriticalCssState;
isCloud?: boolean;
showFatalError: boolean;
hasRetried: boolean;
retry: () => void;
highlightRegenerateButton?: boolean;
extraText?: string; // Optionally, provide a sentence to use after the main message to provide more context.
overrideText?: string; // Optionally, provide a custom message to display instead of the default.
Expand All @@ -28,8 +26,6 @@ const Status: React.FC< StatusTypes > = ( {
cssState,
isCloud = false,
showFatalError,
hasRetried,
retry,
highlightRegenerateButton = false,
extraText,
overrideText,
Expand All @@ -54,8 +50,6 @@ const Status: React.FC< StatusTypes > = ( {
<ShowStopperError
supportLink={ ( isCloud && 'https://jetpack.com/contact-support/' ) || undefined }
cssState={ cssState }
retry={ retry }
showRetry={ ! hasRetried }
/>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { useDebouncedRefreshScore, useSpeedScores } from './lib/hooks';
import styles from './speed-score.module.scss';
import { useModulesState } from '$features/module/lib/stores';
import { useCriticalCssState } from '$features/critical-css/lib/stores/critical-css-state';
import { useLocalCriticalCssGeneratorStatus } from '$features/critical-css/local-generator/local-generator-provider';
import { useLocalCriticalCssGeneratorStatus } from '$features/critical-css/critical-css-context/critical-css-context-provider';
import { queryClient } from '@automattic/jetpack-react-data-sync-client';
import ErrorBoundary from '$features/error-boundary/error-boundary';
import PopOut from './pop-out/pop-out';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import Tips from './tips/tips';
import clsx from 'clsx';
import styles from './settings-page.module.scss';
import { usePremiumFeatures } from '$lib/stores/premium-features';
import LocalCriticalCssGeneratorProvider from '$features/critical-css/local-generator/local-generator-provider';
import CriticalCssProvider from '$features/critical-css/critical-css-context/critical-css-context-provider';
import NoticeManager from '$features/notice/manager';
import { NoticeProvider } from '$features/notice/context';

Expand All @@ -20,7 +20,7 @@ const SettingsPage = ( { children }: SettingsPageProps ) => {

return (
<NoticeProvider>
<LocalCriticalCssGeneratorProvider>
<CriticalCssProvider>
<div id="jb-dashboard" className="jb-dashboard jb-dashboard--main">
<Header />

Expand All @@ -41,7 +41,7 @@ const SettingsPage = ( { children }: SettingsPageProps ) => {
<Footer />
<NoticeManager />
</div>
</LocalCriticalCssGeneratorProvider>
</CriticalCssProvider>
</NoticeProvider>
);
};
Expand Down
Loading
Loading