Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix uncaught TypeError in Columns block #14605

Merged

Conversation

nerrad
Copy link
Contributor

@nerrad nerrad commented Mar 24, 2019

Description

The following error was discovered for the columns block:

Uncaught (in promise) TypeError: Cannot read property 'innerBlocks' of null.

To Reproduce:

  1. Create a columns block with two columns with content.
  2. Delete the block in each column.
  3. Delete the column container (triggers the error in the console).

The fix is just to ensure we're accessing innerBlocksproperty on a valid block object.

How has this been tested?

  • Ensure unit tests/e2e tests don't fail.
  • Ran through reproducible steps on this branch and ensure error doesn't appear again.

Types of changes

This is a non-breaking bug fix.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@nerrad nerrad marked this pull request as ready for review March 24, 2019 19:18
@nerrad nerrad self-assigned this Mar 24, 2019
@nerrad nerrad added [Type] Bug An existing feature does not function as intended [Package] Block library /packages/block-library [Block] Columns Affects the Columns Block labels Mar 24, 2019
@nerrad nerrad added this to the 5.4 (Gutenberg) milestone Mar 24, 2019
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code change look good, I haven't tested it.

export default compose(
/**
* Selects the child column Blocks for this parent Column
*/
withSelect( ( select, { clientId } ) => {
const { getBlocksByClientId } = select( 'core/editor' );
const block = getBlocksByClientId( clientId )[ 0 ];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, I'm wondering if the following would work as well:

const { innerBlocks = DEFAULT_EMPTY_ARRAY } = getBlocksByClientId( clientId )[ 0 ];

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That won't work if the array returned by getBlocksByClientId is empty. It'll result in something like:

"Can't destructure property 'innerBlocks' of 'undefined' or 'null'"

@nerrad nerrad merged commit f779b93 into master Mar 25, 2019
@nerrad nerrad deleted the BUG/fix-uncaught-in-promise-error-property-on-undefined branch March 25, 2019 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Columns Affects the Columns Block [Package] Block library /packages/block-library [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants