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

Update Callers to handle when getBlockType return undefined #35097

Merged

Conversation

amustaque97
Copy link
Member

Description

In most cases we either need to check if blockType is truthy before using it, or use optional chaining if it makes sense. eg const icon = blockType?.icon;

How has this been tested?

Testing Instruction can be found here #34346 (comment)

Screenshots

Types of changes

Fix

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

cc @gwwar

@amustaque97 amustaque97 marked this pull request as ready for review September 23, 2021 18:12
Copy link
Contributor

@gwwar gwwar left a comment

Choose a reason for hiding this comment

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

Thanks for taking a look @amustaque97! I left a few notes.

@gwwar
Copy link
Contributor

gwwar commented Oct 7, 2021

This PR is part of #34462

To manually test this, create a new post, and in the code editor add:

<!-- wp:foo /-->

Go back to code view, and we should see the missing block:
Screen Shot 2021-08-31 at 3 18 22 PM

Unregister the missing block type in console:

wp.data.dispatch('core/blocks').removeBlockTypes('core/missing')

Try doing some actions like going to code view or trying to click the missing block. Theoretically the editor should not WSOD, or throw a JS error (from any of the areas this PR has updated).

Copy link
Contributor

@gwwar gwwar left a comment

Choose a reason for hiding this comment

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

This one just needs one more change and it should be good to land! Sorry for the delay @amustaque97

I also verified in manual testing that we didn't see any errors related to code changes here when unregistering the missing blockType

packages/block-editor/src/components/copy-handler/index.js Outdated Show resolved Hide resolved
@amustaque97
Copy link
Member Author

@gwwar, I have addressed review comments. Please take a look once you get some time.

Copy link
Contributor

@gwwar gwwar left a comment

Choose a reason for hiding this comment

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

Thanks for your work here @amustaque97! I'll land this once we get a clean test run ✅

@gwwar gwwar merged commit 4ed997d into WordPress:trunk Oct 7, 2021
@github-actions github-actions bot added this to the Gutenberg 11.8 milestone Oct 7, 2021
@amustaque97 amustaque97 deleted the fix/update-callers-getBlockType-undefined branch October 8, 2021 03:41
@gwwar gwwar added [Type] Code Quality Issues or PRs that relate to code quality [Package] Block editor /packages/block-editor labels Oct 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Block editor /packages/block-editor [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants