Respect default values of a property of type object during the creation of a Block #26162
Conversation
ntsekouras
added
the
[Feature] Block API
|
Size Change: +25 B (0%) Total Size: 1.19 MB
ℹ️ View Unchanged
|
ntsekouras
requested review from
ajitbohra,
nerrad,
ntwb and
talldan
as code owners
|
@youknowriad, should we include it as a Dev Note in the upcoming WP release? |
There was a problem hiding this comment.
The code itself looks good 👍
I'm wondering if it can have any negative impact on existing plugins that might not want to have those default values of the object to be automatically injected. What if they would like to reset those values, do they need to explicitly set them to undefined?
|
Thanks for reviewing @gziolo!
I think that if someone wouldn't want all the default values to be set, they shouldn't set them on their |
|
I'm not sure this is the right behavior personally. because the default value is actually set, it's just another value, this PR merges nested properties for object values which might not make sense for all kind of objects. |
I can't see how this is not the right behavior personally. So you think it's better to create a block with all the An example usage is on
That's true and even I thought on making this recursive (which might be an option), this will cover the simple objects for now to work properly and we might think of other ways to augment the |
|
Let say, the object argument is a "product" object. and the default value is: while the template provides something like with this PR, the editor will get which is not really what I expect since it's "stealing" the id of another product. (the id is just an example, it could be any property really) |
|
Okay I understand your example, but I still think this kind of thing would be wrong. You should definitely not use an
it seems to me there is a way of generating the Having said that I cannot think of some other cases that make sense, but there could be possibly... So I don't know :) The alternative of keeping this behavior (current handling), is to copy from |
As I said the id is just an example, here's another one: default value template value: you end up with the dummy description for your product. Basically this behavior is wrong for anything that is "optional" and I do believe there's a lot of use-cases like that. In fact objects can also have different shapes for example, imagine an object describing a JSON schema for a scalar value, its default value can be and a template can provide something completely different In this example, the "type" can change completely the format of the object which means merging the two values doesn't make sense. I think your proposal is sane if it was implemented like that from the start and people knew about its behavior but the truth is we don't know how users are using this in the wild, and what kind of objects they're using which means merging might not always be the best way to go. |
Thanks for this discussion @youknowriad, @gziolo - I'll close this PR to not cause any unwanted side effects from current implementations. |
Description
This PR adds some missing functionality in
createBlock(Blocks API) to respect the default values of a property of typeobject. Previously it would overwrite the whole object, ignoring the rest default values. For example if we had inblock.jsonan attribute like this:and created a block with a single object's property set like this:
createBlock( 'test/block', { query: { perPage:10 } }), it would result in keeping only the passed attributes, which in this case isperPage. Thepagesattribute would not be defined.Another not related thing this PR changes, is some whitespaces differences that occurred during the regeneration of
fixtures. The regeneration tool usesspacesand there were some handwritten withtabs.Checklist: