Block API: Introduce block definitions and implementations #6733

youknowriad · 2018-05-14T09:11:41Z

In order to move forward with the server-side awareness of blocks we need to split the block registration into two steps:

1- Defining the existence of the block: Which means specifying its generic properties (name, title, category, description, attribute types)

2- Implementing block types: which means providing edit/save/parse definitions to allow manipulating the block type in the web editor. Other implementation could exist in separate contexts.

This PR is an exploration to split the block settings between two calls:

dispatch( 'core/blocks' ).addBlockType( { name, ...definition } ) (the generic part that can be moved server-side later on.
dispatch( 'core/blocks' ).implementBlockType( { name, edit, save, transforms } )

It leverages the selectors "abstraction" to avoid breaking changes.

This raises several questions:

What belongs to the definition and what is implementation? I'm considering the block name, title, category, description and attributes types (not the way we parse and serialize them) as the canonical definition of the block.
What about the icon? It's considered as implementation for now mainly because we allow svg elements which can't be defined server-side. An option would be to transform it to svg string to allow it to be moved to the definition of the block.
What about the blocks.registerBlockType filter? The most logical thing to do is to split it to two filters: addBlockType (server-side) and implementBlockType (client-side)

At the moment, this raises more questions than it solves, but let's discuss to find the best approach before introducing any deprecations.

aduth · 2018-05-14T13:40:15Z

attributes types (not the way we parse and serialize them)

So not including the source type? Ideally these source types are environment-agnostic, which is partly why I've discouraged using those like the property source, which are more closely tied to the DOM. That said, all of the markup-based sources rely on some ability to parse HTML, which is easier in the browser than it is in other contexts.

aduth · 2018-05-14T13:40:36Z

2- Implementing block types: which means providing edit/save/parse definitions to allow manipulating the block type in the web editor. Other implementation could exist in separate contexts.

So do you see parsing moving to block itself, returning some canonical object form?

youknowriad · 2018-05-14T13:57:50Z

@aduth No, I see parsing/serializing as an implementation detail, For me the source of truth is the block object regardless of the way it's serialized (html, markdown, native UI...). This is why I think it's shouldn't be in the block "definition". Granted that right now we save a serialization but I see it as just a backwards compatibility concern instead of a "conceptual" choice. The conceptual choice is that a post content is an array of blocks, each block is an object with attributes, each attribute has a type.

aduth · 2018-05-14T15:13:22Z

For me the source of truth is the block object regardless of the way it's serialized (html, markdown, native UI...).

This seems like a point we should clarify, as it's been an operating assumption for me that HTML is the source of truth:

To ensure we keep a key component of WordPress’ strength intact, the source of truth for the content should remain in post_content,

https://make.wordpress.org/core/2017/01/17/editor-technical-overview/

noisysocks

The makes a lot of sense to me.

What about the icon? It's considered as implementation for now mainly because we allow svg elements which can't be defined server-side. An option would be to transform it to svg string to allow it to be moved to the definition of the block.

We could have icon supported in both places but only SVGs supported in the implementation.

Alternatively we could move away from inline SVGs and have a seperate iconURL property which points to an external SVG or image.

What about the blocks.registerBlockType filter? The most logical thing to do is to split it to two filters: addBlockType (server-side) and implementBlockType (client-side)

👍

That said, all of the markup-based sources rely on some ability to parse HTML, which is easier in the browser than it is in other contexts.

We could always port hpq to PHP. I've written something similar to this in the past using DOMDocument.

noisysocks · 2018-05-15T00:11:47Z

blocks/store/actions.js

@@ -17,6 +17,20 @@ export function addBlockTypes( blockTypes ) {
 	};
 }

+/**
+ * Returns an action object used in signalling that block types have been added.


This needs updating.

noisysocks · 2018-05-15T00:14:52Z

blocks/store/selectors.js

+
+		return {
+			...blockTypeDefinition,
+			...blockTypeImplementation,


It makes sense to me that the implementation can override attributes that are in the definition.

What about the icon? It's considered as implementation for now mainly because we allow svg elements which can't be defined server-side. An option would be to transform it to svg string to allow it to be moved to the definition of the block.

We could have icon supported both in addBlockType and implementBlockType. SVGs would only be supported in implementBlockType. The attribute specified in the implementation takes priority, as above.

It makes sense to me that the implementation can override attributes that are in the definition.

It's not quite as obvious to me. What's the use case?

noisysocks · 2018-05-15T00:17:55Z

blocks/store/selectors.js

+			...blockTypeDefinition,
+			...blockTypeImplementation,
+			attributes: mapValues( blockTypeDefinition.attributes, ( attribute, key ) => {
+				const implementationAttribute = blockTypeImplementation.attributes ? blockTypeImplementation.attributes[ key ] : {};


Using get( blockTypeImplementation.attributes, [ key ], {} ) here might be easier to understand.

youknowriad · 2018-05-15T07:15:16Z

We had a discussion with @aduth about this and one of the reasons I prefer avoiding the "source" definition to be done server-side is that the parsing (source definition) goes together with serializing (the save function). It doesn't make sense to me to do just parsing unless you consider the block objects as a readonly representation of the post_content which is not the case here.

So if we move the attributes parsing server-side, we'd have to move the save function of blocks server side too but that's not possible as it's a React component.

For these reasons, I prefer to consider the blocks as the source of truth conceptually and leave parsing/serializing as implementation details.

gziolo · 2018-05-16T08:43:55Z

Let me answer your questions based on my reflections after explorations done in #5802 and #5652.

What belongs to the definition and what is implementation? I'm considering the block name, title, category, description and attributes types (not the way we parse and serialize them) as the canonical definition of the block.

I would also include icon, keywords and supports in here. The last one is really important because it is often used with hooks and we will want to run them also on the server.

What about the icon? It's considered as implementation for now mainly because we allow svg elements which can't be defined server-side. An option would be to transform it to svg string to allow it to be moved to the definition of the block.

I think it is fine to allow strings on the server and treat svg as an advanced implementation detail. I wouldn't spend too much time on it, but it would be handy to have a fallback defined on the server so we could lazy load definition of the block on the client.

What about the blocks.registerBlockType filter? The most logical thing to do is to split it to two filters: addBlockType (server-side) and implementBlockType (client-side)

Yes, we should have two types of hooks, one on the server and one on the client. I did that in #5802. My another related comment from a different PR:

To satisfy a few different requirements we should expose a hook on PHP side to make it possible to perform some operations on attributes and other block properties. This was also recognized as a blocker for blocks to use supports align as discussed in here: #5099 (comment). Long story short - we need to have one place where properties get updated to avoid code duplication and confusion. That's why it seems like we should explicitly pick what happens on the server and what on the client.

youknowriad · 2018-05-17T08:40:43Z

So it looks like this PR already meets all/most of the feedback here. I took a look at #5802 and it looks like it could be considered as a follow-up to the current PR.

I'm preparing the client for server-side awareness in this PR
[WIP] Blocks: Make the block registration filterable on the server #5802 moves the definitions to the server (it needs some adaptations)

So, I'd like some feedback from @mtias here before we proceed (Whether you agree or not with this #6733 (comment) )

gziolo · 2018-05-17T08:51:40Z

core-blocks/video/test/index.js

 import { blockEditRender } from '../../test/helpers';

 describe( 'core/video', () => {
 	test( 'block edit matches snapshot', () => {
-		const wrapper = blockEditRender( name, settings );
+		const wrapper = blockEditRender( name, { ...definition, ...implementation } );


Do we need type and default for attributes in tests?
It won't be provided because implementation doesn't contain it.

It's a shallow merge.

youknowriad · 2018-06-20T16:06:01Z

I rebased this one. As discussed in WCEU, let's move this forward to move things to the server as a follow up.

gziolo · 2018-06-25T13:18:36Z

blocks/api/registration.js

@@ -168,7 +168,20 @@ export function registerBlockType( name, settings ) {
 		set( settings, [ 'supports', 'multiple' ], ! settings.useOnce );
 	}

-	dispatch( 'core/blocks' ).addBlockTypes( settings );
+	const implementationOnlyAttributes = [ 'transforms', 'edit', 'save', 'icon', 'getEditWrapperProps' ];


deprecated is missing in here. Is it intentional?

Actually, it's not. I intentionally avoided any deprecations for now. The idea is to do it only once, when we have all the bits in (the server-side part essentially)

I mean, it doesn't matter in this PR, but definitely something, we should keep in mind. Should we add todo comment?

I think it's not necessary because that's the first thing we should do when we move registration to the server. Tell people to register their blocks on the server.

gziolo · 2018-06-25T13:37:56Z

core-blocks/index.js

 	].forEach( ( { name, settings } ) => {
 		registerBlockType( name, settings );
 	} );

+	[ video ].forEach( ( { name, definition, implementation } ) => {
+		dispatch( 'core/blocks' ).addBlockTypes( { name, ...definition } );


I think this is not acceptable solution because it will break all plugins that depend on the following filter:

settings = applyFilters( 'blocks.registerBlockType', settings, name );

In addition this code bypass all validations we have in registerBlockType function.

Good points.

The filter

How to approach this? should we just leave the video block as is for now and refactor all the blocks at once once we create the alternative filters and move things to the server? I'm trying to avoid telling people to rewrite their code twice and only introduce deprecations once we have everything.

In addition this code bypass all validations we have in registerBlockType function.

I guess we should split validation between the two actions. I'll give it a shot

Two times yes :)

It was good for illustration how this can work, but we still need those wrapping API functions to make sure everything is valid.

aduth · 2018-07-10T14:54:05Z

I'm not really on board with where this is going on attributes fragmentation. If the reason is that parsing and serialization should be kept together, what sense do the attributes types have in the server definition? What about comment-serialized attributes? It seems odd to me we have some idea of what attributes exist and where to find them from the server, but that otherwise we distinguish the implementation to have its own behaviors. Would we want separate implementations, and could each implementation have its own sourcing / serialization behaviors? Is that even desirable?

youknowriad · 2018-07-12T10:49:51Z

Would we want separate implementations, and could each implementation have its own sourcing / serialization behaviors? Is that even desirable?

I don't know if that's desirable but if you see the block JSON as the source of truth (instead of the HTML), attributes have a meaning, they are what defines the block content. And just having a type and a default value for each attribute is what we need in that case.

In the https://wordpress.org/plugins/averroes/ plugin I have an example of an alternative implementation where we consider the JSON as the source of truth with custom sourcing / serialization behaviors (from markdown to JSON, from JSON to markdown).

gziolo · 2018-07-20T06:30:31Z

#8046 introduces temporary helper unstable__bootstrapServerSideBlockDefinitions. Let's make also sure this PR replaces it with a proper alternative.

dmsnell · 2018-08-02T11:07:56Z

For these reasons, I prefer to consider the blocks as the source of truth conceptually and leave parsing/serializing as implementation details.

if you see the block JSON as the source of truth (instead of the HTML)

an operating assumption for me that HTML is the source of truth

since the very beginning of the decision to serialize Gutenbergs posts into HTML it has been a truth that "the block doesn't exist outside of memory where the block code is executing." the "block" is defined by code that interprets a bag of attributes.

that we serialize into post_content is a choice for providing a fallback render when WordPress or the block mechanisms aren't present such that a Gutenberg post still reasonably renders when Gutenberg is missing. however, this was not a limiting choice. why shouldn't I write a plugin to replace the parser/serialize or load/save mechanism with one that stores the attributes as JSON-serializable data in another database or via some separate API or internally in Swift into a native data structure in iOS.

our blocks cannot exist without the implementation. they become void of meaning. potentially we can escape this by storing all attributes in the comment attributes thus remove the need to source them from post_content or meta or API calls but this is also somewhat counter to the idea behind the project.

I probably missed the relevant discussion, but what are we solving by making the server more aware of blocks? what would be lacking if we sourced all the attributes and provided them to the dynamic block renderers? what are we trying to accomplish by making this change? It seems like we are making a fundamental change with the patch.

aduth · 2018-08-03T19:31:28Z

@dmsnell Since it's not been mentioned in this pull request, the relevant discussion about server-side registration is at #2751 .

mcsf · 2018-08-06T14:05:41Z

blocks/store/reducer.js

+ *
+ * @return {Object} Updated state.
+ */
+export function implementations( state = {}, action ) {


I don't necessarily oppose, but to me it was unexpected to see this as a separate branch on the client. Does the client really care about which bits of information are part of the definition and which of the implementation? As long as we have distinct actions, we can treat the bits differently as they come in if need be, but then we store everything in a common place for a single block type.

youknowriad · 2018-09-07T12:05:33Z

After the discussions in #core-editor we're going to take another approach to this. Having a json as the entry point to the block definition both server and client side.

youknowriad added the [Feature] Block API API that allows to express the block paradigm. label May 14, 2018

youknowriad self-assigned this May 14, 2018

youknowriad requested review from mtias and a team May 14, 2018 09:11

noisysocks reviewed May 15, 2018

View reviewed changes

gziolo mentioned this pull request May 16, 2018

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

Closed

7 tasks

youknowriad force-pushed the try/block-definition-implementation branch from 2412caf to 40a497a Compare May 17, 2018 08:34

gziolo reviewed May 17, 2018

View reviewed changes

youknowriad force-pushed the try/block-definition-implementation branch from 40a497a to 45445b6 Compare May 17, 2018 08:59

ZebulanStanphill mentioned this pull request Jun 5, 2018

Blockers for launching "Try Gutenberg" callout in core #7147

Closed

youknowriad added 5 commits June 20, 2018 17:03

Blocks: Introduce block definitions and implementations

036e05a

Use the new API for the video block

a691fc7

Clarify implementation only attributes

feb0600

Small changes per review

3d1fd0c

Fix unit tests

44ab993

youknowriad force-pushed the try/block-definition-implementation branch from 45445b6 to 44ab993 Compare June 20, 2018 16:05

youknowriad requested review from gziolo, aduth and a team June 20, 2018 16:06

Fix unit tests

c55f8d9

mtias added this to the 3.2 milestone Jun 21, 2018

gziolo added the [Priority] High Used to indicate top priority items that need quick attention label Jun 25, 2018

gziolo reviewed Jun 25, 2018

View reviewed changes

gziolo modified the milestones: 3.2, 3.3 Jul 5, 2018

youknowriad mentioned this pull request Jul 19, 2018

Packages: Add the blocks module to the packages folder #8046

Merged

youknowriad modified the milestones: 3.3, 3.4 Jul 19, 2018

pento modified the milestones: 3.4, 3.5 Jul 30, 2018

mtias mentioned this pull request Aug 1, 2018

PHP APIs Overview #8352

Closed

16 tasks

mcsf reviewed Aug 6, 2018

View reviewed changes

gziolo modified the milestones: 3.5, 3.6 Aug 8, 2018

gziolo removed the [Priority] High Used to indicate top priority items that need quick attention label Aug 14, 2018

youknowriad modified the milestones: 3.6, 3.7 Aug 15, 2018

youknowriad removed this from the 3.7 milestone Aug 30, 2018

youknowriad closed this Sep 7, 2018

youknowriad deleted the try/block-definition-implementation branch September 7, 2018 12:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Block API: Introduce block definitions and implementations #6733

Block API: Introduce block definitions and implementations #6733

youknowriad commented May 14, 2018 •

edited

Loading

aduth commented May 14, 2018

aduth commented May 14, 2018

youknowriad commented May 14, 2018

aduth commented May 14, 2018

noisysocks left a comment

noisysocks May 15, 2018

noisysocks May 15, 2018

aduth Jul 10, 2018

noisysocks May 15, 2018

youknowriad commented May 15, 2018

gziolo commented May 16, 2018 •

edited

Loading

youknowriad commented May 17, 2018

gziolo May 17, 2018 •

edited

Loading

youknowriad commented Jun 20, 2018

gziolo Jun 25, 2018 •

edited

Loading

youknowriad Jun 25, 2018

gziolo Jun 25, 2018

youknowriad Jun 25, 2018

gziolo Jun 25, 2018

gziolo Jun 25, 2018

youknowriad Jun 25, 2018

gziolo Jun 25, 2018

aduth commented Jul 10, 2018

youknowriad commented Jul 12, 2018

gziolo commented Jul 20, 2018

dmsnell commented Aug 2, 2018

aduth commented Aug 3, 2018

mcsf Aug 6, 2018

youknowriad commented Sep 7, 2018

Block API: Introduce block definitions and implementations #6733

Block API: Introduce block definitions and implementations #6733

Conversation

youknowriad commented May 14, 2018 • edited Loading

aduth commented May 14, 2018

aduth commented May 14, 2018

youknowriad commented May 14, 2018

aduth commented May 14, 2018

noisysocks left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowriad commented May 15, 2018

gziolo commented May 16, 2018 • edited Loading

youknowriad commented May 17, 2018

gziolo May 17, 2018 • edited Loading

Choose a reason for hiding this comment

youknowriad commented Jun 20, 2018

gziolo Jun 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aduth commented Jul 10, 2018

youknowriad commented Jul 12, 2018

gziolo commented Jul 20, 2018

dmsnell commented Aug 2, 2018

aduth commented Aug 3, 2018

Choose a reason for hiding this comment

youknowriad commented Sep 7, 2018

youknowriad commented May 14, 2018 •

edited

Loading

gziolo commented May 16, 2018 •

edited

Loading

gziolo May 17, 2018 •

edited

Loading

gziolo Jun 25, 2018 •

edited

Loading