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: Initial explorations to migrate to server-registered blocks #3962

Merged
merged 1 commit into from Dec 14, 2017

Conversation

Projects
None yet
3 participants
@aduth
Member

aduth commented Dec 12, 2017

Related: #2751

This pull request starts to iterate toward server knowledge of registered blocks. This isn't intended to limit the capabilities of a client editor, such that editors are strictly dependent on a WordPress back-end, but opens options for the server to operate from a list of all known blocks (e.g. a settings page dropdown for "default block type", REST API endpoints listing all available blocks on a site).

Eventually I could see this tying into #2756, where all blocks begin by registering a block type from the server, with an associated script handle to define the client-specific form handling. The script may even be optional for very early block prototyping. Having better awareness of blocks and their associated scripts and styles could also enable two optimizations:

  • Loading front-end scripts and styles for a block only when that block is present in content
  • Loading editor scripts on-demand only when that block is present in content or is about to be inserted. This could encourage creation of blocks with larger dependencies, like a CodeMirror block, without weighing down the page load time for users who don't intend to use it.

Implementation notes:

The first iteration seeks to expand the scope of the already-present server block attributes bootstrapping to include a broader range of block properties, not just attributes.

It also improves the server fixtures generation to load more of a WordPress context, as previously we'd merely stubbed the register_block_type function. As more core functions would have been used (e.g. __), this would not have scaled.

The paragraph block was ported as an example, though I'm not sure I'd want to merge with a single example port. I think it'd be preferable to either port all blocks, or none at all.

Testing instructions:

Verify that there are no regressions in the registration of the paragraph block, including title, icon, category, keywords, supports (custom class name), attributes behaviors.

do_action( 'init' );
echo json_encode( gutenberg_prepare_blocks_for_js() );

This comment has been minimized.

@youknowriad

youknowriad Dec 13, 2017

Contributor

So why do we need these fixtures? For test purpose?

Maybe the unit tests should not rely on the fixtures (on the default blocks), we'll probably lose some coverage but I think the unit tests should be "isolated" tests.

@youknowriad

youknowriad Dec 13, 2017

Contributor

So why do we need these fixtures? For test purpose?

Maybe the unit tests should not rely on the fixtures (on the default blocks), we'll probably lose some coverage but I think the unit tests should be "isolated" tests.

This comment has been minimized.

@aduth

aduth Dec 13, 2017

Member

So why do we need these fixtures? For test purpose?

Right, without these, the block content fixture tests fail too, since otherwise the Node process has no way to know what are the attributes of a paragraph block.

@aduth

aduth Dec 13, 2017

Member

So why do we need these fixtures? For test purpose?

Right, without these, the block content fixture tests fail too, since otherwise the Node process has no way to know what are the attributes of a paragraph block.

This comment has been minimized.

@youknowriad

youknowriad Dec 13, 2017

Contributor

Not for this PR, but I think we should avoid registering the default blocks in the tests completely and just provide "dummy blocks" in tests.

@youknowriad

youknowriad Dec 13, 2017

Contributor

Not for this PR, but I think we should avoid registering the default blocks in the tests completely and just provide "dummy blocks" in tests.

@@ -49,6 +49,12 @@ let defaultBlockName;
* registered; otherwise `undefined`.
*/
export function registerBlockType( name, settings ) {
settings = {

This comment has been minimized.

@youknowriad

youknowriad Dec 13, 2017

Contributor

I guess we should update our documentation to favor SSR registration if we go all server side. At that time we could decide to add a "warning" here if the corresponding registration is not found (for a smooth migration)

@youknowriad

youknowriad Dec 13, 2017

Contributor

I guess we should update our documentation to favor SSR registration if we go all server side. At that time we could decide to add a "warning" here if the corresponding registration is not found (for a smooth migration)

This comment has been minimized.

@aduth

aduth Dec 13, 2017

Member

I guess we should update our documentation to favor SSR registration if we go all server side.

Agree.

At that time we could decide to add a "warning" here if the corresponding registration is not found (for a smooth migration)

Customizer controls are bootstrapped in a very similar way, and I find it interesting there's no inclination for developers to go straight to using the JavaScript APIs. I guess one difference here is that we'd probably assume there is a JavaScript counter-part for defining the UI behaviors. Part of my thinking with #2756 is that by moving the script handle to being a property of the block register API, it would require some unconventional hoop-jumping to register the JS block any other way.

Maybe to your earlier work on feeding the editor components with the registered blocks, it would make sense to warn here, since then the editor would still be generic enough to work with client-only blocks in non-WordPress contexts.

@aduth

aduth Dec 13, 2017

Member

I guess we should update our documentation to favor SSR registration if we go all server side.

Agree.

At that time we could decide to add a "warning" here if the corresponding registration is not found (for a smooth migration)

Customizer controls are bootstrapped in a very similar way, and I find it interesting there's no inclination for developers to go straight to using the JavaScript APIs. I guess one difference here is that we'd probably assume there is a JavaScript counter-part for defining the UI behaviors. Part of my thinking with #2756 is that by moving the script handle to being a property of the block register API, it would require some unconventional hoop-jumping to register the JS block any other way.

Maybe to your earlier work on feeding the editor components with the registered blocks, it would make sense to warn here, since then the editor would still be generic enough to work with client-only blocks in non-WordPress contexts.

@aduth

This comment has been minimized.

Show comment
Hide comment
@aduth

aduth Dec 14, 2017

Member

I'm going to rebase to drop the 615320c commit (moving paragraph block registration to server) so we can at least move forward with basic support for block bootstrapping outside just attributes.

Member

aduth commented Dec 14, 2017

I'm going to rebase to drop the 615320c commit (moving paragraph block registration to server) so we can at least move forward with basic support for block bootstrapping outside just attributes.

@aduth aduth merged commit 9aefea3 into master Dec 14, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@aduth aduth deleted the try/server-register-script branch Dec 14, 2017

@jasonbahl

This comment has been minimized.

Show comment
Hide comment
@jasonbahl

jasonbahl Jan 12, 2018

@aduth love seeing this PR!

jasonbahl commented Jan 12, 2018

@aduth love seeing this PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment