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

Fail gracefully when block in createBlock function is not registered #68043

Open
wants to merge 6 commits into
base: trunk
Choose a base branch
from

Conversation

kmanijak
Copy link
Contributor

@kmanijak kmanijak commented Dec 17, 2024

What?

In this PR I'm checking if block is registered before sanitizing its attributes. If it doesn't exist, return core/missing to gracefully handle the situation.

Why?

This is to preserve editor or inserter crash if createBlock or createBlocksFromInnerBlocksTemplate uses a block that is unregistered for any reason (e.g. due some custom logic in plugins).

Example scenario:

  1. Block variation is build based on createBlocksFromInnerBlocksTemplate and depends on core/post-title
  2. If for any reason core/post-title is unregistered (for example other plugin provides "updated" version of Post Title block)
  3. When inserting the variation, the editor will crash.
  4. If variation is exposed in the inserter (scope: [ 'inserter' ]) it's enough it will appear in results to cause a crash. It will be "rendered" for preview purpose.

To prevent both cases, the core/missing block should be rendered, rather than throwing error, to gracefully handle this situation.

Note

I'm well aware this scenario is against Gutenberg's principles and there should not be the case in which core blocks are unregistered. But at the same time it may happen and unregisterBlockType function is there so any plugin can use it. This PR's purpose is not to encourage that but to gracefully fail instead of crashing editor which prevents further usage of blocks.

How?

Verify if block is registered and return core/missing if not.

Testing Instructions

  1. Create new post
  2. Open browser's console and unregister core/post-title by pasting wp.blocks.unregisterBlockType('core/post-title') (this is to imitate some other plugin does that)
  3. Insert Query Loop block
  4. Choose some variation with title
  5. Expected: Make sure the Editor doesn't crash, there's no error in console and the core/missing is rendered instead of Post Title mentioning its name.
image

Testing Instructions for Keyboard

Screenshots or screencast

Before After
crash-before crash-after

Copy link

github-actions bot commented Dec 17, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: kmanijak <[email protected]>
Co-authored-by: gziolo <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@kmanijak kmanijak changed the title Update/fail gracefully when block is not found Fail gracefully when block is not found in createBlock function Dec 17, 2024
@kmanijak kmanijak changed the title Fail gracefully when block is not found in createBlock function Fail gracefully when block in createBlock function is not registered Dec 17, 2024
@kmanijak kmanijak marked this pull request as draft December 17, 2024 09:52
@kmanijak kmanijak marked this pull request as ready for review December 17, 2024 10:35
@gziolo gziolo added [Feature] Blocks Overall functionality of blocks [Feature] Block API API that allows to express the block paradigm. [Type] Code Quality Issues or PRs that relate to code quality labels Dec 23, 2024
@gziolo
Copy link
Member

gziolo commented Dec 23, 2024

This is a good direction which follows established handling. Example:

} catch {
return createBlock( 'core/missing', {
originalName: name,
originalContent: '',
originalUndelimitedContent: '',
} );
}

// If a Block is undefined at this point, use the core/missing block as
// a placeholder for a better user experience.
if ( undefined === getBlockType( blockName ) ) {
blockAttributes = {
originalName: name,
originalContent: '',
originalUndelimitedContent: '',
};
blockName = 'core/missing';
}
return createBlock(
blockName,
blockAttributes,
synchronizeBlocksWithTemplate( [], innerBlocksTemplate )
);

Now, that createBlock would handle the same exception internally, I would also refactor existing code to remove redundant code. I'm happy to approve the PR once the existing code gets refactored accordingly.

@gziolo gziolo removed the [Feature] Blocks Overall functionality of blocks label Dec 23, 2024
…ases they rely on internal handling of `createBlock` function.
@kmanijak
Copy link
Contributor Author

Now, that createBlock would handle the same exception internally, I would also refactor existing code to remove redundant code. I'm happy to approve the PR once the existing code gets refactored accordingly.

@gziolo, good suggestion! I made the requested changes in b036d44. Let me know if that's what you had in mind.

I see there's one job failing but seems like it's some env issue but I can't rerun the job.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block API API that allows to express the block paradigm. [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