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

Block API: Add parse options to specify custom source handling #16316

Closed
wants to merge 2 commits into from

Conversation

aduth
Copy link
Member

@aduth aduth commented Jun 26, 2019

Related: #16282, #5077
Fixes #4989

This pull request seeks to explore a small part of proper handling of custom sources as part of the parse stage of editor initialization. Specifically, the changes here are alone sufficient to resolve the issue described in #4989. With these changes, a new options object can be passed with wp.blocks.parse to control behavior of the attributes derivation. Unlike #5077, this is exposed as part of the top-level API rather than as a hooks filtering. Hooks could still serve this purpose, but as of latest consideration are being discouraged, and could work unpredictably in nested editor environments.

In-Progress Notes:

  • Needs unit tests
  • There is another direct parse which occurs in the code editor (source) which should be consolidated

Effectively, I'm not too committed to the direction here, and am opening to solicit some early feedback in considering whether to continue as a solution.

@aduth aduth added [Feature] Block API API that allows to express the block paradigm. [Feature] Parsing Related to efforts to improving the parsing of a string of data and converting it into a different f labels Jun 26, 2019
@aduth aduth requested a review from youknowriad June 26, 2019 19:33
Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

While we are on early stages I guess this API could be a basis for a more high-level set of sources e.g: a WP REST API source (that retrieves attributes from a rest API) a post source that retrieves attributes from the post itself etc.

I'm curious to see how we can expand this basis to support async attribute fetching. It would also be important to check how the attribute saving will work. Here we are dealing with how we retrieve a given attribute but that attribute may be changed by the block. The custom attribute sources also need to define a storing mechanism, I guess we want the way to defined how the attribute is read to be similar to the way we defined how the attribute is stored so ultimately the custom source storing API may impact this API.

/**
* Options for parse.
*
* @property {Object<string,Function>} sources An object defining custom source
Copy link
Member

Choose a reason for hiding this comment

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

Hi @aduth,

the value a function receiving an attribute schema and expected to return the desired value.

Would it make sense for the function to also receive the innerHTML plus the commentAttributes?

Imagining the following scenarios:
- an attribute that is derived from the text in a given HTML element.
- A product block that contains the id of the product as a comment, and we want to query some attributes of that product using an external REST API.

@@ -59,7 +59,11 @@ export function* setupEditor( post, edits, template ) {
content = post.content.raw;
}

let blocks = parse( content );
let blocks = parse( content, {
Copy link
Member

Choose a reason for hiding this comment

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

Is there any plan to allow, plugin developers to create custom sources and use them in their blocks?

@@ -251,6 +273,11 @@ export function getBlockAttribute( attributeKey, attributeSchema, innerHTML, com
case 'tag':
value = parseWithAttributeSchema( innerHTML, attributeSchema );
break;

default:
if ( has( parseOptions.sources, attributeSchema.source ) ) {
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to allow an async retrieval of the value? Imagining cases where external requests are required to retrieve the value. I guess it would increase the overall complexity of the module.

@aduth
Copy link
Member Author

aduth commented Jul 5, 2019

Thanks for taking a look @jorgefilipecosta !

It would also be important to check how the attribute saving will work. Here we are dealing with how we retrieve a given attribute but that attribute may be changed by the block.

Yeah, in reflecting on this after the initial publication, I think a major downside to this approach is it breaks down the "cycle" or idempotency of block parsing vs. serialization.

The mechanisms around how a custom source might save are being explored further in #16402, although admittedly meta are one of the simpler examples since they can leverage existing editor functionality around updating post properties.

I don't know that I'm likely to pursue this approach much further. With #16402, we might instead be able to do something where we "defer" the validation to occur only after meta values have been applied. I expect it might look similar to what we have here so far as an options object for parse, and basically changing this line:

let blocks = parse( content );

...to:

let blocks = parse( content, { validate: false } );

(The default would be true to maintain backwards compatibility)

Then, we can expose a function to manually run validation over a set of blocks.

@aduth
Copy link
Member Author

aduth commented Jul 12, 2019

Closing in favor of #16569

@aduth aduth closed this Jul 12, 2019
@youknowriad youknowriad deleted the try/parse-sources branch July 15, 2019 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block API API that allows to express the block paradigm. [Feature] Parsing Related to efforts to improving the parsing of a string of data and converting it into a different f
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validation fails if attribute is persisted to post content & meta.
2 participants