Synced Pattern: A possible error when a block is unmounted

[Mamaduka]

I noticed this while inspecting warnings logged in debug mode - #61929.

The following e2e test for synced patterns triggers an error - can be inserted after refresh. It most likely happens when a block is unmounted due to page navigation.

I can't reproduce it manually because, unlike e2e tests, the browser's DevTools console doesn't keep logs from previous pages.

Steps to reproduce

1. Enabled script debug mode for testing env. Not required but produces better logs. See the snippet below.
2. Start the wp-env/
3. Open pattern e2e test in UI - npm run test:e2e -- test/e2e/specs/editor/various/patterns.spec.js --ui
4. Run the can be inserted after refresh test.
5. See the error logged in the Console.

SCRIPT_DEBUG Patch

diff --git .wp-env.json .wp-env.json
index 20d5597e54b..57d4e1a72d8 100644
--- .wp-env.json
+++ .wp-env.json
@@ -4,6 +4,9 @@
 	"themes": [ "./test/emptytheme" ],
 	"env": {
 		"tests": {
+			"config": {
+				"SCRIPT_DEBUG": true
+			},
 			"mappings": {
 				"wp-content/plugins/gutenberg": ".",
 				"wp-content/mu-plugins": "./packages/e2e-tests/mu-plugins",

Screenshot

[youknowriad]

Maybe you can try reproducing by switching to "text editor" or something.

[Mamaduka]

Maybe you can try reproducing by switching to "text editor" or something.

I tried, and it didn't work :/

[t-hamano]

I looked into this issue and found that the error occurs here.

gutenberg/test/e2e/specs/editor/various/patterns.spec.js

Line 391 in 62b0efd

await admin.createNewPost();

The following changes seem to resolve the issue.

diff --git a/test/e2e/specs/editor/various/patterns.spec.js b/test/e2e/specs/editor/various/patterns.spec.js
index 77f97ee300..daa962aa36 100644
--- a/test/e2e/specs/editor/various/patterns.spec.js
+++ b/test/e2e/specs/editor/various/patterns.spec.js
@@ -388,6 +388,13 @@ test.describe( 'Synced pattern', () => {
                        .getByRole( 'button', { name: 'Add' } )
                        .click();
 
+               // Wait until the pattern is created.
+               await editor.canvas
+                       .getByRole( 'document', {
+                               name: 'Block: Pattern',
+                       } )
+                       .waitFor();
+
                await admin.createNewPost();
                await editor.canvas
                        .getByRole( 'button', { name: 'Add default block' } )

This code also exists here:

gutenberg/test/e2e/specs/editor/various/patterns.spec.js

Lines 434 to 439 in 62b0efd

	// Wait until the pattern is created.
	await editor.canvas
	.getByRole( 'document', {
	name: 'Block: Pattern',
	} )
	.waitFor();

[talldan]

I can't reproduce it manually because, unlike e2e tests, the browser's DevTools console doesn't keep logs from previous pages.

There's an option that preserves the logs (in chrome browsers):

Having said that I still couldn't reproduce an error.

[Mamaduka]

@talldan, can you reproduce the error when running the e2e test?

[talldan]

@talldan, can you reproduce the error when running the e2e test?

Yep, I see the error when running the test 🤔

It doesn't seem anything to do with synced patterns, just that this test exploits an issue that can probably happen for any block. Here's a fix - Fix error when reloading while saving a pattern

Unfortunately it then leads to another intermittent error when running this test:

Cannot read properties of null (reading 'addEventListener')
TypeError: Cannot read properties of null (reading 'addEventListener')
    at http://localhost:8889/wp-content/plugins/gutenberg/build/rich-text/index.min.js?ver=25d77260a9a178816f37:154:15
    at http://localhost:8889/wp-content/plugins/gutenberg/build/rich-text/index.min.js?ver=25d77260a9a178816f37:382:47
    at Array.map (<anonymous>)
    at http://localhost:8889/wp-content/plugins/gutenberg/build/rich-text/index.min.js?ver=25d77260a9a178816f37:382:33
    at http://localhost:8889/wp-content/plugins/gutenberg/build/compose/index.min.js?ver=d354be3eb10eb2a8b95f:4037:25
    at assignRef (http://localhost:8889/wp-content/plugins/gutenberg/build/compose/index.min.js?ver=d354be3eb10eb2a8b95f:3771:5)
    at http://localhost:8889/wp-content/plugins/gutenberg/build/compose/index.min.js?ver=d354be3eb10eb2a8b95f:3873:7
    at assignRef (http://localhost:8889/wp-content/plugins/gutenberg/build/compose/index.min.js?ver=d354be3eb10eb2a8b95f:3771:5)
    at http://localhost:8889/wp-content/plugins/gutenberg/build/compose/index.min.js?ver=d354be3eb10eb2a8b95f:3873:7
    at commitAttachRef (webpack://gutenberg/./node_modules/react-dom/cjs/react-dom.development.js?:23679:20)

I'm guessing the error can only happen when reloading, probably the code is holding on to element references that have been detached from the DOM or something as the page unloads.

I agree with @t-hamano's proposed change, the test should wait for the pattern to be created. I'm not sure if playwright waits automatically for HTTP requests to be resolved before reloading, if not there's a small chance on a slow server that the pattern won't have been created in time when the page is reloaded.

[Mamaduka]

It looks like the bug is specific to the e2e test and impossible to reproduce manually. @t-hamano's solution sounds like a good fix for it.

Thanks for testing everyone!