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

Block API: Support and bootstrap server-registered block attribute schemas #2529

Merged
merged 2 commits into from
Sep 1, 2017

Conversation

aduth
Copy link
Member

@aduth aduth commented Aug 24, 2017

Related: #1905, #886

This pull request is an extension of #1905, allowing blocks registered on the server to define their attributes structure using the same JSON-schema-like syntax supported in the client. Further, blocks registered this way can omit these attributes in their JavaScript registration, instead taking advantage of automatic attributes bootstrapping from the server to the client.

This replaces the need for blocks registered on the server to handle their own validation and defaulting. Attributes defined in a server block registration are automatically validated using rest_validate_value_from_schema.

Included is a port of the Latest Posts blocks attributes definition from client to server.

Before:

attributes: {
postsToShow: {
type: 'number',
default: 5,
},
displayPostDate: {
type: 'boolean',
default: false,
},
layout: {
type: 'string',
default: 'list',
},
columns: {
type: 'number',
default: 3,
},
align: {
type: 'string',
},
},

After:

'attributes' => array(
'postsToShow' => array(
'type' => 'number',
'default' => 5,
),
'displayPostDate' => array(
'type' => 'boolean',
'default' => false,
),
'layout' => array(
'type' => 'string',
'default' => 'list',
),
'columns' => array(
'type' => 'number',
'default' => 3,
),
'align' => array(
'type' => 'string',
'default' => 'center',
),
),

Testing instructions:

Verify that there are no regressions in the insertion and saving of a Latest Posts block. In fact, note that the client now correctly defaults alignment to center, which was previously applied only in the server rendering.

Ensure Mocha and PHPUnit tests pass:

npm test
phpunit

@aduth aduth added the [Feature] Block API API that allows to express the block paradigm. label Aug 24, 2017
@aduth aduth requested review from westonruter and lamosty August 24, 2017 18:36
@codecov
Copy link

codecov bot commented Aug 24, 2017

Codecov Report

Merging #2529 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #2529      +/-   ##
=========================================
- Coverage   31.42%   31.4%   -0.02%     
=========================================
  Files         177     177              
  Lines        5407    5406       -1     
  Branches      946     946              
=========================================
- Hits         1699    1698       -1     
  Misses       3135    3135              
  Partials      573     573
Impacted Files Coverage Δ
blocks/library/latest-posts/index.js 10% <ø> (ø) ⬆️
blocks/api/registration.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6e663a4...15ac75c. Read the comment docs.

Copy link
Member

@westonruter westonruter left a comment

Choose a reason for hiding this comment

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

I love this!

@aduth aduth force-pushed the update/server-schemas branch from 1851f5a to 15ac75c Compare September 1, 2017 02:18
@aduth aduth merged commit 7a05235 into master Sep 1, 2017
@aduth aduth deleted the update/server-schemas branch September 1, 2017 02:41
@@ -36,28 +36,6 @@ registerBlockType( 'core/latest-posts', {

keywords: [ __( 'recent posts' ) ],

attributes: {
Copy link
Member

Choose a reason for hiding this comment

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

I've been thinking about this a bit more. This kind of feels odd to me—that opening the block JS file has no trace of attributes. (We would also need to handle the categories block.)

Copy link
Member Author

Choose a reason for hiding this comment

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

We could have the block implementer include attributes here, but it's redundant information that could easily fall out of sync.

A single source of truth would be ideal, but if we want to support server validation of block attributes, this would mean that single source must live on the server (or in some way compatible*). In some ways, this could even be seen as a good thing, but it also fragments the implementation of a block (some in the server, some in the client) and hinders the ease of implementing a block which can live exclusively in the client.

* Some previous discussion of compatible formats such as block manifest (JSON) file but... that doesn't really eliminate fragmentation as much as it introduces more 😄

Copy link
Member

Choose a reason for hiding this comment

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

Well, if we move all config to a generic json file, and separate edit / save into their own exported edit.js save.js (or a common index.js file), it might be ok.

The source of truth of a block seems like it should be independent from server/client distinction.

Copy link
Member Author

Choose a reason for hiding this comment

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

Related: #1905 (comment)

Recreating matchers in an environment-agnostic setting is a challenge. This isn't as much of a problem for server-rendered blocks because we assume we don't need matchers (unclear yet if this is a fair assumption).

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants