Update packages with React 17 and tests passing.

.github/workflows/visual-regression.yml

@@ -46,7 +46,7 @@ jobs:
       - name: npm install
         run: npm ci -w assets -w tests/backstop -w storybook -w tests/js --include-workspace-root
       - name: Log in to GitHub Container Registry
-        uses: docker/login-action@v3
+        uses: docker/login-action@v4
         with:
           registry: ghcr.io
           username: google-site-kit-bot

.stylelintrc

@@ -1,34 +1,35 @@
 {
-  "extends": [
-    "@wordpress/stylelint-config",
-    "stylelint-config-standard-scss"
-  ],
-  "plugins": [
-    "stylelint-declaration-strict-value",
-    "stylelint-order",
-    "stylelint-scss"
-  ],
-  "rules": {
-    "indentation": "tab",
-    "declaration-empty-line-before": null,
-    "scss/no-global-function-names": null,
-    "scss/dollar-variable-empty-line-before": null,
-    "scss/at-extend-no-missing-placeholder": null,
-    "scss/dollar-variable-pattern": null,
-    "color-function-notation": null,
-    "alpha-value-notation": null,
-    "rule-empty-line-before": null,
-    "at-rule-empty-line-before": null,
-    "string-quotes": "double",
-    "at-rule-no-unknown": null,
-    "declaration-property-unit-allowed-list": null,
-    "function-url-quotes": "always",
-    "selector-class-pattern": null,
-    "order/properties-alphabetical-order": true,
-    "value-keyword-case": [ "lower", {
-        "ignoreKeywords": ["/([A-Z])/"]
-      }
-    ],
-    "selector-nested-pattern": "^(?!&__|&--|&-|&_).*"
-  }
-}
+	"extends": [
+		"@wordpress/stylelint-config",
+		"stylelint-config-standard-scss"
+	],
+	"plugins": [
+		"stylelint-declaration-strict-value",
+		"stylelint-order",
+		"stylelint-scss"
+	],
+	"rules": {
+		"shorthand-property-no-redundant-values": null,
+		"declaration-empty-line-before": null,
+		"scss/no-global-function-names": null,
+		"scss/dollar-variable-empty-line-before": null,
+		"scss/at-extend-no-missing-placeholder": null,
+		"scss/dollar-variable-pattern": null,
+		"color-function-notation": null,
+		"alpha-value-notation": null,
+		"rule-empty-line-before": null,
+		"at-rule-empty-line-before": null,
+		"at-rule-no-unknown": null,
+		"declaration-property-unit-allowed-list": null,
+		"function-url-quotes": "always",
+		"selector-class-pattern": null,
+		"order/properties-alphabetical-order": true,
+		"value-keyword-case": [
+			"lower",
+			{
+				"ignoreKeywords": [ "/([A-Z])/" ]
+			}
+		],
+		"selector-nested-pattern": "^(?!&__|&--|&-|&_).*"
+	}
+}

assets/js/components/ErrorHandler/index.js

@@ -60,14 +60,26 @@ class ErrorHandler extends Component {
 	}
 
 	render() {
-		const { children } = this.props;
+		const { children, hideFilenamesInStack } = this.props;
 		const { error, info } = this.state;
 
 		// If there is no caught error, render the children components normally.
 		if ( ! error ) {
 			return children;
 		}
 
+		let componentStack = info?.componentStack;
+
+		// Hide filenames in stack trace to prevent issues with Visual Regression
+		// tests and hashes changing in the stack trace.
+		if ( hideFilenamesInStack && componentStack ) {
+			componentStack = componentStack
+				.replace( /\(.*\)/g, '(filtered)' )
+				.replace( /at (.+) ?https?:\/\/.*/g, 'at $1 (filtered)' )
+				.replace( /at https?:\/\/.*/g, '' )
+				.replace( /\n    \n    /g, '\n    ' );
+		}
+
 		return (
 			<BannerNotification
 				notificationID="googlesitekit-error"
@@ -81,15 +93,15 @@ class ErrorHandler extends Component {
 					<Fragment>
 						<GenericErrorHandlerActions
 							message={ error.message }
-							componentStack={ info.componentStack }
+							componentStack={ componentStack }
 						/>
 						<pre className="googlesitekit-overflow-auto">
 							{ error.message }
-							{ info.componentStack }
+							{ componentStack }
 						</pre>
 					</Fragment>
 				}
-			></BannerNotification>
+			/>
 		);
 	}
 }

assets/js/components/ErrorHandler/index.stories.js

@@ -24,7 +24,7 @@ import ErrorHandler from './';
 
 function Template() {
 	return (
-		<ErrorHandler>
+		<ErrorHandler hideFilenamesInStack>
 			<ThrowErrorComponent throwErrorOnMount />
 		</ErrorHandler>
 	);

assets/js/components/ModalDialog.js

@@ -132,7 +132,6 @@ function ModalDialog( {
 				<Button
 					className="mdc-dialog__cancel-button"
 					onClick={ handleCancel }
-					disabled={ inProgress }
 					tertiary
 				>
 					{ __( 'Cancel', 'google-site-kit' ) }

assets/js/components/TourTooltips.test.js

@@ -23,6 +23,7 @@ import {
 	render,
 	createTestRegistry,
 	fireEvent,
+	act,
 } from '../../../tests/js/test-utils';
 import TourTooltips, { GA_ACTIONS } from './TourTooltips';
 import { CORE_UI } from '@/js/googlesitekit/datastore/ui/constants';
@@ -227,11 +228,21 @@ describe( 'TourTooltips', () => {
 		);
 	} );
 
-	it( 'should end tour when close icon is clicked', () => {
-		const { getByRole, queryByRole } =
+	it( 'should end tour when close icon is clicked', async () => {
+		const { getByRole, queryByRole, rerender } =
 			renderTourTooltipsWithMockUI( registry );
 
-		fireEvent.click( getByRole( 'button', { name: /close/i } ) );
+		await act( () => {
+			fireEvent.click( getByRole( 'button', { name: /close/i } ) );
+		} );
+
+		rerender();
+
+		// This error is caused by the `react-select` component trying to update
+		// state after the component has unmounted, so we can't fix it ourselves.
+		expect( console ).toHaveErrored(
+			"Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in %s.%s"
+		);
 
 		expect( queryByRole( 'alertdialog' ) ).not.toBeInTheDocument();
 		expect( dismissTourSpy ).toHaveBeenCalled();

assets/js/components/WelcomeModal.tsx

@@ -80,25 +80,34 @@ export default function WelcomeModal() {
 
 	const [ isOpen, setIsOpen ] = useState( true );
 
-	const analyticsConnected = useSelect( ( select: Select ) =>
-		select( CORE_MODULES ).isModuleConnected( MODULE_SLUG_ANALYTICS_4 )
+	const analyticsConnected = useSelect(
+		( select: Select ) =>
+			select( CORE_MODULES ).isModuleConnected( MODULE_SLUG_ANALYTICS_4 ),
+		[]
 	);
 
-	const analyticsGatheringData = useSelect( ( select: Select ) => {
-		if ( ! analyticsConnected ) {
-			return false;
-		}
-		return select( MODULES_ANALYTICS_4 ).isGatheringData();
-	} );
+	const analyticsGatheringData = useSelect(
+		( select: Select ) => {
+			if ( ! analyticsConnected ) {
+				return false;
+			}
+			return select( MODULES_ANALYTICS_4 ).isGatheringData();
+		},
+		[ analyticsConnected ]
+	);
 
-	const searchConsoleGatheringData = useSelect( ( select: Select ) =>
-		select( MODULES_SEARCH_CONSOLE ).isGatheringData()
+	const searchConsoleGatheringData = useSelect(
+		( select: Select ) =>
+			select( MODULES_SEARCH_CONSOLE ).isGatheringData(),
+		[]
 	);
 
-	const isGatheringDataVariantDismissed = useSelect( ( select: Select ) =>
-		select( CORE_USER ).isItemDismissed(
-			WELCOME_GATHERING_DATA_DISMISSED_ITEM_SLUG
-		)
+	const isGatheringDataVariantDismissed = useSelect(
+		( select: Select ) =>
+			select( CORE_USER ).isItemDismissed(
+				WELCOME_GATHERING_DATA_DISMISSED_ITEM_SLUG
+			),
+		[]
 	);
 
 	const showGatheringDataModal = analyticsConnected
@@ -224,8 +233,10 @@ export default function WelcomeModal() {
 		);
 	}, [ viewContext, modalVariant ] );
 
-	const isDataGatheringCompleteModalActive = useSelect( ( select: Select ) =>
-		select( CORE_USER ).isDataGatheringCompleteModalActive()
+	const isDataGatheringCompleteModalActive = useSelect(
+		( select: Select ) =>
+			select( CORE_USER ).isDataGatheringCompleteModalActive(),
+		[]
 	);
 
 	if (

assets/js/components/dashboard-sharing/UserRoleSelect.test.js

@@ -29,7 +29,9 @@ import {
 	render,
 	fireEvent,
 	createTestRegistry,
+	act,
 } from '../../../../tests/js/test-utils';
+import { waitForTimeouts } from '../../../../tests/js/utils';
 import UserRoleSelect from './UserRoleSelect';
 import { MODULE_SLUG_SEARCH_CONSOLE } from '@/js/modules/search-console/constants';
 
@@ -224,9 +226,14 @@ describe( 'CurrentSurvey', () => {
 
 		await waitForRegistry();
 
-		fireEvent.click(
-			container.querySelector( '.googlesitekit-user-role-select__button' )
-		);
+		await act( async () => {
+			fireEvent.click(
+				container.querySelector(
+					'.googlesitekit-user-role-select__button'
+				)
+			);
+			await waitForTimeouts( 100 );
+		} );
 
 		expect(
 			container.querySelector(

assets/js/components/email-reporting/notices/EmailReportingErrorNotices.test.js

@@ -28,6 +28,8 @@ import {
 	provideModules,
 } from '../../../../../tests/js/test-utils';
 import { CORE_SITE } from '@/js/googlesitekit/datastore/site/constants';
+import { MODULES_ANALYTICS_4 } from '@/js/modules/analytics-4/datastore/constants';
+import { defaultSettings as analytics4DefaultSettings } from '@/js/modules/analytics-4/datastore/__fixtures__';
 import * as tracking from '@/js/util/tracking';
 import { VIEW_CONTEXT_MAIN_DASHBOARD_VIEW_ONLY } from '@/js/googlesitekit/constants';
 
@@ -76,6 +78,9 @@ describe( 'EmailReportingErrorNotices', () => {
 
 	it( 'should render the permissions error notice when email reporting is enabled, user is not view-only, and there is a permissions_error category ID and a module slug', () => {
 		provideModules( registry );
+		registry
+			.dispatch( MODULES_ANALYTICS_4 )
+			.receiveGetSettings( analytics4DefaultSettings );
 		registry.dispatch( CORE_SITE ).receiveGetEmailReportingSettings( {
 			enabled: true,
 		} );

assets/js/components/notifications/ActivateAnalyticsNotification.tsx

@@ -65,8 +65,10 @@ const ActivateAnalyticsNotification: FC<
 
 	const showTooltip = useShowTooltip( tooltipSettings );
 
-	const isDismissalFinal = useSelect( ( select: Select ) =>
-		select( CORE_NOTIFICATIONS ).isNotificationDismissalFinal( id )
+	const isDismissalFinal = useSelect(
+		( select: Select ) =>
+			select( CORE_NOTIFICATIONS ).isNotificationDismissalFinal( id ),
+		[]
 	);
 
 	const dismissLabel = useRetriableNotificationDismissButtonLabel( {
@@ -82,13 +84,15 @@ const ActivateAnalyticsNotification: FC<
 		}
 
 		return select( CORE_LOCATION ).isNavigatingTo( adminReauthURL );
-	} );
-
-	const isActivatingAnalytics = useSelect( ( select: Select ) =>
-		select( CORE_MODULES ).isFetchingSetModuleActivation(
-			MODULE_SLUG_ANALYTICS_4,
-			true
-		)
+	}, [] );
+
+	const isActivatingAnalytics = useSelect(
+		( select: Select ) =>
+			select( CORE_MODULES ).isFetchingSetModuleActivation(
+				MODULE_SLUG_ANALYTICS_4,
+				true
+			),
+		[]
 	);
 
 	const activateAnalytics = useActivateModuleCallback(

assets/js/components/notifications/ConnectMoreServicesNotification.tsx

@@ -58,12 +58,15 @@ const ConnectMoreServicesNotification: FC<
 	const showTooltip = useShowTooltip( tooltipSettings );
 	const { navigateTo } = useDispatch( CORE_LOCATION );
 
-	const connectMoreServicesURL = useSelect( ( select: Select ) =>
-		select( CORE_SITE ).getConnectMoreServicesURL()
+	const connectMoreServicesURL = useSelect(
+		( select: Select ) => select( CORE_SITE ).getConnectMoreServicesURL(),
+		[]
 	);
 
-	const isNavigatingToConnectMoreServices = useSelect( ( select: Select ) =>
-		select( CORE_LOCATION ).isNavigatingTo( connectMoreServicesURL )
+	const isNavigatingToConnectMoreServices = useSelect(
+		( select: Select ) =>
+			select( CORE_LOCATION ).isNavigatingTo( connectMoreServicesURL ),
+		[ connectMoreServicesURL ]
 	);
 
 	const viewContext = useViewContext();

assets/js/components/notifications/GoogleTagGatewaySetupBanner.test.js

@@ -23,6 +23,7 @@ import fetchMock from 'fetch-mock';
  */
 import GoogleTagGatewaySetupBanner from './GoogleTagGatewaySetupBanner';
 import {
+	act,
 	createTestRegistry,
 	fireEvent,
 	provideModules,
@@ -311,11 +312,13 @@ describe( 'GoogleTagGatewaySetupBanner', () => {
 				NOTIFICATION_GROUPS.DEFAULT,
 			] );
 
-		fireEvent.click(
-			getByRole( 'button', {
-				name: 'Enable Google tag gateway for advertisers',
-			} )
-		);
+		await act( () => {
+			fireEvent.click(
+				getByRole( 'button', {
+					name: 'Enable Google tag gateway for advertisers',
+				} )
+			);
+		} );
 
 		await waitForRegistry();
 
@@ -329,6 +332,8 @@ describe( 'GoogleTagGatewaySetupBanner', () => {
 			expect( fetchMock ).toHaveFetched( dismissItemEndpoint );
 		} );
 
+		await waitForRegistry();
+
 		expect(
 			registry
 				.select( CORE_NOTIFICATIONS )