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

Issues with settings spread and translations #166

Closed
ryanwelcher opened this issue Oct 20, 2021 · 5 comments · Fixed by #165
Closed

Issues with settings spread and translations #166

ryanwelcher opened this issue Oct 20, 2021 · 5 comments · Fixed by #165
Labels

Comments

@ryanwelcher
Copy link
Contributor

ryanwelcher commented Oct 20, 2021

All of the examples of registering blocks use the following approach to registering blocks.

import json from '../block.json';
import edit from './edit';
import save from './save';

const { name, ...settings } = json;

registerBlockType( name, {
	...settings,
	edit,
	save,
} );

It was pointed out in this comment that spreading the settings object is not technically required as this is done on the server-side for blocks registered. It should be used to override those server-side values or in the case of registering a JavaScript-only block.

Based on a conversation with @gziolo it also seems that if the settings are passed as they are defined in block.json ( as in the approach above ) any translations will not be processed and would need to be provided in that object.

@gziolo
Copy link
Member

gziolo commented Oct 21, 2021

For full clarity. There is an alternative way to ensure that i18n handling gets applied on the client and the code would look like the following:

import json from '../block.json';
import edit from './edit';
import save from './save';

registerBlockType( json, {
	edit,
	save,
} );

However, the main point still is true. When the block gets registered on the server, there is no need to override the same settings. In fact, when some hooks would get applied on the server, their changes would get lost.

@ryanwelcher
Copy link
Contributor Author

For full clarity. There is an alternative way to ensure that i18n handling gets applied on the client and the code would look like the following.

Perhaps I am mistaken, but isn't the first param of registerBlockType the name and the second represents the settings?

@gziolo
Copy link
Member

gziolo commented Oct 22, 2021

For full clarity. There is an alternative way to ensure that i18n handling gets applied on the client and the code would look like the following.

Perhaps I am mistaken, but isn't the first param of registerBlockType the name and the second represents the settings?

It has changed in WordPress 5.8:

https://github.com/WordPress/gutenberg/blob/53374ba92c379480659a7df5ac456756ad3a798c/packages/blocks/src/api/registration.js#L220-L230

It was necessary for the native mobile apps that process block types only with JavaScript for now.

@ryanwelcher
Copy link
Contributor Author

Ah! I wasn't aware of that. Do we think this will change again? If we think it will, perhaps we keep with the current implementation of passing name and settings as different parameters. My concern is that introducing this in the official examples will add more confusion as this approach is not documented anywhere else. It may be worth mentioning somewhere, but IMO I don't think we should add it here.

@gziolo
Copy link
Member

gziolo commented Oct 22, 2021

I can see how it can be confusing at times to pass an object as first param. In general, it should be name and settings in 99% of cases. So all good in the existing examples.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants