Blocks: Restore and test Columns migration

packages/block-library/src/columns/index.js

@@ -1,7 +1,7 @@
 /**
  * External dependencies
  */
-import { times, property, omit } from 'lodash';
+import { times } from 'lodash';
 import classnames from 'classnames';
 import memoize from 'memize';
 
@@ -39,6 +39,32 @@ const getColumnsTemplate = memoize( ( columns ) => {
 	return times( columns, () => [ 'core/column' ] );
 } );
 
+/**
+ * Given an HTML string for a deprecated columns inner block, returns the
+ * column index to which the migrated inner block should be assigned. Returns
+ * undefined if the inner block was not assigned to a column.
+ *
+ * @param {string} originalContent Deprecated Columns inner block HTML.
+ *
+ * @return {?number} Column to which inner block is to be assigned.
+ */
+function getDeprecatedLayoutColumn( originalContent ) {
+	let { doc } = getDeprecatedLayoutColumn;
+	if ( ! doc ) {
+		doc = document.implementation.createHTMLDocument( '' );
+		getDeprecatedLayoutColumn.doc = doc;
+	}
+
+	let columnMatch;
+
+	doc.body.innerHTML = originalContent;
+	for ( const classListItem of doc.body.firstChild.classList ) {
+		if ( ( columnMatch = classListItem.match( /^layout-column-(\d+)$/ ) ) ) {
+			return Number( columnMatch[ 1 ] ) - 1;
+		}
+	}
+}
+
 export const name = 'core/columns';
 
 export const settings = {
@@ -70,31 +96,38 @@ export const settings = {
 				},
 			},
 			isEligible( attributes, innerBlocks ) {
-				return innerBlocks.some( property( [ 'attributes', 'layout' ] ) );
-			},
-			migrate( attributes, innerBlocks ) {
-				function withoutLayout( block ) {
-					return {
-						...block,
-						attributes: omit( block.attributes, [ 'layout' ] ),
-					};
+				// Since isEligible is called on every valid instance of the
+				// Columns block and a deprecation is the unlikely case due to
+				// its subsequent migration, optimize for the `false` condition
+				// by performing a naive, inaccurate pass at inner blocks.
+				const isFastPassEligible = innerBlocks.some( ( innerBlock ) => (
+					/layout-column-\d+/.test( innerBlock.originalContent )
+				) );
+
+				if ( ! isFastPassEligible ) {
+					return false;
 				}
 
+				// Only if the fast pass is considered eligible is the more
+				// accurate, durable, slower condition performed.
+				return innerBlocks.some( ( innerBlock ) => (
+					getDeprecatedLayoutColumn( innerBlock.originalContent ) !== undefined
+				) );
+			},
+			migrate( attributes, innerBlocks ) {
 				const columns = innerBlocks.reduce( ( result, innerBlock ) => {
-					const { layout } = innerBlock.attributes;
+					const { originalContent } = innerBlock;
 
-					let columnIndex, columnMatch;
-					if ( layout && ( columnMatch = layout.match( /^column-(\d+)$/ ) ) ) {
-						columnIndex = Number( columnMatch[ 1 ] ) - 1;
-					} else {
+					let columnIndex = getDeprecatedLayoutColumn( originalContent );
+					if ( columnIndex === undefined ) {
 						columnIndex = 0;
 					}
 
 					if ( ! result[ columnIndex ] ) {
 						result[ columnIndex ] = [];
 					}
 
-					result[ columnIndex ].push( withoutLayout( innerBlock ) );
+					result[ columnIndex ].push( innerBlock );
 
 					return result;
 				}, [] );

test/integration/full-content/fixtures/core__columns__deprecated.html

@@ -0,0 +1,16 @@
+<!-- wp:columns {"columns":3} -->
+<div class="wp-block-columns has-3-columns">
+	<!-- wp:paragraph {"layout":"column-1"} -->
+	<p class="layout-column-1">Column One, Paragraph One</p>
+	<!-- /wp:paragraph -->
+	<!-- wp:paragraph {"layout":"column-1"} -->
+	<p class="layout-column-1">Column One, Paragraph Two</p>
+	<!-- /wp:paragraph -->
+	<!-- wp:paragraph {"layout":"column-2"} -->
+	<p class="layout-column-2">Column Two, Paragraph One</p>
+	<!-- /wp:paragraph -->
+	<!-- wp:paragraph {"layout":"column-3"} -->
+	<p class="layout-column-3">Column Three, Paragraph One</p>
+	<!-- /wp:paragraph -->
+</div>
+<!-- /wp:columns -->

test/integration/full-content/fixtures/core__columns__deprecated.json

@@ -0,0 +1,85 @@
+[
+    {
+        "clientId": "_clientId_0",
+        "name": "core/columns",
+        "isValid": true,
+        "attributes": {
+            "columns": 3
+        },
+        "innerBlocks": [
+            {
+                "clientId": "_clientId_0",
+                "name": "core/column",
+                "isValid": true,
+                "attributes": {},
+                "innerBlocks": [
+                    {
+                        "clientId": "_clientId_0",
+                        "name": "core/paragraph",
+                        "isValid": true,
+                        "attributes": {
+                            "content": "Column One, Paragraph One",
+                            "dropCap": false,
+                            "className": "layout-column-1"
+                        },
+                        "innerBlocks": [],
+                        "originalContent": "<p class=\"layout-column-1\">Column One, Paragraph One</p>"
+                    },
+                    {
+                        "clientId": "_clientId_1",
+                        "name": "core/paragraph",
+                        "isValid": true,
+                        "attributes": {
+                            "content": "Column One, Paragraph Two",
+                            "dropCap": false,
+                            "className": "layout-column-1"
+                        },
+                        "innerBlocks": [],
+                        "originalContent": "<p class=\"layout-column-1\">Column One, Paragraph Two</p>"
+                    }
+                ]
+            },
+            {
+                "clientId": "_clientId_1",
+                "name": "core/column",
+                "isValid": true,
+                "attributes": {},
+                "innerBlocks": [
+                    {
+                        "clientId": "_clientId_0",
+                        "name": "core/paragraph",
+                        "isValid": true,
+                        "attributes": {
+                            "content": "Column Two, Paragraph One",
+                            "dropCap": false,
+                            "className": "layout-column-2"
+                        },
+                        "innerBlocks": [],
+                        "originalContent": "<p class=\"layout-column-2\">Column Two, Paragraph One</p>"
+                    }
+                ]
+            },
+            {
+                "clientId": "_clientId_2",
+                "name": "core/column",
+                "isValid": true,
+                "attributes": {},
+                "innerBlocks": [
+                    {
+                        "clientId": "_clientId_0",
+                        "name": "core/paragraph",
+                        "isValid": true,
+                        "attributes": {
+                            "content": "Column Three, Paragraph One",
+                            "dropCap": false,
+                            "className": "layout-column-3"
+                        },
+                        "innerBlocks": [],
+                        "originalContent": "<p class=\"layout-column-3\">Column Three, Paragraph One</p>"
+                    }
+                ]
+            }
+        ],
+        "originalContent": "<div class=\"wp-block-columns has-3-columns\">\n\t\n\t\n\t\n\t\n</div>"
+    }
+]

test/integration/full-content/fixtures/core__columns__deprecated.parsed.json

@@ -0,0 +1,49 @@
+[
+    {
+        "blockName": "core/columns",
+        "attrs": {
+            "columns": 3
+        },
+        "innerBlocks": [
+            {
+                "blockName": "core/paragraph",
+                "attrs": {
+                    "layout": "column-1"
+                },
+                "innerBlocks": [],
+                "innerHTML": "\n\t<p class=\"layout-column-1\">Column One, Paragraph One</p>\n\t"
+            },
+            {
+                "blockName": "core/paragraph",
+                "attrs": {
+                    "layout": "column-1"
+                },
+                "innerBlocks": [],
+                "innerHTML": "\n\t<p class=\"layout-column-1\">Column One, Paragraph Two</p>\n\t"
+            },
+            {
+                "blockName": "core/paragraph",
+                "attrs": {
+                    "layout": "column-2"
+                },
+                "innerBlocks": [],
+                "innerHTML": "\n\t<p class=\"layout-column-2\">Column Two, Paragraph One</p>\n\t"
+            },
+            {
+                "blockName": "core/paragraph",
+                "attrs": {
+                    "layout": "column-3"
+                },
+                "innerBlocks": [],
+                "innerHTML": "\n\t<p class=\"layout-column-3\">Column Three, Paragraph One</p>\n\t"
+            }
+        ],
+        "innerHTML": "\n<div class=\"wp-block-columns has-3-columns\">\n\t\n\t\n\t\n\t\n</div>\n"
+    },
+    {
+        "blockName": null,
+        "attrs": {},
+        "innerBlocks": [],
+        "innerHTML": "\n"
+    }
+]

test/integration/full-content/fixtures/core__columns__deprecated.serialized.html

@@ -0,0 +1,23 @@
+<!-- wp:columns {"columns":3} -->
+<div class="wp-block-columns has-3-columns"><!-- wp:column -->
+<div class="wp-block-column"><!-- wp:paragraph {"className":"layout-column-1"} -->
+<p class="layout-column-1">Column One, Paragraph One</p>
+<!-- /wp:paragraph -->
+
+<!-- wp:paragraph {"className":"layout-column-1"} -->
+<p class="layout-column-1">Column One, Paragraph Two</p>
+<!-- /wp:paragraph --></div>
+<!-- /wp:column -->
+
+<!-- wp:column -->
+<div class="wp-block-column"><!-- wp:paragraph {"className":"layout-column-2"} -->
+<p class="layout-column-2">Column Two, Paragraph One</p>
+<!-- /wp:paragraph --></div>
+<!-- /wp:column -->
+
+<!-- wp:column -->
+<div class="wp-block-column"><!-- wp:paragraph {"className":"layout-column-3"} -->
+<p class="layout-column-3">Column Three, Paragraph One</p>
+<!-- /wp:paragraph --></div>
+<!-- /wp:column --></div>
+<!-- /wp:columns -->

test/integration/full-content/full-content.spec.js

@@ -117,13 +117,15 @@ describe( 'full post content fixture', () => {
 
 			const blocksActual = parse( content );
 
-			// Block validation logs during deprecation migration. Since this
-			// is expected for deprecated blocks, match on filename and allow.
+			// Block validation may log errors during deprecation migration,
+			// unless explicitly handled from a valid block via isEligible.
+			// Match on filename for deprecated blocks fixtures to allow.
 			const isDeprecated = /__deprecated([-_]|$)/.test( f );
 			if ( isDeprecated ) {
-				// eslint-disable-next-line no-console
+				/* eslint-disable no-console */
 				console.warn.mockReset();
-				expect( console ).toHaveErrored();
+				console.error.mockReset();
+				/* eslint-enable no-console */
 			}
 
 			const blocksActualNormalized = normalizeParsedBlocks( blocksActual );