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

Error when deprecating the default value of a block attribute #11705

Closed
bfintal opened this issue Nov 10, 2018 · 5 comments · Fixed by #12757
Closed

Error when deprecating the default value of a block attribute #11705

bfintal opened this issue Nov 10, 2018 · 5 comments · Fixed by #12757
Labels
[Feature] Block API API that allows to express the block paradigm. [Package] Blocks /packages/blocks [Type] Bug An existing feature does not function as intended
Milestone

Comments

@bfintal
Copy link
Contributor

bfintal commented Nov 10, 2018

Describe the bug
Changing a custom block's default attribute value results in the error: This block contains unexpected or invalid content.

To Reproduce
Steps to reproduce the behavior:

  1. Register a custom block with an attribute that has a default value
  2. Edit a post, then add the custom block, save the post
  3. Edit the registered block and change the default value
  4. Edit the previous post. The block will have an error

Expected behavior
The previously added block shouldn't have an error. Since the block with the error was old, I expect that it shouldn't be affected with a change in an attribute's default value.

Also, removing the default value doesn't result in an error, but changing it does.

Desktop (please complete the following information):

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

Smartphone (please complete the following information):

  • OS: MacOS High Sierra 10.13.6
  • Browser: Chrome
  • Version: 69.0.3497.100

Additional context

  • Gutenberg v4.2
@swissspidy
Copy link
Member

What you're seeing is expected behavior.

The previously added block shouldn't have an error. Since the block with the error was old, I expect that it shouldn't be affected with a change in an attribute's default value.

That's not possible. Every block is validated against the registered block type. When you try to edit an existing block after changing the block type's data, there's obviously a mismatch.

The correct way to change default values like you describe is to use the block deprecation feature.

Please have a look at https://wordpress.org/gutenberg/handbook/block-api/deprecated-blocks/ for how deprecations work.

@swissspidy swissspidy added the [Type] Help Request Help with setup, implementation, or "How do I?" questions. label Nov 10, 2018
@bfintal
Copy link
Contributor Author

bfintal commented Nov 12, 2018

Hi @swissspidy. Yes I'm deprecating blocks right now. I'll explain my scenario more below.

The issue I think is when I have a block with an attribute like this:

height: {
	type: 'number',
	default: 400,
},

I then create a page with that block and leave the height attribute as the default and save my page.

Then I deprecate the block's attributes and then change the default value in my original attribute list:

height: {
	type: 'number',
	default: 300,
},

Note that the deprecated attribute still has the value at 400

Editing the page now shows an error.

@bfintal bfintal changed the title Changing a custom block's default attribute value results in an error Deprecating a custom block's default attribute value results in an error Dec 2, 2018
@bfintal bfintal changed the title Deprecating a custom block's default attribute value results in an error Error when deprecating the default value of a block attribute Dec 2, 2018
@talldan
Copy link
Contributor

talldan commented Dec 10, 2018

Did a bit of debugging and managed to reproduce the issue on the columns block by deprecating it and changing the default columns attribute to a different value.

The issue seems to be a problem for attributes that aren't sourced. When assessing whether a deprecation is valid, the default from the current set of attributes is applied instead of the default from the deprecated attributes. This causes the deprecation to fail validation, and so it's skipped here in the getMigratedBlock function:

let migratedAttributes = getBlockAttributes(
deprecatedBlockType,
originalContent,
attributes
);
// Ignore the deprecation if it produces a block which is not valid.
const isValid = isValidBlockContent(
deprecatedBlockType,
migratedAttributes,
originalContent
);
if ( ! isValid ) {
continue;
}

@talldan talldan added [Type] Bug An existing feature does not function as intended [Feature] Block API API that allows to express the block paradigm. [Package] Blocks /packages/blocks and removed [Type] Help Request Help with setup, implementation, or "How do I?" questions. labels Dec 10, 2018
@talldan
Copy link
Contributor

talldan commented Dec 10, 2018

The root cause of the issue is that when I initially saved the columns block, its default columns attribute isn't being serialised. Here's the output:

<!-- wp:columns -->
<div class="wp-block-columns has-2-columns"><!-- wp:column -->
<div class="wp-block-column"><!-- wp:paragraph -->
<p>test1</p>
<!-- /wp:paragraph --></div>
<!-- /wp:column -->

<!-- wp:column -->
<div class="wp-block-column"><!-- wp:paragraph -->
<p>test2</p>
<!-- /wp:paragraph --></div>
<!-- /wp:column --></div>
<!-- /wp:columns -->

Which seems weird, I would have expected this as the first line:

<!-- wp:columns {"columns":2} -->

Not sure if this is a recent change or if it has always been the case. Will continue digging.

edit - this has been the case for a long time, I must never have noticed this behaviour -

if ( 'default' in attributeSchema && attributeSchema.default === value ) {
return result;
}

@talldan
Copy link
Contributor

talldan commented Dec 10, 2018

Hi @bfintal - thanks for the report. I also think this might be a bug. I've put in a PR for a fix, will hopefully get some guidance from a developer with more experience of this part of the codebase.

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. [Package] Blocks /packages/blocks [Type] Bug An existing feature does not function as intended
Projects
None yet
4 participants