Editor Stats - Poor fidelity with blocks_replaced property in block events.

[Addison-Stavlo]

The core data action replaceInnerBlocks is being tracked, and for each block being added via this action we trigger a block inserted event with the blocks_replaced property marked as true. At first glance this seems reasonable, but replaceInnerBlocks is used in a handful of areas where there may not actually be any blocks to replace.

For example:

• Our Page Layout selector is using replaceInnerBlocks to add a layout pattern to the post editor. However, this is currently only accessible when we create a new post and there are no blocks to replace. Therefore blocks_replaced: true is kind of a misleading stat in this case. Now that the page layout selector is no longer available from the sidebar (therefore not usable after there are blocks in the editor), we could probably update this component to use insertBlocks instead, however this is not the only conflict.
• Core Gutenberg uses replaceInnerBlocks in setup states for blocks like the core/query, core/columns, and core/navigation. Meaning that when someone choses a setup option for these blocks, the blocks corresponding to that setup pattern are inserted via replaceInnerBlocks which again triggers block inserted events with blocks_replaced: true even though no blocks are actually being replaced.
• Core Gutenberg also uses replaceInnerBlocks to sync blocks in inner blocks controllers for blocks such as core/block (the reusable block) and core/template-part. Meaning a spam of block inserted events with blocks_replaced: true can happen whenever we load the editor containing one of these blocks, undo/redo effecting contents of these blocks, or edit when duplicates of these blocks are on the page. For now, we can avoid this spam of false events by conditionally ignoring replaceInnerBlocks when the parent is a template part or reusable block (Block Editor Tracking: Fix innerBlocksReplace tracking #53501).

Im not entirely sure the best way to address this. If we remove tracking for replaceInnerBlocks entirely then we will lose block inserted events where we would expect them to show up (such as the setup states noted in example 2 as well as the page layout selector if we don't convert it to use insertBlocks), but in keeping this tracking we don't really have a good way to determine if the blocks_replaced property being added to the insert blocks events is actually true or not.

cc @david-szabo97 @getdave @marekhrabe @retrofox @kurt213

[jordesign]

@getdave @kurt213 Just going through some old issues - hoping for a gut-check on if we're still tracking InnerBlocksReplace and if this issue is still relevant?

[getdave]

I can check this as I'm in amongst the tracking code. Have no idea how to access Tracks or where this data goes so any private links to that would be much appreciated.

[getdave]

@jordesign We are listening for the dispatch of replaceInnerBlocks action.

https://github.com/Automattic/wp-calypso/blob/280d00d6b3d177f341fa6bc7424c4d92ab985656/apps/wpcom-block-editor/src/wpcom/features/tracking.js#L914C23-L914C50

This is handled by a custom tracking handler...

wp-calypso/apps/wpcom-block-editor/src/wpcom/features/tracking.js

Lines 465 to 472 in 280d00d

	/**
	* Track inner blocks replacement.
	* Page Templates insert their content into the page replacing everything that was already there.
	* @param {Array} rootClientId id of parent block
	* @param {Object | Array} blocks block instance object or an array of such objects
	* @returns {void}
	*/
	const trackInnerBlocksReplacement = ( rootClientId, blocks ) => {

...which ignores various sync'd block types:

wp-calypso/apps/wpcom-block-editor/src/wpcom/features/tracking.js

Lines 491 to 503 in 280d00d

	if ( parentBlock ) {
	const { name } = parentBlock;
	if (
	// Template Part
	name === 'core/template-part' ||
	// Reusable Block
	name === 'core/block' ||
	// Post Content
	name === 'core/post-content'
	) {
	return;
	}
	}

Reviewing the original concerns:

Our Page Layout selector is using replaceInnerBlocks...we could probably update this component to use insertBlocks instead, however this is not the only conflict.

This is still the case. We are listening on replaceInnerBlocks and then dispatching tracking for wpcom_block_inserted. We could try updating the implementation code to use insertBlocks and then this concern probably goes away.

Core Gutenberg uses replaceInnerBlocks in setup states for blocks like the core/query, core/columns, and core/navigation

✅ Columns uses it in setup state
https://github.com/WordPress/gutenberg/blob/130b6e248da39012d3018862ef7153ce3ff81bef/packages/block-library/src/columns/edit.js#L147

https://github.com/WordPress/gutenberg/blob/130b6e248da39012d3018862ef7153ce3ff81bef/packages/block-library/src/columns/edit.js#L314

✅ Navigation uses it although not in any setup state as that's no longer in the UI
https://github.com/WordPress/gutenberg/blob/130b6e248da39012d3018862ef7153ce3ff81bef/packages/block-library/src/navigation/edit/index.js#L881-L883

✅ Query uses it in setup state
https://github.com/WordPress/gutenberg/blob/130b6e248da39012d3018862ef7153ce3ff81bef/packages/block-library/src/query/edit/query-placeholder.js#L107

We can't make changes to WP Core at the moment so that's not going to change any time soon and we have no way of differentiating the replaceInnerBlocks event in these scenarios.

Core Gutenberg also uses replaceInnerBlocks to sync blocks

This is already handled by the exception I mentioned above.

---

So ultimately we can fix 1 out of the 3 issues mentioned.

I'm not sure how critical it is to have the level of fidelity requested here but we could work on updating the Page Layout selector if it's sufficiently important.

[jordesign]

Thanks for looking into it @getdave - to be honest I don't know what we ideally need here. It's been 3 years since the ask - so I'd hazard that fixing one of those things is good for now.

[kurt213]

Hi @jordesign thanks for looping me in and @getdave for investigating.

If it's a relatively trivial fix, then even fixing 1 out of the 3 issues mentioned above is great. But in terms of upcoming analyses and data needs, I don't think the blocks_replaced needs to be a critical priority right now, so happy either way with putting this fix in or de-prioritising it.

[jordesign]

@getdave I'd agree that if it is trivial enough to roll out that fix - let's do so and then close the issue.

[getdave]

Our Page Layout selector is using replaceInnerBlocks to add a layout pattern to the post editor. However, this is currently only accessible when we create a new post and there are no blocks to replace. Therefore blocks_replaced: true is kind of a misleading stat in this case. Now that the page layout selector is no longer available from the sidebar (therefore not usable after there are blocks in the editor), we could probably update this component to use insertBlocks instead, however this is not the only conflict.

Ok I'll look into the above Issue specifically and post an update (cc @scruffian).

[getdave]

Our custom implementation is here

https://github.com/Automattic/jetpack/blob/c4666af30bbf536a1e1482d22318667c4a0fff78/projects/packages/jetpack-mu-wpcom/src/features/starter-page-templates/page-patterns-plugin.tsx#L117-L118

Initial research shows that Gutenberg Core does not rely on replaceInnerBlocks to perform a similar task:

https://github.com/WordPress/gutenberg/blob/aec60be54dfeb45191bbf3bb4b284a486303da23/packages/block-editor/src/components/inserter/hooks/use-insertion-point.js#L151-L181

[getdave]

@jordesign I wonder will this UI being going away as Core has removed it in favour of the Patterns sidebar WordPress/gutenberg#66836?

cc @scruffian

[getdave]

@jordesign @kurt213 I looked into this some more and realised that depsite the fact that we are using replaceInnerBlocks the tracing event fired is wpcom_block_inserted.

wp-calypso/apps/wpcom-block-editor/src/wpcom/features/tracking.js

Lines 532 to 534 in 7c8a2f2

	trackBlocksHandler( blocks, 'wpcom_block_inserted', ( { name } ) => ( {
	block_name: name,
	blocks_replaced: true,

We also pass some more useful info along with the event including:

• blocks_replaced: true
• from_template_selector: true

Moving to insertBlocks actions whilst technically acceptable, won't really provide any more value and infact we'd have to write new code to ensure we don't lose the fidelity of from_template_selector.

As the original issue seems to be worried about firing the blocks_replaced tracking event I don't think we need to worry too much as we can verify that wpcom_block_inserted is being fired. So I guess someone updated the event but didn't update this Issue.

[jordesign]

Sounds like we can let this one be closed then. Thanks folks.