Merge pull request #9978 from google/enhancement/9008-remove-waitfor-…

…actions
google · Jan 17, 2025 · 1e6163b · 1e6163b
2 parents 390cbd5 + 7b78c6f
commit 1e6163b
Show file tree

Hide file tree

Showing 16 changed files with 193 additions and 266 deletions.
diff --git a/assets/js/googlesitekit/data/create-existing-tag-store.js b/assets/js/googlesitekit/data/create-existing-tag-store.js
@@ -35,7 +35,6 @@ import { getExistingTagURLs, extractExistingTag } from '../../util/tag';
 // Actions
 const FETCH_GET_EXISTING_TAG = 'FETCH_GET_EXISTING_TAG';
 const RECEIVE_GET_EXISTING_TAG = 'RECEIVE_GET_EXISTING_TAG';
-const WAIT_FOR_EXISTING_TAG = 'WAIT_FOR_EXISTING_TAG';
 
 /**
  * Creates a store object that includes actions and selectors for getting existing tags.
@@ -89,12 +88,6 @@ export const createExistingTagStore = ( {
 				type: RECEIVE_GET_EXISTING_TAG,
 			};
 		},
-		*waitForExistingTag() {
-			yield {
-				payload: {},
-				type: WAIT_FOR_EXISTING_TAG,
-			};
-		},
 	};
 
 	const controls = {
@@ -107,13 +100,10 @@ export const createExistingTagStore = ( {
 					ampMode,
 				} );
 
+				const { getHTMLForURL } = registry.resolveSelect( CORE_SITE );
+
 				for ( const url of existingTagURLs ) {
-					await registry
-						.dispatch( CORE_SITE )
-						.waitForHTMLForURL( url );
-					const html = registry
-						.select( CORE_SITE )
-						.getHTMLForURL( url );
+					const html = await getHTMLForURL( url );
 					const tagFound = extractExistingTag( html, tagMatchers );
 					if ( tagFound ) {
 						return tagFound;
@@ -123,25 +113,6 @@ export const createExistingTagStore = ( {
 				return null;
 			}
 		),
-		[ WAIT_FOR_EXISTING_TAG ]: createRegistryControl(
-			( registry ) => () => {
-				const isExistingTagLoaded = () =>
-					registry.select( STORE_NAME ).getExistingTag() !==
-					undefined;
-				if ( isExistingTagLoaded() ) {
-					return true;
-				}
-
-				return new Promise( ( resolve ) => {
-					const unsubscribe = registry.subscribe( () => {
-						if ( isExistingTagLoaded() ) {
-							unsubscribe();
-							resolve();
-						}
-					} );
-				} );
-			}
-		),
 	};
 
 	const reducer = ( state = initialState, { type, payload } ) => {

diff --git a/assets/js/googlesitekit/data/create-existing-tag-store.test.js b/assets/js/googlesitekit/data/create-existing-tag-store.test.js
@@ -118,30 +118,6 @@ describe( 'createExistingTagStore store', () => {
 				expect( store.getState().existingTag ).toBe( existingTag );
 			} );
 		} );
-
-		describe( 'waitForExistingTag', () => {
-			it( 'supports asynchronous waiting for tag', async () => {
-				const expectedTag = 'test-tag-value';
-				fetchMock.getOnce(
-					{ query: { tagverify: '1' } },
-					{ body: generateHTMLWithTag( expectedTag ), status: 200 }
-				);
-
-				const promise = registry
-					.dispatch( TEST_STORE )
-					.waitForExistingTag();
-				expect( registry.select( TEST_STORE ).getExistingTag() ).toBe(
-					undefined
-				);
-
-				await promise;
-
-				expect( fetchMock ).toHaveFetchedTimes( 1 );
-				expect( registry.select( TEST_STORE ).getExistingTag() ).toBe(
-					expectedTag
-				);
-			} );
-		} );
 	} );
 
 	describe( 'selectors', () => {

diff --git a/assets/js/googlesitekit/datastore/site/html.js b/assets/js/googlesitekit/datastore/site/html.js
@@ -129,22 +129,6 @@ const baseActions = {
 			type: CHECK_FOR_SETUP_TAG,
 		};
 	},
-
-	/**
-	 * Waits for HTML for to be resolved for the given URL.
-	 *
-	 * @since 1.13.0
-	 * @private
-	 *
-	 * @param {string} url URL for which to fetch HTML.
-	 * @yield {Object} Redux-style action.
-	 */
-	*waitForHTMLForURL( url ) {
-		yield {
-			payload: { url },
-			type: WAIT_FOR_HTML_FOR_URL,
-		};
-	},
 };
 
 const baseControls = {

diff --git a/assets/js/googlesitekit/datastore/site/html.test.js b/assets/js/googlesitekit/datastore/site/html.test.js
@@ -93,34 +93,6 @@ describe( 'core/site html', () => {
 				).toBe( html );
 			} );
 		} );
-
-		describe( 'waitForHTMLForURL', () => {
-			it( 'supports asynchronous waiting for HTML', async () => {
-				const url = 'https://example.com';
-				const html =
-					'<html><head><title>Example HTML</title></head><body><h1>Example HTML H1</h1></body></html>';
-
-				fetchMock.getOnce(
-					{ query: { tagverify: '1' } },
-					{ body: html, status: 200 }
-				);
-				const promise = registry
-					.dispatch( CORE_SITE )
-					.waitForHTMLForURL( url );
-
-				expect(
-					registry.select( CORE_SITE ).getHTMLForURL( url )
-				).toBe( undefined );
-
-				await promise;
-
-				expect( fetchMock ).toHaveFetchedTimes( 1 );
-
-				expect(
-					registry.select( CORE_SITE ).getHTMLForURL( url )
-				).toBe( html );
-			} );
-		} );
 	} );
 
 	describe( 'selectors', () => {

diff --git a/assets/js/modules/adsense/datastore/ad-blocking-recovery.js b/assets/js/modules/adsense/datastore/ad-blocking-recovery.js
@@ -105,9 +105,10 @@ const controls = {
 				homeURL,
 			} );
 
+			const { getHTMLForURL } = registry.resolveSelect( CORE_SITE );
+
 			for ( const url of existingTagURLs ) {
-				await registry.dispatch( CORE_SITE ).waitForHTMLForURL( url );
-				const html = registry.select( CORE_SITE ).getHTMLForURL( url );
+				const html = await getHTMLForURL( url );
 				const tagFound = extractExistingTag(
 					html,
 					adBlockingRecoveryTagMatcher

diff --git a/.../modules/analytics-4/components/settings/PropertyOrWebDataStreamNotAvailableError.test.js b/.../modules/analytics-4/components/settings/PropertyOrWebDataStreamNotAvailableError.test.js
@@ -62,11 +62,22 @@ describe( 'PropertyOrWebDataStreamNotAvailableError', () => {
 			.setMeasurementID( measurementID );
 	} );
 
-	it( 'should not render when properties are not loaded yet', () => {
+	it( 'should not render when properties are not loaded yet', async () => {
+		/*
+		 * We need to freeze the `GET:account-summaries` request twice here, as it will be called a
+		 * second time while resolvers are run post-render. This is due to the initial endpoint
+		 * response being frozen, meaning the `getAccountSummaries()` resolver is unable to resolve
+		 * itself the first time around, and will be called again when `getAccountSummaries()` is
+		 * re-selected by another resolver.
+		 *
+		 * In practice, this is not an issue because the `PropertyOrWebDataStreamNotAvailableError`
+		 * component is not rendered until the account summaries are already loaded.
+		 */
 		freezeFetch(
 			new RegExp(
 				'^/google-site-kit/v1/modules/analytics-4/data/account-summaries'
-			)
+			),
+			{ repeat: 2 }
 		);
 
 		registry
@@ -75,7 +86,7 @@ describe( 'PropertyOrWebDataStreamNotAvailableError', () => {
 				propertyIDs: [ propertyID ],
 			} );
 
-		const { container } = render(
+		const { container, waitForRegistry } = render(
 			<PropertyOrWebDataStreamNotAvailableError
 				hasModuleAccess
 				isDisabled={ false }
@@ -84,6 +95,8 @@ describe( 'PropertyOrWebDataStreamNotAvailableError', () => {
 		);
 
 		expect( container ).toBeEmptyDOMElement();
+
+		await waitForRegistry();
 	} );
 
 	it( 'should not render when Web Data Streams are not loaded yet', async () => {

diff --git a/assets/js/modules/analytics-4/components/setup/SetupForm.test.js b/assets/js/modules/analytics-4/components/setup/SetupForm.test.js
@@ -513,7 +513,8 @@ describe( 'SetupForm', () => {
 				triggerChange( getByRole );
 			} );
 
-			await waitForRegistry();
+			// An additional wait is required in order for all resolvers to finish.
+			await act( waitForDefaultTimeouts );
 		} );
 
 		it( 'should revert the enhanced measurement from off to on', () => {

diff --git a/assets/js/modules/analytics-4/datastore/properties.js b/assets/js/modules/analytics-4/datastore/properties.js
@@ -31,11 +31,7 @@ import { createRegistrySelector } from '@wordpress/data';
  * Internal dependencies
  */
 import API from 'googlesitekit-api';
-import {
-	commonActions,
-	combineStores,
-	createRegistryControl,
-} from 'googlesitekit-data';
+import { commonActions, combineStores } from 'googlesitekit-data';
 import { CORE_USER } from '../../../googlesitekit/datastore/user/constants';
 import { CORE_SITE } from '../../../googlesitekit/datastore/site/constants';
 import { CORE_MODULES } from '../../../googlesitekit/modules/datastore/constants';
@@ -53,7 +49,6 @@ import {
 	isValidPropertyID,
 	isValidPropertySelection,
 } from '../utils/validation';
-import { actions as webDataStreamActions } from './webdatastreams';
 import { createValidatedAction } from '../../../googlesitekit/data/utils';
 import { getItem, setItem } from '../../../googlesitekit/api/cache';
 
@@ -202,7 +197,6 @@ const fetchSetGoogleTagIDMismatch = createFetchStore( {
 } );
 
 // Actions
-const WAIT_FOR_PROPERTY_SUMMARIES = 'WAIT_FOR_PROPERTY_SUMMARIES';
 const MATCHING_ACCOUNT_PROPERTY = 'MATCHING_ACCOUNT_PROPERTY';
 const SET_HAS_MISMATCHED_TAG = 'SET_HAS_MISMATCHED_GOOGLE_TAG_ID';
 const SET_IS_WEBDATASTREAM_AVAILABLE = 'SET_IS_WEBDATASTREAM_AVAILABLE';
@@ -294,11 +288,11 @@ const baseActions = {
 				}
 			}
 
-			yield webDataStreamActions.waitForWebDataStreams( propertyID );
-
-			let webdatastream = registry
-				.select( MODULES_ANALYTICS_4 )
-				.getMatchingWebDataStreamByPropertyID( propertyID );
+			let webdatastream = yield commonActions.await(
+				registry
+					.resolveSelect( MODULES_ANALYTICS_4 )
+					.getMatchingWebDataStreamByPropertyID( propertyID )
+			);
 
 			if ( ! webdatastream ) {
 				const webdatastreams = registry
@@ -367,12 +361,12 @@ const baseActions = {
 	*matchAccountProperty( accountID ) {
 		const registry = yield commonActions.getRegistry();
 
-		yield baseActions.waitForPropertySummaries( accountID );
-
 		const referenceURL = registry.select( CORE_SITE ).getReferenceSiteURL();
-		const propertySummaries = registry
-			.select( MODULES_ANALYTICS_4 )
-			.getPropertySummaries( accountID );
+		const propertySummaries = yield commonActions.await(
+			registry
+				.resolveSelect( MODULES_ANALYTICS_4 )
+				.getPropertySummaries( accountID )
+		);
 
 		const property = yield baseActions.matchPropertyByURL(
 			( propertySummaries || [] ).map( ( { _id } ) => _id ),
@@ -516,18 +510,6 @@ const baseActions = {
 		return null;
 	},
 
-	/**
-	 * Waits for property summaries to be loaded for an account.
-	 *
-	 * @since 1.118.0
-	 */
-	*waitForPropertySummaries() {
-		yield {
-			payload: {},
-			type: WAIT_FOR_PROPERTY_SUMMARIES,
-		};
-	},
-
 	/**
 	 * Updates settings for a given measurement ID.
 	 *
@@ -731,17 +713,7 @@ const baseActions = {
 	},
 };
 
-const baseControls = {
-	[ WAIT_FOR_PROPERTY_SUMMARIES ]: createRegistryControl(
-		( { resolveSelect } ) => {
-			return async () => {
-				await resolveSelect(
-					MODULES_ANALYTICS_4
-				).getAccountSummaries();
-			};
-		}
-	),
-};
+const baseControls = {};
 
 function baseReducer( state, { type, payload } ) {
 	switch ( type ) {
@@ -788,6 +760,15 @@ const baseResolvers = {
 			yield fetchGetPropertyStore.actions.fetchGetProperty( propertyID );
 		}
 	},
+	*getPropertySummaries( accountID ) {
+		const { resolveSelect } = yield commonActions.getRegistry();
+
+		yield commonActions.await(
+			resolveSelect( MODULES_ANALYTICS_4 ).getAccountSummaries(
+				accountID
+			)
+		);
+	},
 	*getPropertyCreateTime() {
 		const registry = yield commonActions.getRegistry();
 		// Ensure settings are available to select.

diff --git a/assets/js/modules/analytics-4/datastore/properties.test.js b/assets/js/modules/analytics-4/datastore/properties.test.js
@@ -1372,6 +1372,36 @@ describe( 'modules/analytics-4 properties', () => {
 		} );
 
 		describe( 'getPropertySummaries', () => {
+			it( 'should use a resolver to make a network request', async () => {
+				const accountSummariesEndpoint = new RegExp(
+					'^/google-site-kit/v1/modules/analytics-4/data/account-summaries'
+				);
+
+				fetchMock.get( accountSummariesEndpoint, {
+					body: fixtures.accountSummaries,
+					status: 200,
+				} );
+
+				const accountID = '12345';
+				const initialPropertySummaries = registry
+					.select( MODULES_ANALYTICS_4 )
+					.getPropertySummaries( accountID );
+				expect( initialPropertySummaries ).toBeUndefined();
+
+				await untilResolved(
+					registry,
+					MODULES_ANALYTICS_4
+				).getPropertySummaries( accountID );
+
+				expect( fetchMock ).toHaveFetched( accountSummariesEndpoint, {
+					body: {
+						data: {
+							nextPageToken: '',
+						},
+					},
+				} );
+			} );
+
 			it( 'should return an empty array if no properties are present for the account', () => {
 				registry
 					.dispatch( MODULES_ANALYTICS_4 )

diff --git a/assets/js/modules/analytics-4/datastore/settings.js b/assets/js/modules/analytics-4/datastore/settings.js
@@ -80,7 +80,7 @@ const store = {
 };
 export default store;
 
-export async function submitChanges( { select, dispatch } ) {
+export async function submitChanges( { dispatch, select, resolveSelect } ) {
 	let propertyID = select( MODULES_ANALYTICS_4 ).getPropertyID();
 	if ( propertyID === PROPERTY_CREATE ) {
 		const accountID = select( MODULES_ANALYTICS_4 ).getAccountID();
@@ -112,11 +112,7 @@ export async function submitChanges( { select, dispatch } ) {
 		let webDataStreamAlreadyExists = false;
 
 		if ( isValidPropertyID( propertyID ) ) {
-			await dispatch( MODULES_ANALYTICS_4 ).waitForWebDataStreams(
-				propertyID
-			);
-
-			webDataStreamAlreadyExists = select(
+			webDataStreamAlreadyExists = await resolveSelect(
 				MODULES_ANALYTICS_4
 			).doesWebDataStreamExist( propertyID, webDataStreamName );
 		}