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 @@ -5,7 +5,6 @@ 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 { 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 @@ -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 @@ -153,7 +166,10 @@ export function useSetProviderErrorsAction() {
*/
export function useRegenerateCriticalCssAction() {
const [ , resetReason ] = useRegenerationReason();
return useCriticalCssAction( 'request-regenerate', z.void(), 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, resetReason );
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,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 +119,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 @@ -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
4 changes: 4 additions & 0 deletions projects/plugins/boost/changelog/fix-critical-css-status
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fixed

Critical CSS: Improved UI responsiveness during a retry after failed generation.
Loading