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.

Sign up for GitHub

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

Fix server rendered widget not previewing #29197

Merged
merged 3 commits into from
Feb 24, 2021
Merged

Fix server rendered widget not previewing #29197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
53aed87
Fix archive widget not previewing
kevin940726 Feb 22, 2021
8c7cd25
Update package-lock.json
kevin940726 Feb 22, 2021
736788f
Mark as experimental
kevin940726 Feb 23, 2021
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 package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 15 additions & 6 deletions packages/blocks/src/api/factory.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ import {
getBlockTypes,
getGroupingBlockName,
} from './registration';
import { normalizeBlockType, sanitizeBlockAttributes } from './utils';
import {
normalizeBlockType,
__experimentalSanitizeBlockAttributes,
} from './utils';

/**
* Returns a block object given its type and attributes.
Expand All @@ -42,7 +45,10 @@ import { normalizeBlockType, sanitizeBlockAttributes } from './utils';
* @return {Object} Block object.
*/
export function createBlock( name, attributes = {}, innerBlocks = [] ) {
const sanitizedAttributes = sanitizeBlockAttributes( name, attributes );
const sanitizedAttributes = __experimentalSanitizeBlockAttributes(
name,
attributes
);

const clientId = uuid();

Expand Down Expand Up @@ -104,10 +110,13 @@ export function __experimentalCloneSanitizedBlock(
) {
const clientId = uuid();

const sanitizedAttributes = sanitizeBlockAttributes( block.name, {
...block.attributes,
...mergeAttributes,
} );
const sanitizedAttributes = __experimentalSanitizeBlockAttributes(
block.name,
{
...block.attributes,
...mergeAttributes,
}
);

return {
...block,
Expand Down
1 change: 1 addition & 0 deletions packages/blocks/src/api/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ export {
isValidIcon,
getBlockLabel as __experimentalGetBlockLabel,
getAccessibleBlockLabel as __experimentalGetAccessibleBlockLabel,
__experimentalSanitizeBlockAttributes,
} from './utils';

// Templates are, in a general sense, a basic collection of block nodes with any
Expand Down
32 changes: 22 additions & 10 deletions packages/blocks/src/api/test/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
isUnmodifiedDefaultBlock,
getAccessibleBlockLabel,
getBlockLabel,
sanitizeBlockAttributes,
__experimentalSanitizeBlockAttributes,
} from '../utils';

describe( 'block helpers', () => {
Expand Down Expand Up @@ -231,10 +231,13 @@ describe( 'sanitizeBlockAttributes', () => {
title: 'Test block',
} );

const attributes = sanitizeBlockAttributes( 'core/test-block', {
defined: 'defined-attribute',
notDefined: 'not-defined-attribute',
} );
const attributes = __experimentalSanitizeBlockAttributes(
'core/test-block',
{
defined: 'defined-attribute',
notDefined: 'not-defined-attribute',
}
);

expect( attributes ).toEqual( {
defined: 'defined-attribute',
Expand All @@ -243,7 +246,10 @@ describe( 'sanitizeBlockAttributes', () => {

it( 'throws error if the block is not registered', () => {
expect( () => {
sanitizeBlockAttributes( 'core/not-registered-test-block', {} );
__experimentalSanitizeBlockAttributes(
'core/not-registered-test-block',
{}
);
} ).toThrowErrorMatchingInlineSnapshot(
`"Block type 'core/not-registered-test-block' is not registered."`
);
Expand All @@ -263,7 +269,10 @@ describe( 'sanitizeBlockAttributes', () => {
title: 'Test block',
} );

const attributes = sanitizeBlockAttributes( 'core/test-block', {} );
const attributes = __experimentalSanitizeBlockAttributes(
'core/test-block',
{}
);

expect( attributes ).toEqual( {
hasDefaultValue: 'default-value',
Expand All @@ -287,9 +296,12 @@ describe( 'sanitizeBlockAttributes', () => {
title: 'Test block',
} );

const attributes = sanitizeBlockAttributes( 'core/test-block', {
nodeContent: [ 'test-1', 'test-2' ],
} );
const attributes = __experimentalSanitizeBlockAttributes(
'core/test-block',
{
nodeContent: [ 'test-1', 'test-2' ],
}
);

expect( attributes ).toEqual( {
nodeContent: [ 'test-1', 'test-2' ],
Expand Down
2 changes: 1 addition & 1 deletion packages/blocks/src/api/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ export function getAccessibleBlockLabel(
* @param {Object} attributes The block's attributes.
* @return {Object} The sanitized attributes.
*/
export function sanitizeBlockAttributes( name, attributes ) {
export function __experimentalSanitizeBlockAttributes( name, attributes ) {
// Get the type definition associated with a registered block.
const blockType = getBlockType( name );

Expand Down
1 change: 1 addition & 0 deletions packages/server-side-render/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"dependencies": {
"@babel/runtime": "^7.12.5",
"@wordpress/api-fetch": "file:../api-fetch",
"@wordpress/blocks": "file:../blocks",
"@wordpress/components": "file:../components",
"@wordpress/data": "file:../data",
"@wordpress/deprecated": "file:../deprecated",
Expand Down
9 changes: 7 additions & 2 deletions packages/server-side-render/src/server-side-render.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { __, sprintf } from '@wordpress/i18n';
import apiFetch from '@wordpress/api-fetch';
import { addQueryArgs } from '@wordpress/url';
import { Placeholder, Spinner } from '@wordpress/components';
import { __experimentalSanitizeBlockAttributes } from '@wordpress/blocks';

export function rendererPath( block, attributes = null, urlQueryArgs = {} ) {
return addQueryArgs( `/wp/v2/block-renderer/${ block }`, {
Expand Down Expand Up @@ -60,12 +61,16 @@ export class ServerSideRender extends Component {
urlQueryArgs = {},
} = props;

const sanitizedAttributes =
attributes &&
__experimentalSanitizeBlockAttributes( block, attributes );
Copy link
Member

Choose a reason for hiding this comment

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

Just thinking out loud: An alternative could be to use createBlock here. That way we wouldn't need to add any extra public functions to @wordpress/blocks.

const sanitizedAttributes = createBlock( block, attributes ).attributes;

But I suppose it makes sense for @wordpress/blocks to have sanitizeBlockAttributes as it might be useful in other situations? What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

Clever! But I fear that it's semantically incorrect? In the future, we could add more logics to the createBlock factory than sanitizeBlockAttributes only, that would be hard to catch any unexpected side-effects here.

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough!


// If httpMethod is 'POST', send the attributes in the request body instead of the URL.
// This allows sending a larger attributes object than in a GET request, where the attributes are in the URL.
const isPostRequest = 'POST' === httpMethod;
const urlAttributes = isPostRequest ? null : attributes;
const urlAttributes = isPostRequest ? null : sanitizedAttributes;
const path = rendererPath( block, urlAttributes, urlQueryArgs );
const data = isPostRequest ? { attributes } : null;
const data = isPostRequest ? { attributes: sanitizedAttributes } : null;

// Store the latest fetch request so that when we process it, we can
// check if it is the current request, to avoid race conditions on slow networks.
Expand Down