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

[WIP] Blocks: Make the block registration filterable on the server #5802

Closed
wants to merge 3 commits into from

Conversation

gziolo
Copy link
Member

@gziolo gziolo commented Mar 26, 2018

[UPDATE: moved to #6733]

Description

Implements: #2751, #4116.
Replaces: #5652.

The updated plan is as follows (originally shared in #5652):

  • register_block_type is going to be the only way to register dynamic block and it needs to happen on the server
  • attributes, supports, category, title?, description? will have to be registered on the server. It will be possible to update them only with the PHP hook.
  • All other properties will be provided on the client using extendBlockType with the new corresponding JS hook.
  • registerBlockType will remain with deprecation message for the next 2-3 releases to make sure developers have time to refactor their plugins.

Screenshots (jpeg or gifs if applicable):

There should be no visual changes.

Types of changes

Breaking change with the deprecation strategy to be proposed.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code has proper inline documentation.

@gziolo gziolo added [Feature] Block API API that allows to express the block paradigm. [Type] Technical Prototype Offers a technical exploration into an idea as an example of what's possible [Status] In Progress Tracking issues with work in progress [Feature] Extensibility The ability to extend blocks or the editing experience labels Mar 26, 2018
@gziolo gziolo self-assigned this Mar 26, 2018
@gziolo gziolo force-pushed the update/block-server-side-settings branch from 06953cf to 5ddf413 Compare March 26, 2018 14:45
customClassName: false,
html: false,
},
isPrivate: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this on the client?

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 don't like this property at all :) I will move it to the server, too 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

To make things easier, I moved isPrivate to supports property and renamed to insertable.

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

Any reason this is limited to dynamic blocs?

@@ -172,20 +172,10 @@ export const name = 'core/block';

export const settings = {
title: __( 'Shared Block' ),
Copy link
Contributor

Choose a reason for hiding this comment

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

What about the title and description? It seems to me that blocks discovery would benefit from these properties being declared on the server as well

Copy link
Member Author

@gziolo gziolo Mar 27, 2018

Choose a reason for hiding this comment

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

I'm going back and forth with those two. After another round of musings, I think it makes sense to move them to the server. In addition, we should move non-SVG version of the icon. This might allow lazy loading of blocks on the client because we would have something to display in the inserters.

Everything JS-based becomes a progressive enhancement loaded on the client when requested.

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 moved title, description, icon and keywords to the server, too.

@gziolo
Copy link
Member Author

gziolo commented Mar 27, 2018

Any reason this is limited to dynamic blocs?

Just to keep the scope of the PR smaller. I would like to divide the efforts into 2 chunks. First, I'd like to refactor all dynamic blocks and provide deprecation strategy to that area. In the long run it should be enforced to declare all the properties listed in this PR on the server.

Next step would be to convert all static blocks to be server-side rendered and promote such usage.

@mtias suggested keeping the client-side registration for very simple blocks forever. We will need to come up with a set of rules when that would be possible. The biggest issue we discovered so far is when hooks updates attributes schema. My personal preference would be to have it possible only on the server to make sure that it happens in one place and it ensures that dynamic blocks have always the same schema on the server and the client.

@nb
Copy link
Member

nb commented Apr 3, 2018

How is this related to #4841?

@aduth
Copy link
Member

aduth commented Apr 3, 2018

registerBlockType will remain with deprecation message for the next 2-3 releases to make sure developers have time to refactor their plugins.

If someone wanted to implement a Gutenberg editor outside a WordPress context, how would they do so without being able to register a block? We're limiting capabilities of the client, when we should continue to treat it as first-class, even if data is being pre-loaded by the server. The benefits of registering blocks on the server should be obvious enough to entice plugin authors into doing so.

@gziolo gziolo modified the milestones: 2.6, 2.7 Apr 5, 2018
@gziolo gziolo requested a review from mcsf April 10, 2018 12:18
@mcsf
Copy link
Contributor

mcsf commented Apr 10, 2018

suggested keeping the client-side registration for very simple blocks forever. We will need to come up with a set of rules when that would be possible

This is tricky. On one hand, I was going to comment that we can keep the client-side interface with no restrictions if we make it clear — through naming, foremost — that it's meant for debugging and quick prototyping, thus making it acceptable for the very smallest plugins to use it if they so choose. On the other hand, it's hard to reconcile with:

If someone wanted to implement a Gutenberg editor outside a WordPress context, how would they do so without being able to register a block? We're limiting capabilities of the client, when we should continue to treat it as first-class

I mean, we could say that the server-side method is the canonical one and, as an encouragement, only offer filters for it and not for the client-side method. However, on a technical level, there isn't a strong argument for not opening this up to client-side filtering (except, perhaps, for a concern of consistency), what with hooks readily available.

Trying to think more broadly about Gutenberg as the first-class editor and its role within WordPress, maybe it makes sense — borrowing terminology from React — to think of WordPress as a provider for the editor. Under that light, it would make sense to:

  1. make the server-side method filterable, as WordPress's canonical registration method;
  2. make the client-side method non-filterable, as a low-level interface for debugging/prototyping;
  3. make a very thin "standalone provider" for Gutenberg, independent of WordPress and other backends, to be loaded with Gutenberg if one wishes to use the latter as a standalone editor;
  4. within that standalone provider, leveraging hooks or something equivalent, implement a filterable block-registration interface with ultimately calls the low-level interface cited in (2).

@gziolo
Copy link
Member Author

gziolo commented May 16, 2018

This one is stale and @youknowriad is working on a different set of changes in #6733. Let's close it and continue there.

@gziolo gziolo closed this May 16, 2018
@gziolo gziolo deleted the update/block-server-side-settings branch May 16, 2018 07:55
@designsimply designsimply added [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. and removed [Status] In Progress Tracking issues with work in progress labels Aug 16, 2018
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] Extensibility The ability to extend blocks or the editing experience [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. [Type] Technical Prototype Offers a technical exploration into an idea as an example of what's possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants