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

Extensibility: Introduce BlockSave which would work similar to BlockEdit #3800

Closed
andreiglingeanu opened this issue Dec 4, 2017 · 26 comments
Labels
[Feature] Blocks Overall functionality of blocks [Feature] Extensibility The ability to extend blocks or the editing experience

Comments

@andreiglingeanu
Copy link
Contributor

andreiglingeanu commented Dec 4, 2017

Issue Overview

This issue explores ways to give more flexibility for extending core/* block types.
Context: https://wordpress.org/gutenberg/handbook/extensibility/

The current set of available hooks is very limited and is not enough for providing meaningful customizations.

Specifically:

  1. I see no way to add more raw attributes (the ones that are NOT computed via HTML selectors). getSaveContent.extraProps provides no help here. It only gives us a way to add inline HTML attributes to the final block output.
  2. We need BlockSave which would be similar to BlockEdit hook. Ideally, BlockSave should be called in getSaveContent which would properly allow us to proxy the final result through a component (via HOCs/render props) that will modify the resulting HTML.

export function getSaveContent( blockType, attributes ) {
const { save } = blockType;
let saveContent;
if ( save.prototype instanceof Component ) {
saveContent = createElement( save, { attributes } );
} else {
saveContent = save( { attributes } );
// Special-case function render implementation to allow raw HTML return
if ( 'string' === typeof saveContent ) {
return saveContent;
}
}
const addExtraContainerProps = ( element ) => {
if ( ! element || ! isObject( element ) ) {
return element;
}
// Applying the filters adding extra props
const props = applyFilters( 'getSaveContent.extraProps', { ...element.props }, blockType, attributes );
return cloneElement( element, props );
};
const contentWithExtraProps = Children.map( saveContent, addExtraContainerProps );
// Otherwise, infer as element
return renderToString( contentWithExtraProps );
}

Probably more once I get more deeply into that...

Related Issues and/or PRs

#2474 #3318


I understand that giving such powerful things into the users hands is quite dangerous. The posts may break in case the third-party plugin gets disabled in many unexpected ways (a missing BlockSave, in that particular example, would potentially blow up the attributes parsing with selector | src sources).

But, I really believe that there is a lot more to be done in that area that waits to be explored.

@youknowriad
Copy link
Contributor

youknowriad commented Dec 4, 2017

I see no way to add more raw attributes (the ones that are NOT computed via HTML selectors). getSaveContent.extraProps provides no help here. It only gives us a way to add inline HTML attributes to the final block output.

the getSaveContent.extraProps is the equivalent of BlockSave right now.
Can you clarify (maybe give an example, what attributes you want to add to blocks...), I didn't understand this point. What are "raw attributes not computed via html selectors"?

@andreiglingeanu
Copy link
Contributor Author

andreiglingeanu commented Dec 4, 2017

I was looking to add some values into attrs (which are parseable from the PHP side, without mutating the block innerHTML) and with getSaveContent.extraProps there's no way to do so.


What are "raw attributes not computed via html selectors"?

Values that go into attrs of a block -- color and textColor in that case, without ever touching the innerHTML:

<!-- wp:button {"color":"#eee","textColor":"#0693e3"} -->
<div class="wp-block-button alignnone" style="background-color:#eee" border_type="dashed"><a style="color:#0693e3">heyasda</a></div>
<!-- /wp:button -->

@youknowriad
Copy link
Contributor

@andreamiddleton These attributes are not saved or extracted from the innerHTML, you can achieve what you want by hooking into registerBlockType and adding a new attribute. Here's an example https://github.com/WordPress/gutenberg/blob/master/blocks/hooks/custom-class-name.js#L86

@andreiglingeanu
Copy link
Contributor Author

andreiglingeanu commented Dec 4, 2017

Oh, that's very cool! I was really close to that part of the code, thanks.

This kinda makes the BlockSave useless. But it may be useful for other, more advanced, use cases which I can't think of right now.

@andreiglingeanu
Copy link
Contributor Author

andreiglingeanu commented Dec 4, 2017

Getting back to it, I was forced to do it that way:

let settings = wp.blocks.getBlockType('core/button')
wp.blocks.unregisterBlockType('core/button')
wp.blocks.registerBlockType('core/button', {
	...oldSettings,
	attributes: {
		...settings.attributes,
		heya: {
			type: 'string',
			default: '#cf2e2e',
		},
	},
})

The reason for that hack was this: wp-blocks script does two things:

  1. Registers the hooks (in wp.hooks.*) to be received
  2. Calls each wp.blocks.registerBlockType method of core's blocks immediately, in a sync way

This in itself is not a problem. The problem arise when third-party plugins try to hook into registerBlockType. They literally have no time to do their transformations because action 1) and 2) happen one after another, with no way to interfere between them.


A possible solution:

Split the wp-blocks script into wp-register-blocks-hooks and wp-blocks and allow third-party scripts to perform actions right after wp-register-blocks-hooks but before wp-blocks. Otherwise everyone will be forced to re-register blocks, which may become not a very cheap operation.

@youknowriad
Copy link
Contributor

Good point @andreamiddleton I'm reopening and renaming the issue. We should try to address this issue.

@youknowriad youknowriad reopened this Dec 4, 2017
@youknowriad youknowriad changed the title Extensibility: More ways to extend existing blocks Extensibility: Impossible to hook into registerBlockType because of the scripts loading order Dec 4, 2017
@youknowriad
Copy link
Contributor

cc @gziolo @aduth

@aduth
Copy link
Member

aduth commented Dec 5, 2017

Alternatively we change blocks hooks to operate on the global hooks instance, then simply encourage plugin authors to register their hooks prior to the wp-blocks handle being loaded.

Related: #3493 (comment)

@andreiglingeanu
Copy link
Contributor Author

Yes, I have tried that option. Of course it didn't worked out for me they way one would expect. Frankly, either one will work for me, whichever you get to decide to go with.

@gziolo
Copy link
Member

gziolo commented Dec 6, 2017

Alternatively we change blocks hooks to operate on the global hooks instance, then simply encourage plugin authors to register their hooks prior to the wp-blocks handle being loaded.

It's going to be partially updated with #3827. wp-hooks is always registered before wp-blocks, so it should be possible. This change enforces plugin developer to always register a hook before the modified module is going to to be imported.

@mtias mtias added the [Feature] Extensibility The ability to extend blocks or the editing experience label Dec 7, 2017
@kmgalanakis
Copy link

Correct me if I'm wrong, but it's also impossible to hook to the blocks.BlockEdit hook, after the last update (1.9)

@andreiglingeanu
Copy link
Contributor Author

@kmgalanakis kinda so. It works for blocks that are added after the initial load. Those that happened to be in the editor at the first time just got rendered without taking blocks.BlockEdit into account.

@kmgalanakis
Copy link

@andreiglingeanu I cannot get it working even for the blocks that are added after the initial load, unfortunately. Have you checked yourself with the latest master?

@gziolo
Copy link
Member

gziolo commented Dec 14, 2017

@andreiglingeanu @kmgalanakis I'm working on the PR which should allow to add a filter after BlockEdit got imported, but wasn't yet rendered. I hope that it will help to manage those filters and solve the issue with the enforced dependency of JS files.

@gziolo
Copy link
Member

gziolo commented Dec 19, 2017

block.BlockEdit should be fine now. I think @youknowriad is thinking how to solve the issue with blocks.registerBlockType :)

@andreiglingeanu
Copy link
Contributor Author

andreiglingeanu commented Dec 19, 2017

Confirmed, blocks.BlockEdit is just fine, blocks.registerBlockType is not

@gziolo
Copy link
Member

gziolo commented Jan 15, 2018

#4428 adds another improvement for block.BlockEdit. It will allow to add/remove filters at any time in the Gutenberg's lifecycle and it will automatically re-render all related components :)

@gziolo
Copy link
Member

gziolo commented Jan 24, 2018

Just merged #4428 which should further improve using hooks with the core block. Hopefully, it won't be necessary to unregister them to add modifications from now on.

@andreiglingeanu
Copy link
Contributor Author

Sounds great!

@gziolo gziolo changed the title Extensibility: Impossible to hook into registerBlockType because of the scripts loading order Extensibility: Introduce BlockSave which would work similar to BlockEdit Jan 31, 2018
@gziolo
Copy link
Member

gziolo commented Jan 31, 2018

@andreiglingeanu I renamed this issue because the original title in my opinion is no longer relevant to what is missing. Does it make sense?

@andreiglingeanu
Copy link
Contributor Author

Yes, makes total sense

It's interesting to me how blocks will behave once plugins that used to intercept the saving process would get disabled. Will those blocks just be marked as stale/broken?

@gziolo
Copy link
Member

gziolo commented Jan 31, 2018

There is an unknown type handler set in here:

setUnknownTypeHandlerName( freeform.name );
. So it will be treated as a freeform block - the one that handles also old posts created with the Classic editor.

It applies to both edit and save. They will still work but the block will be treated as HTML code.

@gziolo
Copy link
Member

gziolo commented Feb 16, 2018

@aduth, did we introduce something similar with your change to how save is being processed? I think blocks.getSaveElement plays a similar role to what we can do with blocks.BlockEdit.

blocks.getSaveElement: A filter that applies to the result of a block's save function. This filter is used to replace or extend the element, for example using wp.element.cloneElement to modify the element's props or replace its children, or returning an entirely new element.

It is a bit different than:

blocks.BlockEdit: Used to modify the block's edit component. It receives the original block edit component and returns a new wrapped component.

but this is done this way by design to provide a better automation for the saved content.

Link to full docs for reference: https://github.com/WordPress/gutenberg/blob/master/docs/extensibility.md#modifying-blocks-experimental.

@aduth
Copy link
Member

aduth commented Feb 16, 2018

Correct, I should have been more explicit with linking in #4786, but the new blocks.getSaveElement adds a higher level of control over the save output than just injecting props. It's used for backwards-compatibility for the new RawHTML component.

@gziolo
Copy link
Member

gziolo commented Feb 17, 2018

@andreiglingeanu, I think we addressed all concerns with the linked PRs. Let's close this one. If there are further issues discovered let's create separate actionable issues. Thanks for your feedback and all your help 💯

@gziolo gziolo closed this as completed Feb 17, 2018
@andreiglingeanu
Copy link
Contributor Author

It's great, thank you for taking it in consideration! Will definitely try everything in practice and see how it plays out, thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks [Feature] Extensibility The ability to extend blocks or the editing experience
Projects
None yet
Development

No branches or pull requests

6 participants