WooExpress: Checkout and Cart Blocks Deletion Causes Editor to Crash

[tarhi-saad]

Describe the bug

On a WooExpress site, attempting to delete the Cart or Checkout templates within the editor results in a failure, leading to an editor crash.

TypeError: Cannot read properties of null (reading 'name')
    at https://example.com/wp-content/plugins/jetpack/_inc/blocks/editor-experimental.js?minify=false&ver=42e2a7407c643039ec08:37:16226
    at Array.find (<anonymous>)

The source of this bug is most likely related to block locking.

Findings

See original issue https://github.com/woocommerce/woocommerce-gift-cards/issues/701 for additional discussion

I've delved deeper into the issue and found strong indications that the problem originates from the WooCommerce Gift Cards plugin:

The bug can be replicated using a simple block instead of the Gift Card plugin, as follows:

1. Create a block using the @wordpress/create-block package
2. Go to src/block.json, and the following code:

"attributes": {
        "lock": {
            "type": "object",
            "default": {
                "remove": true,
                "move": false
            }
        }
    },
"parent": ["woocommerce/checkout-order-summary-block"],

3. Run npm run build, then npm run plugin-zip
4. Deactivate the Gift Card from your WooExoress site and upload this new plugin. You use the CLI as described here: p1693928195787919/1693489673.470409-slack-C7U3Y3VMY

Jetpack performs a verification to ascertain if specific blocks are children of a premium-content parent block. This can be viewed in the relevant code section here:

// Avoid transforming if any parent is a premium-content block. Blocks share same parents since they
// are siblings, so checking the first one is enough.
if ( blockHasParentPremiumBlock( blocks[ 0 ] ) ) {

The critical function can be seen here:

export const blockHasParentPremiumBlock = block => {
    const { getBlocksByClientId, getBlockParents } = select( 'core/block-editor' );
    const parents = getBlocksByClientId( getBlockParents( block.clientId ) );
    return !! parents.find( parent => parent.name.indexOf( 'premium-content/' ) === 0 );
};

Here's the sequence of events as I understand it:

• When we opt to delete the Checkout (or Cart) template, Jetpack triggers the aforementioned premium-content check.
• This check only encompasses a single Block, specifically, the Gift Card Form Block.
• Notably, during this action, the Gift Card Form Block acquires a new ClientId. This likely indicates a successful removal, but for reasons unknown, it's reintroduced, leading to a fresh ClientId.
• Consequently, the getBlockParents function points to a now non-existent parent ClientId. As a result, we get a JS type error here:

// In this instance, `parents` is equal to `[ null ]`
return !! parents.find( parent => parent.name.indexOf( 'premium-content/' ) === 0 );

Update

Here's a summary of our observations:

• Deleting the Checkout (or Cart) Block triggers a crash when it contains an external Inner Block (e.g., the Gift Cart) possessing the lock attribute.
• While attempting to delete the Checkout (or Cart) Block, Jetpack ascertains if the said "external Inner Block" is premium content. In this process, the "Inner Block" is re-added with a fresh "clientId." However, getBlockParents (clientId) points to the clientId of the now-deleted parent rather than returning an empty array. Consequently, the getBlocksByClientId method produces [null], leading to a crash at this specific line:

export const blockHasParentPremiumBlock = block => {
    const { getBlocksByClientId, getBlockParents } = select( 'core/block-editor' );
    const parents = getBlocksByClientId( getBlockParents( block.clientId ) );
    return !! parents.find( parent => parent.name.indexOf( 'premium-content/' ) === 0 ); // ====> Here
};

• Deselecting all blocks in the editor seems to avoid triggering the bug. After saving the changes, the bug can't be reproduced even after a page reload.
• Jetpack executes the premium-content validation for a Block (or Inner Block) upon selection.

At this point, I believe that the issue relies on a combination of the following:

Forced checkout inner blocks, and; Jetpack's premium content checking.

Reinserting an external Inner Block (e.g., the Gift Card) upon deleting the entire Checkout (or Cart) Block shouldn't be an expected behavior. If a solution isn't possible from our end, then a potential fix from Jetpack could be to manage the null parent case in the code mentioned above.

To reproduce

Steps to reproduce the behavior:

You'll need a WooExpress site. You can create one for free from here.

1. Go to Appearance -> Editor -> Templates -> Checkout (or Cart)
2. Clear customizations of the Checkout (or Cart) template if necessary and reload the page

5. Click on Edit to access the template editor

6. Within the Checkout (or Cart) template editor, open the List View and try to delete the Checkout (or Cart) Block

7. Ensure the editor indeed crashed.

Expected behavior

We should be able to Remove the Checkout (or Cart) template. And the editor shouldn't crash.

Screenshots

The Editor's Crash Message

The console error message

Environment

WordPress (please complete the following information):

• WordPress version: [e.g. 5.9]
• Gutenberg version (if installed): [e.g. 12.0.0]
• WooCommerce version: [e.g. 6.1]
• WooCommerce Blocks version: [e.g. 7.0.0]
• Site language:
• Any other plugins installed: WooCommerce Gift Cards

Desktop (please complete the following information):

• OS: [e.g. macOS, Windows]
• Browser [e.g. chrome, safari]
• Version [e.g. 22]

Smartphone (please complete the following information):

• Device: [e.g. iPhone6]
• OS: [e.g. iOS8.1]
• Browser [e.g. stock browser, safari]
• Version [e.g. 22]

Additional context

Slack discussion: p1693577197864469/1693489673.470409-slack-C7U3Y3VMY Check this issues's discussion: https://github.com/woocommerce/woocommerce-gift-cards/issues/701

[Brianmitchtay]

Took me a while to find this issue because the error message was not included in the text body, so re-stating the error message here for searchability since I almost created a duplicate issue myself.

TypeError: Cannot read properties of null (reading 'name')
    at https://example.com/wp-content/plugins/jetpack/_inc/blocks/editor-experimental.js?minify=false&ver=42e2a7407c643039ec08:37:16226
    at Array.find (<anonymous>)

6849834-zen. I lost an hour working around this issue with a merchant. Really a poor user experience given how many common payment gateways are not compatible with the checkout block. This issue means once the Checkout block is in place there is no going back

[Brianmitchtay]

Marked this as high priority given the poor UX it causes, and that the block versions of cart and checkout are the default on a fresh Woo Express install. Feel free to reprioritize as necessary.

[PanosSynetos]

Hey @Brianmitchtay - Indeed, for the errors we shouldn't just attach an image but paste the error too.

@tarhi-saad @ralucaStan any ideas on when this issue will be planned? Thanks!

[tarhi-saad]

Hello @Brianmitchtay! 👋 Appreciate you sharing the error details. I've updated the issue description accordingly. 🙌

This issue means once the Checkout block is in place there is no going back

While we are working on a fix, you can bypass the issue by deselecting all blocks in the editor, as demonstrated in the subsequent recording:

https://github.com/woocommerce/woocommerce-blocks/assets/14235870/11ac2bc8-203a-45a7-8fa0-794d5c0fcc3e

[tarhi-saad]

@senadir, I could use some insight into the block-locking feature. My current understanding is limited. While digging into the code, I find it challenging to piece together the entire logic. The function useLockedChildren within assets/js/blocks/cart-checkout-shared/hacks.ts might be responsible for reinserting deleted locked blocks. Yet, even after modifying a line in assets/js/blocks/checkout/edit.tsx:

From const blockProps = useBlockPropsWithLocking(); To const blockProps = useBlockProps(); The locked Inner Block is still reinserted upon deletion. This leads me to believe there’s another part in the code I’ve overlooked.

On another note, if tweaking the block-locking feature risks unintended regressions, should we bring this to the Jetpack team's attention? The function in question can be found here:

export const blockHasParentPremiumBlock = block => {
    const { getBlocksByClientId, getBlockParents } = select( 'core/block-editor' );
    const parents = getBlocksByClientId( getBlockParents( block.clientId ) );
    return !! parents.find( parent => parent.name.indexOf( 'premium-content/' ) === 0 );
};

The source of the problem lies in premium.js, where the Jetpack team doesn't account for a null parent. This omission results in the following error that is responsible for causing the Editor to crash: TypeError: Cannot read properties of null (reading 'name').

[ralucaStan]

Saad will timebox a fix for it this week once his work on his current focus is wrapped. If we don't find a fix this is going into the next cycle.

[PanosSynetos]

Hey @Brianmitchtay - This PR fixes the issue with the crash when deleting cart/checkout blocks

• https://github.com/woocommerce/woocommerce-blocks/pull/11024

It will be released this week in WC Blocks 11.2.0; this is not a release that goes into WC monthly;