-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Proposal: Block Params #49391
Proposal: Block Params #49391
Conversation
Size Change: +539 B (0%) Total Size: 1.34 MB
ℹ️ View Unchanged
|
Flaky tests detected in 6eb6316. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4558877573
|
There is a similar proposal (at least in terms of the 'meta' name) in Custom labelling/naming of blocks in List View. May need to figure out the right way to marry these two APIs together (or keep them apart). |
@noisysocks Just surfacing #43986 in case you missed it or it helps to inform this PR 🙇
Thanks for flagging. We split out the metadata part in the PR above. |
value={ downloadButtonText } | ||
value={ | ||
downloadButtonText ?? | ||
_x( 'Download', 'button label' ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noting that this means that the translated Download
text will no longer get serialized in the saved content. There is a hack to ensure this value gets set upon insertion using a side effect to avoid validation issues when the user changes the language. It was previously assigned to the default value wrapped with i18n helper.
Ideally, it would be a dynamic token that gets translated to a current locale on both the client and the server.
@noisysocks and I discussed the idea privately, and I wanted to summarize the talking points here. We definitely need something like that. It’s a great starting point and it should work in most cases based on a quick assessment. A slightly different approach to the same problem worth exploring is to use a proxy wrapper for the value passed when creating a block: const block = createBlock( { // passing an object to access advanced use-cases
name: 'core/file',
attributes: {
fileName: file.name,
blobURL: transientAttributeValue( createBlobURL( file ) ),
},
} ); Behind the scenes we could wrap the value with a proxy object like
@noisysocks, expaned on the idea with the following use cases:
We need to discover shortcomings though. There are no perfect solutions 😅 |
Looking at these use-cases. I have some questions:
|
It's all about the values of the attribute at a given time. An Image (File – from the code example, too) block is a great example. When you drag and drop an image from the filesystem, the block editor shows the in-memory version, while uploading it to the server, to improve user experience. This value for the image is local to the machine and can't be accessed by anyone else, so we don't want to serialize it in the saved content, nor keep it in the undo/redo history. We only care about the uploaded URL that gets set as a value when the whole process end. In effect, there isn't a simple way to encode this information in the block attributes definition. |
One potential solution could be to keep the temporary value in a separate attribute (transient one) and have the block fallback to that one when defined. It feels acceptable to me. |
The two disadvantages I see with making "transient" a property of the attribute definition (in
I don't know if it's of any actual practical concern but a nice feature about the approach in this PR as opposed to I think the three issues I fix in this PR as well as the Pay with PayPal case in #29693 are representative of the problem space and so what I'll do is open a few different PRs that attempt to solve them with different approaches. Then we'll see what we learn.
|
It's worth reminding that this exact notion of "transient" attributes was explored back in 2018. The closest PRs are #11504 and #12724 and the proposals were hardly consensual. But mixing transients into regular attributes — something I suggested back then — is something I'm very reticent about now. Like Grzegorz, at the time I was envisioning a broader solution to accommodate not just the instantiation of blocks (i.e. using a seed value like However, this was before the introduction of entities and the accompanying overhaul of undo/redo (2019). In this new paradigm, we added actions like |
I'm leaning towards the idea shared by @youknowriad where the transient value should be represented as a separate attribute that is never persisted by the block editor. It's a very simple way to apply changes to the existing blocks that use transient values for showing media previews as exercised in #49457. In addition to that, it's a minimal change that adds only a flag to the definition of attributes which is the most convenient way for block developers. The API changes necessary in this PR would be more complex and difficult to explain in the documentation. |
In that case I think we should do a formal comparison with previous attempts (#11504 and #12724) to understand what changed (are there new circumstances? or is the new solution different in a meaningful way?). |
What & why
This PR experiments with adding
params
to the block API.The
params
object is likeattributes
in that it is passed to the block'sedit
functions for the lifecycle of that block. Unlikeattributes
, though,params
is:setParams
passed toedit
.params
is not written to the database and does not appear in the markup returned fromserialize( block )
.save
. This is to prevent block validation / deprecation issues.If you consider that blocks are kind of like React components, then you can think of
attributes
as state andparams
as props. It's a way of passing data to a block without affecting its state.The motivation for this is to fix a longstanding issue we've had in Gutenberg which is that we use attributes to store "internal" state in certain blocks. For example an undo level is created when dragging-and-dropping an image or file into the editor because the blob URL is stored in the
url
attribute while the file is uploaded. I've included a fix for the File block (61ad707) to illustrate howparams
helps.I've also included fixes for two other issues to illustrate that there are many uses for such an API:
__internalWidgetId
attribute in favour ofparams
.__internalWidgetId
is how the widget editor tracks which widget entity should be updated when a block is modified. It's a hack that works because__internalWidgetId
does not appear in theblock.json
and so is filtered out byserialize()
.params.menuId
. See Widget Importer: ensure it works with importing menus #47285.Alternatives
One alternative would be to use attributes but mark them as "private".
The downside of this is that implementation details appear for all to see in
block.json
.