Upgrade outdated packages; Bump React to v18 by thelovekesh · Pull Request #7548 · ampproject/amp-wp

thelovekesh · 2023-05-15T21:32:57Z

Summary

Previously we attempted to upgrade React in #7394 but due to React version inconsistencies on the upstream, we ended up blocking that PR.

This PR aims to upgrade React to v18 with the following enhancements:

Migrated JS(React) test cases to @testing-library/react.
Update the AfterActivationSiteScan service to work with React >= 18.
Migrated from deprecated registerStore in the @wordpress/data package.
Add jest resolver to fix failing tests due to the UUID package.
Updated tests container in wp-env commands.
Update workflow to run E2E tests with Node 16.

Checklist

My code is tested and passes existing tests.
My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

- Remove unsage of `render` from `@wordpress/element` - Use `act` from `@testing-library/react` instead of `react-dom/test-utils` - Update test cases for AMPDocumentStatusNotification component - Update test cases for AMPRevalidateNotification component - Update test cases for AMPValidationStatusNotification component - Update test cases for Icons component - Update test cases for SidebarNotification component - Update test cases for SidebarNotificationsContainer component - Update test cases for withAMPToolbarButton component - Update test cases for useAMPDocumentToggle hook - Update test cases for AMPToggle component - Update test cases for Nav component - Update test cases for ThemesContextProvider component - Update test cases for TemplateModeOption component - Update test cases for SiteScanSourcesList component - Update test cases for Selectable component - Update test cases for RedirectToggle component - Update test cases for ProgressBar component - Update test cases for PluginsContextProvider component - Update test cases for NavMenu component - Update test cases for Loading component - Update test cases for DevToolsToggle component - Update test cases for ConditionalDetails component - Update test cases for AMPNotice component - Update test cases for AMPSettingToggle component - Update test cases for AmpAdminNotice component - Update test cases for useValidationErrorStateUpdates hook - Update test cases for usePostDirtyStateChanges hook - Update test cases for Error component - Update test cases for CarouselNav component - Update test cases for useErrorsFetchingStateChanges hook

See: WordPress/gutenberg#47388 (comment)

…ecated

github-actions · 2023-05-23T15:21:15Z

Plugin builds for eeebbd9 are ready 🛎️!

Download development build
Download production build

thelovekesh · 2023-05-23T17:40:39Z

src/Admin/AfterActivationSiteScan.php

 			&&
 			! is_network_admin()
 			&&
+			( $react instanceof _WP_Dependency && version_compare( $react->ver, '18', '>=' ) )


@westonruter Please note the AfterActivationSiteScan service will only work with React 18 and will not show the Site Scan Notice on plugin/themes activation if WP < 6.2.

Unless the latest version of the Gutenberg plugin is active, right?

Yes. The latest Gutenberg plugin uses React 18, so if it's activated then it will work on WP < 6.2.

westonruter

What an epic lift! Great work. Just a couple small comments, and aside from the failing tests, looks really good. (Granted, I'm not an expert on React!)

westonruter · 2023-05-23T20:11:14Z

assets/src/block-validation/components/error/test/error.js

+			// container
+			// 	.querySelector(
+			// 		`.amp-error--${getErrorTypeClassName(status)} button`
+			// 	)
+			// 	.click();


Why is this commented-out code here?

This code has been migrated to use the click event from @testing-library/react and I missed removing it. Will remove it 👍🏼

westonruter · 2023-05-23T20:17:19Z

.../src/block-validation/components/with-amp-toolbar-button/test/add-toolbar-button-to-block.js

-		createStore({
-			reviewLink: 'http://review-link.test',
-			unreviewedValidationErrors: [
-				{
-					clientId: block.clientId,
-					code: 'DISALLOWED_TAG',
-					status: 3,
-					term_id: 12,
-					title: 'Invalid script: <code>jquery.js</code>',
-					type: 'js_error',
-				},
-			],
-			validationErrors: [
-				{
-					clientId: block.clientId,
-					code: 'DISALLOWED_TAG',
-					status: 3,
-					term_id: 12,
-					title: 'Invalid script: <code>jquery.js</code>',
-					type: 'js_error',
-				},
-			],
-		});
-	});


I recall we chatted about this a bit, but I see why the change of status to 1? Why are unreviewedValidationErrors and validationErrors seemingly merged?

Previously we were using registerStore to create redux stores and then initializing them at required places. With this previously, we were able to initialize it with the initial state of our choice.

With the current implementation, we are initializing our store itself where it has been defined so there will be a single instance of store everywhere. We have combined both of them in one because it's the job of the reducer to decide which validation error will be assigned to unreviewed or reviewed as per

amp-wp/assets/src/block-validation/store/index.js

Lines 82 to 123 in 9f7014a

case SET_VALIDATION_ERRORS:

return {

...state,

ampCompatibilityBroken: Boolean(

action.validationErrors.filter(

({ status }) =>

status ===

VALIDATION_ERROR_NEW_REJECTED_STATUS ||

status ===

VALIDATION_ERROR_ACK_REJECTED_STATUS

)?.length

),

reviewedValidationErrors:

action.validationErrors.filter(

({ status }) =>

status ===

VALIDATION_ERROR_ACK_ACCEPTED_STATUS ||

status ===

VALIDATION_ERROR_ACK_REJECTED_STATUS

),

unreviewedValidationErrors:

action.validationErrors.filter(

({ status }) =>

status ===

VALIDATION_ERROR_NEW_ACCEPTED_STATUS ||

status ===

VALIDATION_ERROR_NEW_REJECTED_STATUS

),

keptMarkupValidationErrors:

action.validationErrors.filter(

({ status }) =>

status ===

VALIDATION_ERROR_NEW_REJECTED_STATUS ||

status ===

VALIDATION_ERROR_ACK_REJECTED_STATUS

),

validationErrors: action.validationErrors,

};

Previously, unreviewedValidationErrors was being passed manually(which shouldn't be the case) to check if we are getting the expected results. By passing 1 as status the reducer automatically adds the validation error to the unreviewedValidationErrors state.

Also, see how Gutenberg creates stores: https://github.com/WordPress/gutenberg/blob/trunk/packages/notices/src/store/index.js

assets/src/components/conditional-details/test/index.js

westonruter · 2023-05-23T20:36:38Z

.github/workflows/build-test-measure.yml

          COMPOSE_INTERACTIVE_NO_CLI: true

+
+      # TODO: Remove it once node version in .nvmrc is updated to >=16.


Wait, why isn't .nvmrc updated to node 18 if the engines.node was updated in package.json?

Aah, I missed reverting the changes in engines. Since many deps in @wordpress/* are outdated, let's keep node to 14.

…ading attr

westonruter

🎉

thelovekesh · 2023-07-17T16:03:41Z

QA Passed ✅

All JS Unit tests are passing after being migrated to @testing-library/react.
AfterActivationSiteScan service only works when React 18 support is present.

ampprojectbot and others added 22 commits May 16, 2023 03:03

Update Gutenberg package dependencies

6154f24

Bump React and ReactDOM to 18.2.0

5dbd5c4

Bump react-test-renderer to 18.2.0

2cc3288

Add @testing-library/react

a3981ad

Update jest snapshots

f40dfa6

Use mocked DOM events from testing-library

3cc6522

Update lockfile

e9d7684

Update Gutenberg package dependencies

8dd9f2c

Update jest snapshots

9a94fff

Fix bug due to improper effect unmount in useDelayedFlag hook

18c09e8

Upgrade to React 18

1fdf6eb

Update Gutenberg package dependencies

46616f8

Add custom jest resolver to fix export statement syntax errors

5bfd951

See: WordPress/gutenberg#47388 (comment)

Update jest snapshots after upgrading @wordpress/components package

8c0ece9

Update store registration as registerStore is deprecated

f484f1a

Remove block validation store registration

bcb0232

Update usage of block-validation store

1412b5a

Update usage of block-validation store in test cases

d1df79c

Update store registration for block editor as registerStore is depr…

f302dc2

…ecated

Update packages

6a5aa7a

Update jest snapshots after upgrading @wordpress/components package

42874b0

thelovekesh force-pushed the upgrade/react-and-nodejs branch from 8dd1acf to 42874b0 Compare May 15, 2023 21:33

thelovekesh mentioned this pull request May 15, 2023

Upgrade React to v18, update npm deps and migrate JS tests to @testing-library/react #7394

Closed

thelovekesh added 3 commits May 16, 2023 12:23

Update e2e-tests workflow to run e2e tests with Node 16

b4faf01

Fix state batch updates in Options context provider

494d78a

Bump deps to latest version

895be5e

thelovekesh marked this pull request as ready for review May 23, 2023 15:16

Update AfterActivationSiteScan service to work with React 18 only

6cd4845

thelovekesh commented May 23, 2023

View reviewed changes

Update PHPUnit tests workflow to run multisite in existing job

9f7014a

thelovekesh force-pushed the upgrade/react-and-nodejs branch from 4b407be to 9f7014a Compare May 23, 2023 18:28

westonruter reviewed May 23, 2023

View reviewed changes

thelovekesh added 3 commits May 24, 2023 09:19

Remove commented code from block-validation tests

cd2ae21

Revert package.json engines

53630f1

Update gallery handler test cases to remove lazy loading attribute

d09a12a

thelovekesh force-pushed the upgrade/react-and-nodejs branch from a9734e4 to c9a285f Compare May 24, 2023 12:04

Update gallery handler test cases to remove extra whitespaces with lo…

e565be2

…ading attr

thelovekesh force-pushed the upgrade/react-and-nodejs branch 2 times, most recently from e20fd2a to 146e4a3 Compare May 24, 2023 13:25

Update gallery handler test cases to remove lazy loading attribute

eeebbd9

thelovekesh force-pushed the upgrade/react-and-nodejs branch from 146e4a3 to eeebbd9 Compare May 24, 2023 13:38

westonruter added this to the v2.4.2 milestone May 24, 2023

westonruter approved these changes May 24, 2023

View reviewed changes

This comment was marked as duplicate.

Sign in to view

westonruter merged commit 4d05185 into develop May 24, 2023

westonruter deleted the upgrade/react-and-nodejs branch May 24, 2023 17:58

westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Jul 18, 2023

	case SET_VALIDATION_ERRORS:
	return {
	...state,
	ampCompatibilityBroken: Boolean(
	action.validationErrors.filter(
	({ status }) =>
	status ===
	VALIDATION_ERROR_NEW_REJECTED_STATUS \|\|
	status ===
	VALIDATION_ERROR_ACK_REJECTED_STATUS
	)?.length
	),

	reviewedValidationErrors:
	action.validationErrors.filter(
	({ status }) =>
	status ===
	VALIDATION_ERROR_ACK_ACCEPTED_STATUS \|\|
	status ===
	VALIDATION_ERROR_ACK_REJECTED_STATUS
	),

	unreviewedValidationErrors:
	action.validationErrors.filter(
	({ status }) =>
	status ===
	VALIDATION_ERROR_NEW_ACCEPTED_STATUS \|\|
	status ===
	VALIDATION_ERROR_NEW_REJECTED_STATUS
	),

	keptMarkupValidationErrors:
	action.validationErrors.filter(
	({ status }) =>
	status ===
	VALIDATION_ERROR_NEW_REJECTED_STATUS \|\|
	status ===
	VALIDATION_ERROR_ACK_REJECTED_STATUS
	),

	validationErrors: action.validationErrors,
	};

		COMPOSE_INTERACTIVE_NO_CLI: true


		# TODO: Remove it once node version in .nvmrc is updated to >=16.

Conversation

thelovekesh commented May 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

github-actions bot commented May 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

westonruter left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

westonruter left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as duplicate.

Uh oh!

thelovekesh commented Jul 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

thelovekesh commented May 15, 2023 •

edited

Loading

github-actions bot commented May 23, 2023 •

edited

Loading