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

Add Server Side Render component. #5602

Merged
merged 32 commits into from
Apr 26, 2018
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
cd58776
Add blocks-renderer API endpoint. Add skeleton for ServerSideRender c…
miina Mar 12, 2018
b0ede76
Update SSR preview when editing attributes.
miina Mar 13, 2018
340136e
Fix binding preview change.
miina Mar 13, 2018
514dd4f
Fix binding preview change.
miina Mar 13, 2018
a90f40d
Add tests. Fix permission check for get_item.
miina Mar 13, 2018
0c127e1
Remove debugger line.
miina Mar 14, 2018
b0b977a
Fix some phpcs.
miina Mar 14, 2018
395a168
Fix some jscs.
miina Mar 14, 2018
1d05c2d
Show 'loading' when changing response.
miina Mar 14, 2018
c40dd25
Use wp.apiRequest for consistency.
miina Mar 14, 2018
f323651
Update ServerSideRender readme to note intended usage
westonruter Mar 14, 2018
c4abc69
Rename classes, methods, and property to be more aligned with the res…
westonruter Mar 14, 2018
6557877
Register block-renderer endpoint for each dynamic block
westonruter Mar 14, 2018
228c756
Fixes to README. Use isEqual. Renaming method.
miina Mar 16, 2018
b48ef81
Fix checking for existing response.
miina Mar 16, 2018
e39332d
Add putting together query URL from object.
miina Mar 16, 2018
46506a9
Fix some jscs.
miina Mar 16, 2018
ce1f7c6
Fix using correct props with fetching.
miina Mar 16, 2018
e0fec83
Improve putting together query URL. Use attributes as separate param.
miina Mar 17, 2018
17e404a
Make path constant more readable.
miina Mar 17, 2018
39b867a
Add setting global if is present.
miina Mar 29, 2018
4c3129b
Merge branch 'master' of github.com:WordPress/gutenberg into add/780-…
miina Apr 6, 2018
3c60d4b
Update Readme.
miina Apr 6, 2018
a10bfac
Register post_id. Add context. Fix tests.
miina Apr 16, 2018
a166324
Fix CS.
miina Apr 16, 2018
f0f4a77
Merge remote-tracking branch 'origin/master' into add/780-server-side…
miina Apr 16, 2018
6d1ee65
Add more tests. Add exception for namespace to as ruleset.
miina Apr 25, 2018
2bbbeb7
Fix eslint.
miina Apr 25, 2018
6f7d62e
Merge branch 'master' of github.com:WordPress/gutenberg into add/780-…
miina Apr 25, 2018
67af6b7
Add context param to API request.
miina Apr 25, 2018
007fb1d
Add version numbers to the phpdocs
pento Apr 26, 2018
1c9ef56
Tweak the README a little
pento Apr 26, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions components/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export { default as ResponsiveWrapper } from './responsive-wrapper';
export { default as SandBox } from './sandbox';
export { default as SelectControl } from './select-control';
export { default as Spinner } from './spinner';
export { default as ServerSideRender } from './server-side-render';
export { default as TabPanel } from './tab-panel';
export { default as TextControl } from './text-control';
export { default as TextareaControl } from './textarea-control';
Expand Down
24 changes: 24 additions & 0 deletions components/server-side-render/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
ServerSideRender
=======

ServerSideRender component is used for server-side-rendering preview in Gutenberg editor, specifically for dynamic blocks.


## Usage

Render core/latest-posts preview.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/latest-posts/archives/

```jsx
<ServerSideRender
block="core/latest-posts"
Copy link
Member

Choose a reason for hiding this comment

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

We should probably register another block for something that makes more sense server-rendered (maybe tag cloud widget?) to use as the example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could add Archives block from #5495. It's otherwise ready except for the preview which was waiting for SSR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mtias For usage example I merged the code from this PR to #5495 which adds Archives block -- thought that this way this PR wouldn't have to wait for potential fixes for the Archives block.

So basically #5495 now includes adding Archives block + SSR component with the idea that SSR component could be merged separately first.

Let me know if that's OK or if you'd prefer having the Archives block added here under this PR.

cc @westonruter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mtias Updated the description with added screenshots of Archives block based on SSR. Let me know if there's anything else you see in the PR that requires addressing.

{ this.props.attributes }
/>
```

## Output

Output is using the `render_callback` set when defining the block. For example if `block="core/latest-posts"` as in the example then the output will match `render_callback` output of that block.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/latest-posts/archives/


## API Endpoint
API endpoint for getting the output for ServerSideRender is `/gutenberg/v1/blocks-renderer/:block`. It accepts any params which are used as `attributes` for the block's `render_callback` method.


70 changes: 70 additions & 0 deletions components/server-side-render/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/**
* WordPress dependencies
*/
import {
Component,
RawHTML,
} from '@wordpress/element';
import { __ } from '@wordpress/i18n';
import { addQueryArgs } from '@wordpress/url';

export class ServerSideRender extends Component {
constructor( props ) {
super( props );
this.state = {
response: {},
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the empty object as a representation of missing state a pattern from somewhere else? I'd expect null or undefined to be a better fit. This works well with the fact that the request resolution already checks for response truthiness:

if ( response && response.output ) {

Meaning that we can easily distinguish the three possible states, e.g.:

  • no content: response === null
  • content missing: response !== null && isEmpty( response )
  • count found: ! isEmpty( response )

attributes: props,
};
}

componentDidMount() {
this.getOutput();
}

componentWillReceiveProps( nextProps ) {
if ( JSON.stringify( nextProps ) !== JSON.stringify( this.props ) ) {
Copy link
Member

Choose a reason for hiding this comment

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

What about using isEqual from lodash?

Copy link
Contributor

Choose a reason for hiding this comment

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

  1. I was going to comment that there was room to figure out how aggressive this component should be with refetches and ask if a shallow equality check could make any sense. However, by design, block attributes should (until abused) retain a very simple shape. isEqual should do it.

  2. More importantly, it's not clear to me why there is state synchronization at all. Perhaps this is a relic of a previous solution? response appears to be the only datum requiring local state. Component props should be fed directly into getOutput. If the goal is to trigger a refetch and a rerender, the following should be enough:

componentWillReceiveProps( nextProps ) {
  if ( ! isEqual( nextProps, this.props ) ) {
    this.getOutput();
  }
}

this.setState( { attributes: nextProps }, this.getOutput );
}
}

getOutput() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is a name like fetch more apt?

this.setState( { response: {} } );
const { block } = this.props;
const attributes = this.state.attributes;
const apiURL = addQueryArgs( wpApiSettings.root + 'gutenberg/v1/blocks-renderer/' + block, {
...attributes,
_wpnonce: wpApiSettings.nonce,
} );
return window.fetch( apiURL, {
Copy link
Contributor

Choose a reason for hiding this comment

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

May want to use wp.apiRequest for consistency. See #5253.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think more of a guide on using would help with that. I was working on another PR that used wp.apiRequest and it was leading to test failures (maybe it was how #4710 used in redux connect) so I removed, tests started passing and I could move forward. The only reason my modifications caused issues using wp.apiRequest as far as I could tell was that the tests themselves were not being passed enough context.

credentials: 'include',
} ).then( response => {
response.json().then( data => ( {
data: data,
status: response.status,
} ) ).then( res => {
if ( res.status === 200 ) {
this.setState( { response: res } );
}
} );
} );
}

render() {
const response = this.state.response;
if ( response.isLoading || ! response.data ) {
return (
<div key="loading" className="wp-block-embed is-loading">
Copy link
Member

Choose a reason for hiding this comment

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

For what reason do we need a key?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replaced with Placeholder within #6571.

Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't reuse classes but components. If there's something in the way the embed blocks handles the loading state that is useful, it should be extracted in a separate UI component.


<p>{ __( 'Loading...' ) }</p>
Copy link
Member

Choose a reason for hiding this comment

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

Do we need both a div and a p, or would one or the other have been sufficient?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replaced with Placeholder within #6571.

</div>
);
}

const html = response.data.output;
return (
<RawHTML key="html">{ html }</RawHTML>
);
}
}

export default ServerSideRender;
144 changes: 144 additions & 0 deletions lib/class-wp-rest-blocks-renderer-controller.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
<?php
/**
* Blocks Renderer REST API: WP_REST_Blocks_Renderer_Controller class
*
* @package gutenberg
* @since ?
*/

/**
* Controller which provides REST endpoint for rendering blocks.
*
* @since ?
*
* @see WP_REST_Controller
*/
class WP_REST_Blocks_Renderer_Controller extends WP_REST_Controller {
Copy link
Member

Choose a reason for hiding this comment

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

Should this be singular? Block_Renderer...

Copy link
Member

Choose a reason for hiding this comment

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

Yah, same with the endpoint route.

Copy link
Member

Choose a reason for hiding this comment

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

Done in c4abc69


/**
* Constructs the controller.
*
* @access public
*/
public function __construct() {

// @codingStandardsIgnoreLine - PHPCS mistakes $this->namespace for the namespace keyword.
$this->namespace = 'gutenberg/v1';
$this->rest_base = 'blocks-renderer';
}

/**
* Registers the necessary REST API routes.
*
* @access public
*/
public function register_routes() {

// @codingStandardsIgnoreLine - PHPCS mistakes $this->namespace for the namespace keyword.
register_rest_route( $this->namespace, '/' . $this->rest_base . '/(?P<name>[\w-]+\/[\w-]+)', array(
Copy link
Member

Choose a reason for hiding this comment

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

One idea: instead of letting name be a parameter, we could register over all of the registered blocks and create an endpoint for each one. This could then allow for the block's attributes schema to be used as an attributes object arg in the args here, and it would eliminate the need for prepare_attributes logic in get_item_output. (See https://core.trac.wordpress.org/ticket/38583)

This would allow for a list of all the server-side renderable blocks to be discovered by looking at the REST API schema. Only the blocks returned by get_dynamic_block_names() would be included.

'args' => array(
'name' => array(
'description' => __( 'Unique registered name for the block.', 'gutenberg' ),
'type' => 'string',
),
),
array(
'methods' => WP_REST_Server::READABLE,
'callback' => array( $this, 'get_item_output' ),
'permission_callback' => array( $this, 'get_item_output_permissions_check' ),
'args' => array(
'context' => $this->get_context_param( array( 'default' => 'view' ) ),
),
),
'schema' => array( $this, 'get_public_item_schema' ),
) );
}

/**
* Checks if a given request has access to read blocks.
*
* @since ?
* @access public
*
* @return true|WP_Error True if the request has read access, WP_Error object otherwise.
*/
public function get_item_output_permissions_check() {
if ( ! current_user_can( 'edit_posts' ) ) {
return new WP_Error( 'gutenberg_block_cannot_read', __( 'Sorry, you are not allowed to read Gutenberg blocks as this user.', 'gutenberg' ), array(
'status' => rest_authorization_required_code(),
) );
}

return true;
}

/**
* Returns block output from block's registered render_callback.
*
* @since ?
* @access public
*
* @param WP_REST_Request $request Full details about the request.
* @return WP_REST_Response|WP_Error Response object on success, or WP_Error object on failure.
*/
public function get_item_output( $request ) {
if ( ! isset( $request['name'] ) ) {
return new WP_Error( 'rest_block_invalid_name', __( 'Invalid block name.', 'gutenberg' ), array( 'status' => 404 ) );
}

$registry = WP_Block_Type_Registry::get_instance();
$block = $registry->get_registered( $request['name'] );

if ( ! $block || ! $block instanceof WP_Block_Type ) {
return new WP_Error( 'rest_block_invalid_name', __( 'Invalid block name.', 'gutenberg' ), array( 'status' => 404 ) );
}

$atts = $this->prepare_attributes( $request->get_params() );
Copy link
Member

Choose a reason for hiding this comment

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

Since we have $block here, this means that we have $block->attributes which can be used with rest_validate_value_from_schema() and rest_sanitize_value_from_schema(). For example:

$atts = array();
$schema = $block->attributes;
foreach ( $request->get_params() as $key => $value ) {
	if ( ! isset( $schema[ $key ] ) ) {
		return new WP_Error( 'rest_unrecognized_block_attribute' /* ... */ );
	}
	$validity = rest_validate_value_from_schema( $value, $schema[ $key ], $key );
	if ( is_wp_error( $validity ) ) {
		return $validity;
	}
	$atts[ $key ] = rest_sanitize_value_from_schema( $value, $schema[ $key ] );
}

This should eliminate the need for prepare_attributes entirely.


$data = array(
'output' => $block->render( $atts ),
);
return rest_ensure_response( $data );
}

/**
* Fix potential boolean value issues. The values come as strings and "false" and "true" might generate issues if left like this.
*
* @param array $attributes Attributes.
* @return mixed Attributes.
*/
public function prepare_attributes( $attributes ) {
foreach ( $attributes as $key => $value ) {
if ( 'false' === $value ) {
$attributes[ $key ] = false;
} elseif ( 'true' === $value ) {
$attributes[ $key ] = true;
}
}

return $attributes;
}

/**
* Retrieves block's output schema, conforming to JSON Schema.
*
* @since ?
* @access public
*
* @return array Item schema data.
*/
public function get_item_schema() {
return array(
'$schema' => 'http://json-schema.org/schema#',
'title' => 'blocks-renderer',
'type' => 'object',
'properties' => array(
'output' => array(
Copy link
Member

Choose a reason for hiding this comment

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

I think the rendered name would be more in-line with the rest of the REST API.

Copy link
Member

Choose a reason for hiding this comment

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

Done in c4abc69

'description' => __( 'The block\'s output.', 'gutenberg' ),
'type' => 'string',
'required' => true,
),
),
);
}
}
1 change: 1 addition & 0 deletions lib/load.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
require dirname( __FILE__ ) . '/class-wp-block-type.php';
require dirname( __FILE__ ) . '/class-wp-block-type-registry.php';
require dirname( __FILE__ ) . '/class-wp-rest-blocks-controller.php';
require dirname( __FILE__ ) . '/class-wp-rest-blocks-renderer-controller.php';
require dirname( __FILE__ ) . '/blocks.php';
require dirname( __FILE__ ) . '/client-assets.php';
require dirname( __FILE__ ) . '/compat.php';
Expand Down
11 changes: 11 additions & 0 deletions lib/register.php
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,17 @@ function gutenberg_register_post_types() {
}
add_action( 'init', 'gutenberg_register_post_types' );

/**
* Registers the REST API routes needed by the Gutenberg editor.
*
* @since ?
*/
function gutenberg_register_rest_routes() {
$controller = new WP_REST_Blocks_Renderer_Controller();
$controller->register_routes();
}
add_action( 'rest_api_init', 'gutenberg_register_rest_routes' );

/**
* Gets revisions details for the selected post.
*
Expand Down
Loading