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

Blocks API: Add default implementation for save setting #14510

Merged
merged 2 commits into from
Mar 20, 2019

Conversation

gziolo
Copy link
Member

@gziolo gziolo commented Mar 19, 2019

Description

There is a proposal to make save setting optional when registering a block. This is an outcome of Block Registration RFC:
https://github.com/WordPress/gutenberg/blob/2df7f19b360d9d223212167fe5dc5531a3a8accd/docs/rfc/block-registration.md#save

This PR adds the default implementation of save setting optional by optimizing for the quite common use case where this method renders nothing on the client and leaves the handling to the server. This is expressed in something similar to the following code:

const save = () = > null;

Such a trivial change makes it easier to express all blocks which are referred in docs as dynamic.

Testing

npm test - make sure all unit tests still pass.
npm run test-e2e - make sure all e2e tests still pass.

Ensure all dynamic blocks still work as before.

All corresponding docs were updated, both usage and example simplified.

@gziolo gziolo added [Type] Enhancement A suggestion for improvement. [Feature] Block API API that allows to express the block paradigm. labels Mar 19, 2019
@gziolo gziolo self-assigned this Mar 19, 2019
@gziolo gziolo requested a review from tellyworth March 19, 2019 11:13
@gziolo gziolo added the [Feature] Block Directory Related to the Block Directory, a repository of block plugins label Mar 19, 2019
@@ -106,12 +106,6 @@ describe( 'blocks', () => {
expect( block ).toBeUndefined();
} );

it( 'should reject blocks without a save function', () => {
Copy link
Member

Choose a reason for hiding this comment

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

Alternatively (or even in place of the "filtered" case below), it could test that save was passed as a non-function.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, I will add it as well.

Copy link
Member Author

@gziolo gziolo Mar 19, 2019

Choose a reason for hiding this comment

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

The reason why I used the approach with a filter was the fact that the existing check verifies if this is still an object after filtering. Maybe I should add another validation and error messaging to check that in test instead and update the removed test as you suggested.

Copy link
Member Author

Choose a reason for hiding this comment

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

I figured out it's going to be easier to open a follow-up which improves error handling together with updates to unit tests: #14529.

@gziolo gziolo merged commit 4d65116 into master Mar 20, 2019
@gziolo gziolo deleted the update/add-default-save-handler-block-api branch March 20, 2019 12:01
@gziolo gziolo added this to the 5.4 (Gutenberg) milestone Mar 20, 2019
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. [Feature] Block Directory Related to the Block Directory, a repository of block plugins [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants