CYS: Investigate multiple requests being made to the patterns endpoint (

woocommerce#51819) * Notify wc/admin/options store changes after the data is saved in the server and not before * Add changelog file * Remove the subscription to the OPTIONS_STORE_NAME since its trigered many times, using useSelect instead * Add changelog file
Neosate · Oct 2, 2024 · 4d7d049 · 4d7d049
1 parent 9be9fd9
commit 4d7d049
Show file tree

Hide file tree

Showing 4 changed files with 124 additions and 118 deletions.
diff --git a/packages/js/data/changelog/fix-50907 b/packages/js/data/changelog/fix-50907
@@ -0,0 +1,4 @@
+Significance: minor
+Type: fix
+
+Notify wc/admin/options store changes after the data is saved in the server and not before
diff --git a/packages/js/data/src/options/actions.ts b/packages/js/data/src/options/actions.ts
@@ -40,10 +40,9 @@ export function setIsUpdating( isUpdating: boolean ) {
 }
 
 export function* updateOptions( data: Options ) {
-	yield setIsUpdating( true );
-	yield receiveOptions( data );
-
 	try {
+		yield setIsUpdating( true );
+
 		const results: unknown = yield apiFetch( {
 			path: WC_ADMIN_NAMESPACE + '/options',
 			method: 'POST',
@@ -57,6 +56,9 @@ export function* updateOptions( data: Options ) {
 				`Invalid update options response from server: ${ results }`
 			);
 		}
+
+		yield receiveOptions( data );
+
 		return { success: true, ...results };
 	} catch ( error ) {
 		yield setUpdatingError( error );

diff --git a/plugins/woocommerce-admin/client/customize-store/assembler-hub/opt-in/opt-in.tsx b/plugins/woocommerce-admin/client/customize-store/assembler-hub/opt-in/opt-in.tsx
@@ -3,7 +3,7 @@
  */
 import { OPTIONS_STORE_NAME } from '@woocommerce/data';
 import apiFetch from '@wordpress/api-fetch';
-import { resolveSelect, select, subscribe, useDispatch } from '@wordpress/data';
+import { dispatch, resolveSelect, select, useSelect } from '@wordpress/data';
 import { useContext, useEffect } from '@wordpress/element';
 // @ts-expect-error No types for this exist yet.
 // eslint-disable-next-line @woocommerce/dependency-group
@@ -19,143 +19,139 @@ import { unlock } from '@wordpress/edit-site/build-module/lock-unlock';
  * Internal dependencies
  */
 import { FontFamily, FontFace } from '../../types/font';
-import { usePatterns } from '../hooks/use-patterns';
 import { installFontFamilies } from '../utils/fonts';
 import { FONT_FAMILIES_TO_INSTALL } from '../sidebar/global-styles/font-pairing-variations/constants';
 import { OptInContext, OPTIN_FLOW_STATUS } from './context';
 
+type FontFamilies = {
+	custom: Array< FontFamily >;
+	theme: Array< FontFamily >;
+};
+
 const { useGlobalSetting } = unlock( blockEditorPrivateApis );
 
-export const OptInSubscribe = () => {
-	const { setOptInFlowStatus } = useContext( OptInContext );
+async function installPatterns() {
+	await apiFetch( {
+		path: '/wc-admin/patterns',
+		method: 'POST',
+	} );
 
-	const [ enabledFontFamilies, setFontFamilies ]: [
-		{
-			custom: Array< FontFamily >;
-			theme: Array< FontFamily >;
-		},
-		( font: {
-			custom: Array< FontFamily >;
-			theme: Array< FontFamily >;
-		} ) => void
-	] = useGlobalSetting( 'typography.fontFamilies' );
+	// @ts-expect-error -- No types for this exist yet.
+	await dispatch( coreStore ).invalidateResolutionForStoreSelector(
+		'getBlockPatterns'
+	);
+}
 
-	const {
+async function installFonts(
+	enabledFontFamilies: FontFamilies
+): Promise< FontFamilies > {
+	await installFontFamilies();
+
+	const globalStylesId =
 		// @ts-expect-error No types for this exist yet.
-		__experimentalSaveSpecifiedEntityEdits: saveSpecifiedEntityEdits,
-	} = useDispatch( coreStore );
-
-	const installPatterns = async () => {
-		await apiFetch< {
-			success: boolean;
-		} >( {
-			path: '/wc-admin/patterns',
-			method: 'POST',
-		} );
-	};
+		select( coreStore ).__experimentalGetCurrentGlobalStylesId();
 
-	const installFonts = async () => {
-		await installFontFamilies();
-
-		const globalStylesId =
-			// @ts-expect-error No types for this exist yet.
-			select( coreStore ).__experimentalGetCurrentGlobalStylesId();
-
-		const installedFontFamilies = ( await resolveSelect(
-			coreStore
-			// @ts-expect-error No types for this exist yet.
-		).getEntityRecords( 'postType', 'wp_font_family', {
-			_embed: true,
-			per_page: -1,
-		} ) ) as Array< {
-			id: number;
-			font_family_settings: FontFamily;
-			_embedded: {
-				font_faces: Array< {
-					font_face_settings: FontFace;
-				} >;
+	const installedFontFamilies = ( await resolveSelect(
+		coreStore
+		// @ts-expect-error No types for this exist yet.
+	).getEntityRecords( 'postType', 'wp_font_family', {
+		_embed: true,
+		per_page: -1,
+	} ) ) as Array< {
+		id: number;
+		font_family_settings: FontFamily;
+		_embedded: {
+			font_faces: Array< {
+				font_face_settings: FontFace;
+			} >;
+		};
+	} >;
+
+	const parsedInstalledFontFamilies = ( installedFontFamilies || [] ).map(
+		( fontFamilyPost ) => {
+			return {
+				id: fontFamilyPost.id,
+				...fontFamilyPost.font_family_settings,
+				fontFace:
+					fontFamilyPost?._embedded?.font_faces.map(
+						( face ) => face.font_face_settings
+					) || [],
 			};
-		} >;
-
-		const parsedInstalledFontFamilies = ( installedFontFamilies || [] ).map(
-			( fontFamilyPost ) => {
-				return {
-					id: fontFamilyPost.id,
-					...fontFamilyPost.font_family_settings,
-					fontFace:
-						fontFamilyPost?._embedded?.font_faces.map(
-							( face ) => face.font_face_settings
-						) || [],
-				};
+		}
+	);
+
+	const { custom } = enabledFontFamilies;
+
+	const enabledFontSlugs = [
+		...( custom ? custom.map( ( font ) => font.slug ) : [] ),
+	];
+
+	const fontFamiliesToEnable = parsedInstalledFontFamilies.reduce(
+		( acc, font ) => {
+			if (
+				enabledFontSlugs.includes( font.slug ) ||
+				FONT_FAMILIES_TO_INSTALL[ font.slug ] === undefined
+			) {
+				return acc;
 			}
-		);
 
-		const { custom } = enabledFontFamilies;
-
-		const enabledFontSlugs = [
-			...( custom ? custom.map( ( font ) => font.slug ) : [] ),
-		];
-
-		const fontFamiliesToEnable = parsedInstalledFontFamilies.reduce(
-			( acc, font ) => {
-				if (
-					enabledFontSlugs.includes( font.slug ) ||
-					FONT_FAMILIES_TO_INSTALL[ font.slug ] === undefined
-				) {
-					return acc;
-				}
-
-				return [
-					...acc,
-					{
-						...font,
-					},
-				];
-			},
-			[] as Array< FontFamily >
-		);
+			return [ ...acc, { ...font } ];
+		},
+		[] as Array< FontFamily >
+	);
 
-		setFontFamilies( {
-			...enabledFontFamilies,
-			custom: [
-				...( enabledFontFamilies.custom ?? [] ),
-				...( fontFamiliesToEnable ?? [] ),
-			],
-		} );
-
-		saveSpecifiedEntityEdits( 'root', 'globalStyles', globalStylesId, [
-			'settings.typography.fontFamilies',
-		] );
+	const {
+		// @ts-expect-error No types for this exist yet.
+		__experimentalSaveSpecifiedEntityEdits: saveSpecifiedEntityEdits,
+	} = dispatch( coreStore );
+
+	saveSpecifiedEntityEdits( 'root', 'globalStyles', globalStylesId, [
+		'settings.typography.fontFamilies',
+	] );
+
+	return {
+		...enabledFontFamilies,
+		custom: [
+			...( enabledFontFamilies.custom ?? [] ),
+			...( fontFamiliesToEnable ?? [] ),
+		],
 	};
+}
 
-	const { invalidateCache } = usePatterns();
+export const OptInSubscribe = () => {
+	const { setOptInFlowStatus } = useContext( OptInContext );
 
-	useEffect( () => {
-		const unsubscribe = subscribe( async () => {
-			const isOptedIn =
-				select( OPTIONS_STORE_NAME ).getOption(
-					'woocommerce_allow_tracking'
-				) === 'yes';
+	const [ enabledFontFamilies, setFontFamilies ]: [
+		FontFamilies,
+		( font: FontFamilies ) => void
+	] = useGlobalSetting( 'typography.fontFamilies' );
 
-			if ( isOptedIn ) {
-				setOptInFlowStatus( OPTIN_FLOW_STATUS.LOADING );
-				await installPatterns();
-				invalidateCache();
-				await installFonts();
+	const isOptedIn = useSelect( ( selectStore ) => {
+		const allowTracking = selectStore( OPTIONS_STORE_NAME ).getOption(
+			'woocommerce_allow_tracking'
+		);
+		return allowTracking === 'yes';
+	}, [] );
 
-				setOptInFlowStatus( OPTIN_FLOW_STATUS.DONE );
+	useEffect(
+		function optedInListener() {
+			if ( ! isOptedIn ) return;
 
-				unsubscribe();
+			async function installPatternsAndFonts() {
+				await installPatterns();
+				const fontFamilies = await installFonts( enabledFontFamilies );
+				setFontFamilies( fontFamilies );
 			}
-			// @ts-expect-error The type is not updated.
-		}, OPTIONS_STORE_NAME );
 
-		return () => {
-			unsubscribe();
-		};
-		// We don't want to run this effect on every render, only once because it is a subscription.
+			setOptInFlowStatus( OPTIN_FLOW_STATUS.LOADING );
+			installPatternsAndFonts().finally( () => {
+				setOptInFlowStatus( OPTIN_FLOW_STATUS.DONE );
+			} );
+		},
+		// We don't want to run this effect on every render, only when `woocommerce_allow_tracking` changes.
 		// eslint-disable-next-line react-hooks/exhaustive-deps
-	}, [] );
+		[ isOptedIn ]
+	);
 
 	return null;
 };
diff --git a/plugins/woocommerce/changelog/fix-50907 b/plugins/woocommerce/changelog/fix-50907
@@ -0,0 +1,4 @@
+Significance: minor
+Type: fix
+
+Remove the subscription to the OPTIONS_STORE_NAME since its trigered many times, using useSelect instead