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

Custom block attributes are not saved #7016

Closed
GenaBitu opened this Issue May 30, 2018 · 8 comments

Comments

Projects
None yet
3 participants
@GenaBitu
Copy link

GenaBitu commented May 30, 2018

Describe the bug
Custom block attributes are not saved

To Reproduce
Steps to reproduce the behavior:

  1. Create a plugin with custom block
  2. Add an attribute with no source to the block (according to the documentation this should mean the attribute is saved in the block comment)
  3. In edit function, add an AJAX call which in the callback calls props.setAttributes

Expected behavior
The attribute is set: Switching to the code editor shows it in the comment and it is present after save and reload

Actual behavior
The attribute is set in the sense that it is in props.attributes, but it isn't present in the block comment in the code editor and isn't saved.

Desktop (please complete the following information):

  • Debian testing
  • Chromium 62.0.3202.89

Additional context

@GenaBitu GenaBitu referenced this issue Jun 1, 2018

Closed

Gutenberg #28

@mitogh

This comment has been minimized.

Copy link
Contributor

mitogh commented Jun 2, 2018

Facing the same problem with meta, if you have a block that has meta and updates are done independently in multiple locations

The only change stored is the last one.

As on this line:

The value of this.props.meta fallbacks to the initial value of the post when loaded so if no Update (click on the button to save) happens this property will always be the initial value and in consequence updates from different blocks to the meta attributes of a post only saves the last value being set to the meta.

@GenaBitu

This comment has been minimized.

Copy link
Author

GenaBitu commented Jun 4, 2018

Not sure if that's the same issue - in my case, the attributes are ok until saving.

@GenaBitu

This comment has been minimized.

Copy link
Author

GenaBitu commented Jun 5, 2018

The issue is still present in 3.0.0 with the difference that the attribute isn't present in the save function (triggered by switching to the code editor)

@GenaBitu

This comment has been minimized.

Copy link
Author

GenaBitu commented Jun 10, 2018

After some more trial and error, it seems the error is only present if the attribute is of type array - might be related to #7242.

@aduth

This comment has been minimized.

Copy link
Member

aduth commented Jun 11, 2018

@GenaBitu I was able to track down the issue here, and while it's admittedly not the most obvious, the issue here is use of Array#push which modifies the reference value. Specifically, it conflicts with the block serialization behavior where a default value is not included in the serialized comments since its omission is inferred as being the default. For your block, when first inserted, the value is set to the default [], you later call Array#push, and because the pushed value is still strictly equal to the default, it is not included in the serialized comment.

The simple solution for you is to use Array#concat and Array#slice to produce new array references:

var newDir = $( this ).html();
if (newDir === "..") {
	// Mutates original `path`: 
	// path.pop();

	// Does not mutate original `path`:
	path = path.slice( 0, path.length - 1 );
} else {
	// Mutates original `path`: 
	// path.push( newDir );

	// Does not mutate original `path`:
	path = path.concat( newDir );
}
props.setAttributes({path: path});
ajax_query(props);

There's a bigger issue here which is that it's an easy situation for a developer to find themselves in, and not an easy to debug or avoid.

I might imagine a few improvements on Gutenberg's end:

  • Use Object.freeze on the block default attribute value to avoid mutation
  • Use a deep equality check to determine whether the default value is equal to its current value
    • This requires either preventing modification to the original default, or assigning the default into the block's initial attributes as a copy, to ensure the original remains intact.
@aduth

This comment has been minimized.

Copy link
Member

aduth commented Jun 11, 2018

Closing in favor of #7252 for future improvements, since the original question has been resolved here. Please let me know if you're still encountering issues.

@GenaBitu

This comment has been minimized.

Copy link
Author

GenaBitu commented Jun 11, 2018

Thank you very much, it works, this would have never occurred to me...
I think it would make sense to have the defaults immutable - it would make it much more idiot-proof...

@aduth

This comment has been minimized.

Copy link
Member

aduth commented Jun 11, 2018

FYI: This also applies to more than just a block's attributes. In general, you should not modify directly (as in via property assignment or Array#push, Array#pop, etc) values passed through as props to edit or save.

See also: https://reactjs.org/docs/components-and-props.html#props-are-read-only

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.