Gutenberg: Validation fails if attribute is persisted to post content & meta.

Created on 10 Feb 2018  路  15Comments  路  Source: WordPress/gutenberg

If a block saves markup with a representation of an attribute in HTML and that same attribute as meta, block validation fails. The meta is saved, the markup is good in post content, block validation fails.

Issue Overview

Consider this requirement for a block:
1) Must save completely valid HTML markup to post content that, when the client has no JavaScript, is totally human and machine (assistive devices, search engines, etc) readable, and can be progressively enhanced by front-end JavaScript/CSS.
2) The fields represented in that HTML markup should be queryable in the database.

Requirement 1 is provided by a Gutenberg static HTML block and requirement 2 is satisfied by the WordPress Meta API if the field is saved as post meta.

In my mind, saving an attribute as post meta, and static HTML representing its value in a semantic, human readable form is best for front-end requirements -- accessibility, no JS fallbacks, SEO, the philosophy of progressive enhancement -- and back-end needs -- we need to query by field data and update field data.

BTW I'm aware that this duplicates data. Given that we have a hook that runs after post meta is updated and we have a function for converting attributes to static HTML, that should be solvable.

Steps to Reproduce (for bugs)

  1. Register a block with a meta attribute and register the meta field
  2. Create block edit function so that you can edit attribute and see that setAttributes is updating state correctly.
    3.Create block save function that for a non-meta attribute .

Expected Behavior

No validation error.

Current Behavior

Block validation fails.

Possible Solution

I think the problem here is it's being parsed as if it's a an attribute being stored in markup/normal serialization, but I'm not sure. If so, can that be disabled in these situations?

Screenshots / Video

screen shot 2018-02-10 at 2 07 12 pm
screen shot 2018-02-10 at 2 07 30 pm

Related Issues and/or PRs

Todos

  • [ ] Tests
  • [ ] Documentation
[Feature] Blocks [Type] Bug

Most helpful comment

When will this be fixed? I mean this is some serious bug and makes meta attributes useless

All 15 comments

I created a workaround, but it involves a second "fake" content attribute, which is sub-optimal: https://gist.github.com/Shelob9/36d7096d919995ae615d4d4bbf10ffbc But, it serves as a proof of concept for this, which is cool.

The problem is that the validation is executed while parsing the blocks, and the parsing doesn't know about the post object as it's supposed to be generic enough to be used in context with post information.

We might have to rethink adding specific block attributes sources like "meta" in a generic way where we could hook into the parser, the selector, the onChange etc... and enhance how these work.

cc @aduth

We might have to rethink adding specific block attributes sources like "meta" in a generic way where we could hook into the parser, the selector, the onChange etc... and enhance how these work.

Strange coincidence, I was iterating on an idea here last night.

Approach I'd thought was a filter in getBlockAttribute which editPost adds a hook for and allows custom source: "meta" handling.

One related issue I'd come across in starting this is that many of our post selectors are in editor, which ought to be migrated (especially as we plan to expose all of them).

Noting the console warning and error in plain text for reference:

Block validation: Expected token of type `EndTag`
Block validation: Block validation failed for `shelob9/metatest`

While this is still acknowledged to be a bug, it's not one I'd considered a blocker for 5.0, given both the workaround and considering that the circumstances under which it would impact the vast majority of users approaches near impossible, if one considers that it would become plainly obvious to be broken to a block implementer in the course of minimal testing.

5077 can serve as incomplete prior art toward a possible resolution, and may in-fact tie to some potential optimizations, given that the meta attribute computation in the getBlock selector (eliminated in #5077) has shown in recent performance audits to be a contributing factor to degraded experience.

Closed #13621 as a duplicate of this one. As noted there, this manifests itself by always producing an undefined value for a meta-sourced attribute in the save of a block implementation. The reason for this is that save is called during the initial parse of a block to verify block validity, but meta values are populated only after the blocks are parsed and loaded into the editor, since they are values extracted from the post being edited.

Noting also some recent related conversation to this point / issue in #13088 (https://github.com/WordPress/gutenberg/pull/13088#issuecomment-458999774).

@aduth What is the work around to this? I'm a bit new to this, and I can't seem to figure out why my attribute isn't persisting after a page reload. It seems like initially my attribute isn't loaded, but when I delete the block and create a new one, both edit and save have the correct value.

@cbrakhage Have you looked over the code @Shelob9 had shared at https://github.com/WordPress/gutenberg/issues/4989#issuecomment-364682609 ? It seems to involve duplicating the attribute value as saved in both the markup of a block and the post meta.

When will this be fixed? I mean this is some serious bug and makes meta attributes useless

It looks like #16402 fixed this. I can't reproduce anymore.

@epiqueras If you're using the sample plugin I shared in #16402 as a base for testing, it doesn't include generated save output, so would not trigger the validation error observed here. In the sample, changing this line to return el( Stars, { stars: arguments[ 0 ].attributes.stars } ); should prompt the issue.

In order to resolve this issue, there are still changes necessary after #16402 to separate validation from the initial parse to account for the applied meta attributes.

Strange, I did change it to return props.attributes.stars.

A fix is now proposed at #16569

As of #17153, meta attribute values are no longer initialized into parsed blocks, which invalidates this approach. The decision here is that meta attributes are not meant to be used on save functions since save functions are supposed to be pure and the meta value can change dynamically outside of the block.

Was this page helpful?
0 / 5 - 0 ratings