Double gutenberg_ prefix given to some PHP functions during the build

[adamziel]

Description

Add the following code to packages/block-library/blocks/navigation.php and run the build:

function block_core_navigation_get_post_ids( ) {
}

function block_core_navigation_get_post_ids_from_block( ) {
}

It gets bundled in build/block-library/blocks/navigation.php as:

function gutenberg_block_core_navigation_get_post_ids( ) {
}

function gutenberg_gutenberg_block_core_navigation_get_post_ids_from_block( ) {
}

This is caused by the following webpack transform:

gutenberg/tools/webpack/blocks.js

Lines 122 to 169 in 7c9286a

	transform: ( content ) => {
	const prefix = 'gutenberg_';
	content = content.toString();
	// Within content, search and prefix any function calls from
	// `prefixFunctions` list. This is needed because some functions
	// are called inside block files, but have been declared elsewhere.
	// So with the rename we can call Gutenberg override functions, but the
	// block will still call the core function when updates are back ported.
	content = content.replace(
	new RegExp( prefixFunctions.join( '|' ), 'g' ),
	( match ) => `${ prefix }${ match }`
	);
	// Within content, search for any function definitions. For
	// each, replace every other reference to it in the file.
	return (
	Array.from(
	content.matchAll(
	/^\s*function ([^\(]+)/gm
	)
	)
	.reduce( ( result, [ , functionName ] ) => {
	// Prepend the Gutenberg prefix, substituting any
	// other core prefix (e.g. "wp_").
	return result.replace(
	new RegExp( functionName, 'g' ),
	( match ) =>
	prefix +
	match.replace( /^wp_/, '' )
	);
	}, content )
	// The core blocks override procedure takes place in
	// the init action default priority to ensure that core
	// blocks would have been registered already. Since the
	// blocks implementations occur at the default priority
	// and due to WordPress hooks behavior not considering
	// mutations to the same priority during another's
	// callback, the Gutenberg build blocks are modified
	// to occur at a later priority.
	.replace(
	/(add_action\(\s*'init',\s*'gutenberg_register_block_[^']+'(?!,))/,
	'$1, 20'
	)
	);
	},
	noErrorOnMissing: true,
	},

In short:

1. The transform finds function block_core_navigation_get_post_ids
2. It performs a naive str_replace to gutenberg_block_core_navigation_get_post_ids
3. In the process, block_core_navigation_get_post_ids_from_block also gets affected
4. Then, the transform acts on the block_core_navigation_get_post_ids_from_block separately

And we end up with a double prefix.

Initially seen in #40752 (comment)

Related: #40657

cc @spacedmonkey @gziolo @youknowriad @ellatrix @noisysocks @getdave

[adamziel]

I initially thought of replacing function $name to function gutenberg_$name instead of $name to gutenberg_$name, but that would not handle cases like $post_ids = array_map( 'block_core_navigation_get_post_ids_from_block', iterator_to_array( $inner_blocks ) );.

The only other solution I can think if is checking each match for the presence of gutenberg_ prefix right before replacing. Otherwise, we'd need a PHP parser.

[noisysocks]

Lol nice. Could we use a negative lookahead? Haven't tested it but something like this.

/^\s*function (?!gutenberg_)([^\(]+)/gm

[adamziel]

@noisysocks that would work, we'd just have to stop precomputing all the matches first:

Array.from(
	content.matchAll(
		/^\s*function ([^\(]+)/gm
	)
)
	.reduce( ( result, [ , functionName ] ) => {
		// Prepend the Gutenberg prefix
	}, content )

And start matching and replacing in batches of one:

while ( true ) {
	const match = content.match(
		/^\s*function (?!gutenberg_)([^\(]+)/gm
	);
	if ( ! match ) break;
	// Prepend the Gutenberg prefix
}

[ockham]

Also happening to block_core_navigation_insert_hooked_blocks_into_rest_response (because it fully includes another function name, block_core_navigation_insert_hooked_blocks).

[ockham]

@noisysocks that would work, we'd just have to stop precomputing all the matches first [...] [a]nd start matching and replacing in batches of one: [...]

Alternatively, we might be able to keep the batch processing if we made sure we're only replacing full function names, i.e. tweak this part so that functionName needs to be followed by a non-alphanum, non-underscore:

gutenberg/tools/webpack/blocks.js

Lines 190 to 193 in 6c47fe2

	new RegExp(
	functionName,
	'g'
	),

Finally, a quick-and-dirty fix might be to remove gutenberg_ before adding gutenberg_ 😬

diff --git a/tools/webpack/blocks.js b/tools/webpack/blocks.js
index 080ee2e0124..329510e9fba 100644
--- a/tools/webpack/blocks.js
+++ b/tools/webpack/blocks.js
@@ -194,7 +194,7 @@ module.exports = [
 													( match ) =>
 														prefix +
 														match.replace(
-															/^wp_/,
+															/^(wp_|gutenberg_)/,
 															''
 														)
 												);

[adamziel]

Moving the rewriting to PHP and using the built-in tokenizer would make this much easier.