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

Blocks: Introduce registerBlockTypeFromMetadata API #30293

Merged
merged 8 commits into from Apr 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions bin/plugin/commands/common.js
Expand Up @@ -116,6 +116,7 @@ function calculateVersionBumpFromChangelog(
if (
lineNormalized.startsWith( '### deprecation' ) ||
lineNormalized.startsWith( '### enhancement' ) ||
lineNormalized.startsWith( '### new api' ) ||
lineNormalized.startsWith( '### new feature' )
) {
versionBump = 'minor';
Expand Down
27 changes: 24 additions & 3 deletions docs/reference-guides/block-api/block-metadata.md
Expand Up @@ -438,7 +438,7 @@ return array(

## Internationalization

WordPress string discovery automatically will translate fields marked in the documentation as translatable using the `textdomain` property when specified in the `block.json` file. In that case, localized properties will be automatically wrapped in `_x` function calls on the backend of WordPress when executing `register_block_type_from_metadata`. These translations are added as an inline script to the `wp-block-library` script handle in WordPress core or to the plugin's script handle.
WordPress string discovery system can automatically translate fields marked in this document as translatable. First, you need to set the `textdomain` property in the `block.json` file that provides block metadata.

**Example:**

Expand All @@ -451,19 +451,40 @@ WordPress string discovery automatically will translate fields marked in the doc
}
```

The way `register_block_type_from_metadata` processes translatable values is roughly equivalent to:
### PHP

In PHP, localized properties will be automatically wrapped in `_x` function calls on the backend of WordPress when executing `register_block_type_from_metadata`. These translations get added as an inline script to the plugin's script handle or to the `wp-block-library` script handle in WordPress core.

The way `register_block_type_from_metadata` processes translatable values is roughly equivalent to the following code snippet:

```php
<?php
$metadata = array(
'title' => _x( 'My block', 'block title', 'my-plugin' ),
'description' => _x( 'My block is fantastic!', 'block description', 'my-plugin' ),
'keywords' => array( _x( 'fantastic', 'block keywords', 'my-plugin' ) ),
'keywords' => array( _x( 'fantastic', 'block keyword', 'my-plugin' ) ),
);
```

Implementation follows the existing [get_plugin_data](https://codex.wordpress.org/Function_Reference/get_plugin_data) function which parses the plugin contents to retrieve the plugin’s metadata, and it applies translations dynamically.

### JavaScript

In JavaScript, you need to use `registerBlockTypeFromMetadata` method from `@wordpress/blocks` package to process loaded block metadata. All localized properties get automatically wrapped in `_x` (from `@wordpress/i18n` package) function calls similar to how it works in PHP.

**Example:**

```js
import { registerBlockTypeFromMetadata } from '@wordpress/blocks';
import Edit from './edit';
import metadata from './block.json';

registerBlockTypeFromMetadata( metadata, {
edit: Edit,
// ...other client-side settings
} );
```

## Backward Compatibility

The existing registration mechanism (both server side and frontend) will continue to work, it will serve as low-level implementation detail for the `block.json` based registration.
Expand Down
8 changes: 2 additions & 6 deletions packages/block-library/src/index.js
Expand Up @@ -4,12 +4,11 @@
import '@wordpress/core-data';
import '@wordpress/block-editor';
import {
registerBlockType,
registerBlockTypeFromMetadata,
setDefaultBlockName,
setFreeformContentHandlerName,
setUnregisteredTypeHandlerName,
setGroupingBlockName,
unstable__bootstrapServerSideBlockDefinitions, // eslint-disable-line camelcase
} from '@wordpress/blocks';

/**
Expand Down Expand Up @@ -106,10 +105,7 @@ const registerBlock = ( block ) => {
return;
}
const { metadata, settings, name } = block;
if ( metadata ) {
unstable__bootstrapServerSideBlockDefinitions( { [ name ]: metadata } );
}
registerBlockType( name, settings );
registerBlockTypeFromMetadata( { name, ...metadata }, settings );
};

/**
Expand Down
12 changes: 8 additions & 4 deletions packages/block-library/src/index.native.js
Expand Up @@ -10,6 +10,7 @@ import { sortBy } from 'lodash';
import {
hasBlockSupport,
registerBlockType,
registerBlockTypeFromMetadata,
setDefaultBlockName,
setFreeformContentHandlerName,
setUnregisteredTypeHandlerName,
Expand Down Expand Up @@ -127,10 +128,13 @@ const registerBlock = ( block ) => {
return;
}
const { metadata, settings, name } = block;
registerBlockType( name, {
...metadata,
...settings,
} );
registerBlockTypeFromMetadata(
{
name,
...metadata,
},
settings
);
};

/**
Expand Down
4 changes: 4 additions & 0 deletions packages/blocks/CHANGELOG.md
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### New API

- `registerBlockTypeFromMetadata` method can be used to register a block type using the metadata loaded from `block.json` file ([#30293](https://github.com/WordPress/gutenberg/pull/30293)).

## 8.0.0 (2021-03-17)

### Breaking Change
Expand Down
20 changes: 20 additions & 0 deletions packages/blocks/README.md
Expand Up @@ -713,6 +713,26 @@ _Returns_

- `?WPBlock`: The block, if it has been successfully registered; otherwise `undefined`.

<a name="registerBlockTypeFromMetadata" href="#registerBlockTypeFromMetadata">#</a> **registerBlockTypeFromMetadata**

Registers a new block provided from metadata stored in `block.json` file.
It uses `registerBlockType` internally.

_Related_

- registerBlockType

_Parameters_

- _metadata_ `Object`: Block metadata loaded from `block.json`.
- _metadata.name_ `string`: Block name.
- _metadata.textdomain_ `string`: Textdomain to use with translations.
- _additionalSettings_ `Object`: Additional block settings.

_Returns_

- `?WPBlock`: The block, if it has been successfully registered; otherwise `undefined`.

<a name="registerBlockVariation" href="#registerBlockVariation">#</a> **registerBlockVariation**

Registers a new block variation for the given block type.
Expand Down
17 changes: 17 additions & 0 deletions packages/blocks/src/api/i18n-block.json
@@ -0,0 +1,17 @@
{
Copy link
Member Author

Choose a reason for hiding this comment

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

@gwwar, should we put here variations as well which we plan to integrate with block.json soon?

There is one more place where we would need to put it on the allow list.

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 it'd be worth a try, will it affect the needed PR order?

Copy link
Member Author

Choose a reason for hiding this comment

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

Let me think about what we need to change to enable variations in block.json on the PHP side. As far as I can tell, it's mostly the same set of changes like here now that WP_Block_Type and the REST API endpoint support block variations. Given that you won't be able to register block variations on the server until WP 5.8 is out the order shouldn't matter at all. We also have a mechanism in place that favors definitions from the server but fallback to the client when not set there. I think it's safe to change here at the same time 👍🏻

Copy link
Member Author

Choose a reason for hiding this comment

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

Added in b851c2f. Every change to docs get synced with https://developer.wordpress.org/block-editor/reference-guides/block-api/block-metadata/ so I will open another PR that covers the part that describes variations in the block.json file. We should plan to merge it around WordPress 5.8 RC.1 when all Dev Notes get published.

"title": "block title",
"description": "block description",
"keywords": [ "block keyword" ],
"styles": [
{
"label": "block style label"
}
],
"variations": [
Copy link
Member Author

@gziolo gziolo Apr 23, 2021

Choose a reason for hiding this comment

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

@nosolosw and @jorgefilipecosta – this i18n schema format is very flexible, it works with quite complex structures like the one for block variations.

Copy link
Member

Choose a reason for hiding this comment

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

That's good news :)

{
"title": "block variation title",
"description": "block variation description",
"keywords": [ "block variation keyword" ]
}
]
}
1 change: 1 addition & 0 deletions packages/blocks/src/api/index.js
Expand Up @@ -108,6 +108,7 @@ export { getCategories, setCategories, updateCategory } from './categories';
// children of another block.
export {
registerBlockType,
registerBlockTypeFromMetadata,
registerBlockCollection,
unregisterBlockType,
setFreeformContentHandlerName,
Expand Down
115 changes: 115 additions & 0 deletions packages/blocks/src/api/registration.js
Expand Up @@ -5,9 +5,13 @@
*/
import {
camelCase,
isArray,
isEmpty,
isFunction,
isNil,
isObject,
isPlainObject,
isString,
mapKeys,
omit,
pick,
Expand All @@ -20,11 +24,13 @@ import {
*/
import { applyFilters } from '@wordpress/hooks';
import { select, dispatch } from '@wordpress/data';
import { _x } from '@wordpress/i18n';
import { blockDefault } from '@wordpress/icons';

/**
* Internal dependencies
*/
import i18nBlockSchema from './i18n-block.json';
import { isValidIcon, normalizeIconObject } from './utils';
import { DEPRECATED_ENTRY_KEYS } from './constants';
import { store as blocksStore } from '../store';
Expand Down Expand Up @@ -310,6 +316,115 @@ export function registerBlockType( name, settings ) {
return settings;
}

/**
* Translates block settings provided with metadata using the i18n schema.
*
* @param {string|string[]|Object[]} i18nSchema I18n schema for the block setting.
* @param {string|string[]|Object[]} settingValue Value for the block setting.
* @param {string} textdomain Textdomain to use with translations.
*
* @return {string|string[]|Object[]} Translated setting.
*/
function translateBlockSettingUsingI18nSchema(
i18nSchema,
settingValue,
textdomain
) {
if ( isString( i18nSchema ) && isString( settingValue ) ) {
// eslint-disable-next-line @wordpress/i18n-no-variables, @wordpress/i18n-text-domain
Copy link
Member Author

Choose a reason for hiding this comment

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

@swissspidy, do we need to do something special to ensure this particular translation call is ignored during WP-CLI processing?

Copy link
Member

Choose a reason for hiding this comment

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

String extraction will skip anything that‘s not a string literal, so no.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

@gziolo How are the translations supposed to be in wp.i18n without a specific wp.i18n.setLocaleData() call? The strings from the block.json aren't assigned to a JavaScript file thus not automatically part of a JSON translation file. But I might be missing something here? Or are we fully relying on server-side registration for the strings?

Copy link
Member Author

@gziolo gziolo Jun 21, 2021

Choose a reason for hiding this comment

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

In the context of WordPress core, they are always registered on the server and translated there before passed to the client. At the moment, there is a temporary JS method unstable__bootstrapServerSideBlockDefinitions that takes care of setting data generated with PHP. In the future, we are going to use the REST API endpoint for blocks, but the general idea is going to be the same - translations get applied when registering a block on the server.

In practice, this implementation is a fallback for everything else that depends only on JavaScript:

  • standalone block editor like Storybook integration (although this is a bad example as it is never translated)
  • native mobile apps still consume only JavaScript codebase so they have to collect all translations themselves (it might change when we start using the REST API endpoint for blocks)

return _x( settingValue, i18nSchema, textdomain );
Copy link
Member

Choose a reason for hiding this comment

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

Aren't we going to cause a string extraction because of the usage of _x function? Maybe because we are using variables the extraction does not happen? On the PHP side, we use an internal function to avoid this issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

See comment from @swissspidy in #30293 (comment):

String extraction will skip anything that‘s not a string literal

Copy link
Member

Choose a reason for hiding this comment

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

Things like the block description currently don't have a context and now are going to have one. I guess their current translation will be invalidated, should we make some dev note or something equivalent to dev note for translators?

Copy link
Member Author

Choose a reason for hiding this comment

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

As far as I understand it will be automatically handled during the WordPress release cycle, and it's the same process that happens for every newly added or updated translation. We did a batch updated of UI labels in the block editor in the past, and I don't remember any dev notes related to that.

Copy link
Member

Choose a reason for hiding this comment

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

In the branch of PR #31120, I made a simple test and replaced "return _x( settingValue, i18nSchema, textdomain );" with "return 'ok';". Nothing changed. With this change shouldn't all the block titles and descriptions appear as "ok"?

Copy link
Member Author

Choose a reason for hiding this comment

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

It might require adding a fallback to read the value from the client when it isn't provided from the server. It would be useful for WordPress 5.7 when using with the Gutenberg plugin.

Copy link
Member Author

@gziolo gziolo Apr 26, 2021

Choose a reason for hiding this comment

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

I investigate it further and it's a different use case than apiVersion setting that I was referring to:

if ( serverSideBlockDefinitions[ blockName ] ) {
// We still need to polyfill `apiVersion` for WordPress version
// lower than 5.7. If it isn't present in the definition shared
// from the server, we try to fallback to the definition passed.
// @see https://github.com/WordPress/gutenberg/pull/29279
if (
serverSideBlockDefinitions[ blockName ].apiVersion ===
undefined &&
definitions[ blockName ].apiVersion
) {
serverSideBlockDefinitions[ blockName ].apiVersion =
definitions[ blockName ].apiVersion;
}
continue;
}

We register all blocks on the server and we set now all translatable fields there and they are exposed to the client. The values from the server take precedence to respect all PHP filters that could get applied to the metadata. It's the same data after all, so there is no reason to override it with the same metadata read on the client.

To test it, you could change the method I embedded above and enforce that those fields get overridden with the values loaded on the client. Maybe you could also tweak register_block_type_from_metadata calls on the server to somehow ignore translatable fields using block_type_metadata_settings filter, but you rather would need to prevent registration of the block you want to test with on the server.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for the clarification 👍

}
if (
isArray( i18nSchema ) &&
! isEmpty( i18nSchema ) &&
isArray( settingValue )
) {
return settingValue.map( ( value ) =>
translateBlockSettingUsingI18nSchema(
i18nSchema[ 0 ],
value,
textdomain
)
);
}
if (
isObject( i18nSchema ) &&
! isEmpty( i18nSchema ) &&
isObject( settingValue )
) {
return Object.keys( settingValue ).reduce( ( accumulator, key ) => {
if ( ! i18nSchema[ key ] ) {
accumulator[ key ] = settingValue[ key ];
return accumulator;
}
accumulator[ key ] = translateBlockSettingUsingI18nSchema(
i18nSchema[ key ],
settingValue[ key ],
textdomain
);
return accumulator;
}, {} );
}
return settingValue;
}

/**
* Registers a new block provided from metadata stored in `block.json` file.
* It uses `registerBlockType` internally.
*
* @see registerBlockType
*
* @param {Object} metadata Block metadata loaded from `block.json`.
* @param {string} metadata.name Block name.
* @param {string} metadata.textdomain Textdomain to use with translations.
* @param {Object} additionalSettings Additional block settings.
*
* @return {?WPBlock} The block, if it has been successfully registered;
* otherwise `undefined`.
*/
export function registerBlockTypeFromMetadata(
{ name, textdomain, ...metadata },
additionalSettings
) {
const allowedFields = [
'apiVersion',
'title',
'category',
'parent',
'icon',
'description',
'keywords',
'attributes',
Copy link
Contributor

Choose a reason for hiding this comment

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

Does i18n support get added for string default values? For example let's say we have

"attributes": {
		"label": {
			"type": "string",
                         "default": "Home",
		},
		"opensInNewTab": {
			"type": "boolean",
                         "default": false,
		},
}

Copy link
Member Author

Choose a reason for hiding this comment

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

No, we should rather avoid it because it won't produce predictable content. Depending on the language picked in the admin UI, you could get a different output for the same block. We need to remember also that, the value of an attribute when strictly equal to the default value it doesn't get stored in HTML comment of the block definition. The other challenge is, how would you decide which default values need to be translated and for which blocks?

Copy link
Contributor

Choose a reason for hiding this comment

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

We need to remember also that, the value of an attribute when strictly equal to the default value it doesn't get stored in HTML comment of the block definition.

🤔 good to highlight this case.

The other challenge is, how would you decide which default values need to be translated and for which blocks?

If we wanted to support it, we could add an additional attribute. I found that I did need it in one of the blocks, but the behavior can also be simulated with a useEffect hook in the block.

Copy link
Member Author

Choose a reason for hiding this comment

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

If we wanted to support it, we could add an additional attribute. I found that I did need it in one of the blocks, but the behavior can also be simulated with a useEffect hook in the block.

When using a side effect as part of the React component lifecycle, you are updating an attribute that might create an undo level so you need to use a special version of the store action that mitigates that. Another approach is to use a block variation that gets applied as the default one in the inserter. The benefit of that is that it sets attributes dynamically when inserting the block.

'providesContext',
'usesContext',
'supports',
'styles',
'example',
'variations',
];

const settings = pick( metadata, allowedFields );

if ( textdomain ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I checked, in the native version we set the locale data without defining a text domain, not sure if this could be a problem if in the future we move some of the fields to the block.json file to do the on fly translation.

Copy link
Member Author

Choose a reason for hiding this comment

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

default is the default text domain, so this is what I'm using in #31120. I think it should be fine.

Object.keys( i18nBlockSchema ).forEach( ( key ) => {
if ( ! settings[ key ] ) {
return;
}
settings[ key ] = translateBlockSettingUsingI18nSchema(
i18nBlockSchema[ key ],
settings[ key ],
textdomain
);
} );
}

unstable__bootstrapServerSideBlockDefinitions( {
[ name ]: settings,
} );

return registerBlockType( name, additionalSettings );
}

/**
* Registers a new block collection to group blocks in the same namespace in the inserter.
*
Expand Down