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 Validation - Replace with expected #10444

Closed
puggan opened this issue Oct 9, 2018 · 27 comments
Closed

Block Validation - Replace with expected #10444

puggan opened this issue Oct 9, 2018 · 27 comments
Labels
[Feature] Extensibility The ability to extend blocks or the editing experience [Feature] Paste [Type] Question Questions about the design or development of the editor.

Comments

@puggan
Copy link

puggan commented Oct 9, 2018

Is your feature request related to a problem? Please describe.
When developing a new block, every change to it make the existing block on my test page invalid.
Here i get 2 option, keep as html, or convert to (default)-blocks.

Describe the solution you'd like
I'd like a 3th option, to keep the expected.
It's marked as invalid as the saved-html and rendered-html missmatch.
As a developer I'm not intrested in the saved-html, just let me use the new genereated html,
without having to remove the box, and re-add all content.

Describe alternatives you've considered
Opening the javascript-console, copy pasting the expected and current values, printed there, and do a replace in the database.

@braco
Copy link

braco commented Oct 10, 2018

YES!

CMS developers in corporate environments don't want the user to be overriding HTML at the expense of a site-wide change.

There should be a strict mode to a module that always overwrites the user's changes or unexpected results.

The attributes should be able to serve as the source of truth, not the HTML.

@puggan
Copy link
Author

puggan commented Oct 10, 2018

To implement a side-wide change in that manner, you can use the the migration feature, to go from one existing version, to a newer version. My case was more in the development stage, between thus 2 versions.

@braco
Copy link

braco commented Oct 14, 2018

It's the same problem. There should an attribute-first setting that would take care of both problems – the attributes in this case are critical, the HTML is secondary. The fact that minor HTML changes break the CMS is something that needs to be addressed, as it's protecting one use case at the expense of many others.

@designsimply designsimply added the Needs Technical Feedback Needs testing from a developer perspective. label Oct 16, 2018
@designsimply
Copy link
Member

I'd like a 3th option, to keep the expected.
It's marked as invalid as the saved-html and rendered-html missmatch.

To clarify, you are saying you want an option to keep unsupported HTML changes and render them anyway? My understanding is that works against one of the principles of Gutenberg which is to keep the HTML very clean and to use the Custom HTML block or the Classic block otherwise.

As a developer I'm not interested in the saved-html, just let me use the new generated html,
without having to remove the box, and re-add all content.

Could you talk a little bit more about a specific use case, maybe include some steps with example code to show the problem a little more clearly? I am working to help test and clarify issues and I think a specific example here might make the issue more actionable or allow someone to offer another solution for your use case.

@designsimply designsimply added the [Status] Needs More Info Follow-up required in order to be actionable. label Oct 16, 2018
@braco
Copy link

braco commented Oct 16, 2018

@designsimply: there needs to be a setting to defer to a Gutenberg block's layout assertions rather than the user. It was a strange decision to treat React-driven markup changes like a fatal error, forcing the user to deal with "an external source has changed". This only really makes sense in the context of a user installing third party plugins, but surely the vision for WP is larger than that?

There are many use cases, like puggan's, when you want to either rapidly iterate in your development (deferring to the React component's changes) or in a controlled/corporate CMS environment, where you want to retain the attributes and alter the layout in some superficial way. To have to deprecate a block just to make a small change is not realistic for many workflows.

This problem is bad enough that developers are already trying to find ways to circumvent the mechanism by rendering the website based on the attributes, skipping the save function entirely.

@chrisvanpatten
Copy link
Member

This shouldn't be considered a solution, but just as a recommendation: when developing blocks I find it's most efficient to save null and render via a render_callback until I'm confident with my desired markup.

I do agree, we will need a better way to iterate on the saved markup of a block in the long term. Deprecated handlers are a start but it's not possible to apply a deprecated handler automatically to an entire corpus of content in a database. Hopefully a future server-side parsing API would make that possible.

@braco
Copy link

braco commented Oct 17, 2018

Thanks @chrisvanpatten, that's helpful.

Is any consideration bring given to making an attribute-first mode for Gutenberg blocks? Beyond development, this would always overwrite the user's HTML, with intention, while preserving their attributes. This would probably be paired with { html: false } in the block settings.

There are entire companies built around this use case, like contentful.com.

@blindstuff
Copy link

Agree with @braco

CMS developers in corporate environments don't want the user to be overriding HTML at the expense of a site-wide change.

The attributes should be able to serve as the source of truth, not the HTML.

This seems like such a great idea to me. Many times users want a small change in style/layout for a block that can be used throughout a large portion of the site without replacing the block.

@chrisvanpatten
Copy link
Member

@braco @blindstuff I think for you both I'd recommend using dynamic blocks over static blocks for these cases, with render_callback happening on the server. You can still have an inline edit representation of your block rendering with React in real time in the editor, but your save would be null and ultimately you'd use PHP to render the block content.

For blocks with markup that's likely to change, this is a good way to decouple post_content and your block's rendering.

@blindstuff
Copy link

@chrisvanpatten Thanks for the suggestion. Will look into it.

@chrisvanpatten chrisvanpatten changed the title Block Validation - Replace with expectd Block Validation - Replace with expected Oct 29, 2018
@hos-shams
Copy link

Couldn't be more agree with @braco

The attributes should be able to serve as the source of truth, not the HTML.

We constantly need to change the HTML code. There might be a responsive issue or we need to simply add a new feature which requires change the markup. Adding a new entry to the deprecated array is a nightmare.

Even there's a worse scenario. Imagine all of the blocks from a theme have a <Wrapper> component (which in my case I have). And it renders something like this:

<div class="wrapper">
  <Video />
  <Overlay />
  <div class="container">
    { props.children }
  </div>
</div>

What if I want to add a <ParallaxImage /> component to the .wrapper element? Or simply add another class to the .container or add a data-something="" to the .wrapper tag? These changes make all of the existing blocks invalid while they're actually valid from the developer perspective and can be rendered with current attribute values. That's impossible to update all the existing blocks and add a deprecated version.

So I suggest let an individual block to opt-out from the validation check. Maybe add a skipValidation to supports object of registerBlockType.

@chrisvanpatten @designsimply would it be doable?

@roborourke
Copy link
Contributor

roborourke commented Nov 16, 2018

This would be super nice to have specifically for the development experience. I noticed the new "Resolve" button in recent versions but it doesn't seem to do anything when I click it.

I know @chrisvanpatten's suggestion will work as far as the initial development might go but when you have to come back to a block on a site that's already got content you certainly don't want to be adding an entry to deprecated every time you make a change. Plus on the point of using server side / dynamic blocks that not necessarily ideal either if the content you'd be generating is important for search.

Anyway, some sort of constant - maybe just using SCRIPT_DEBUG as a flag that lets me keep the latest version of block output would be delightful.

@designsimply designsimply added [Type] Question Questions about the design or development of the editor. [Feature] Extensibility The ability to extend blocks or the editing experience [Type] Help Request Help with setup, implementation, or "How do I?" questions. and removed [Status] Needs More Info Follow-up required in order to be actionable. labels Nov 30, 2018
@maxxwv
Copy link

maxxwv commented Dec 1, 2018

@braco @blindstuff I think for you both I'd recommend using dynamic blocks over static blocks for these cases, with render_callback happening on the server. You can still have an inline edit representation of your block rendering with React in real time in the editor, but your save would be null and ultimately you'd use PHP to render the block content.

I agree with @braco about the attributes being the source of truth, and asking a developer to make the edit functionality/admin display live in JavaScript while the front-end display and functionality lives in PHP is not a good way to do it. The other thing this affects is output formatting and internal data save formatting.

In my case, I've created an floor plan block - it includes many number fields, including square footage. I'd like to present it to the user as 2,400, but when I attempt to add a value of Intl.NumberFormat().format(sqFootage) in the save() function, the output doesn't match the stored attribute value of 2400 and the block barfs. I shouldn't be saving formatting delimiters in the block attributes, and I shouldn't have to use a regex to strip formatting delimiters in the edit() function's onChange() method before I call setAttributes().

Please also note that this is related to #5872 - closed as of yesterday, but I'm still experiencing it today. Because the RichText component adds a <br> element in the output, the validation fails and my block barfs.

@braco
Copy link

braco commented Dec 8, 2018

@braco @blindstuff I think for you both I'd recommend using dynamic blocks over static blocks for these cases, with render_callback happening on the server.

@chrisvanpatten I had a chance to look at this today – I don't think this is a good solution. It takes rendering away from Javascript/React and pushes it into PHP. Now you'd have two different renders that are supposed to be maintained in parallel? I guess you could write the attributes out in block-name:{JSON} format and let your front end scan for that, JSON.parse() the content and re-render the React component, but yikes

@skorasaurus
Copy link
Member

This is unfortunately a longtime issue that many others have raised (#7604 and #6925 have linked to it).

As a relatively inexperienced developer, I've found this experience (of trying to handle validation of changes to our blocks) to a bit discouraging and overbearing as I and others constantly iterate their blocks to have an immediate understanding of how changes in our code affect its output in Gutenberg.

@lattam
Copy link

lattam commented Jan 30, 2019

I would really appreciate some mode/option without validation. The development of custom blocks is starting to be a nightmare. I have a website with several custom block and even the smallest change destroys the whole layout. Every time I reload the page I'm praying not to get the error "This block has encountered the error and cannot be preview" of death. I don't know what I'm doing wrong. I am aware of deprecated option but I'm not sure I want to write migration every week when we need to change one small thing.
Thank you for your consideration.

@roborourke
Copy link
Contributor

Just to add it might be an idea to factor in the attribute source for block validation - if all attributes are stored in the comment delimiter rather than in the markup or meta etc then the markup output isn't going to be that important.

@designsimply designsimply added [Feature] Paste and removed Needs Technical Feedback Needs testing from a developer perspective. [Type] Help Request Help with setup, implementation, or "How do I?" questions. labels Feb 12, 2019
@youknowriad
Copy link
Contributor

Closing as a duplicate of #7604

We're interested in improving these flows but this is a hard problem and I haven't seen good proposals so far.

@lastant
Copy link

lastant commented Jul 1, 2019

@youknowriad just add an option say validate (set to true by default) to the register_block_type() and/or window.wp.blocks.registerBlockType() and do the validation based on this value. What is so complicated about this issue...

@youknowriad
Copy link
Contributor

That's not complex but is this what the user wants? If we add this API, I'm certain it will be abused at the expense of the user sometimes (changing the content of blocks without notice). Not saying it shouldn't be added but that's something to consider.

@spwebguy
Copy link

spwebguy commented Jul 4, 2019

That's not complex but is this what the user wants? If we add this API, I'm certain it will be abused at the expense of the user sometimes (changing the content of blocks without notice). Not saying it shouldn't be added but that's something to consider.

It's the same with plugins. You could literally change the output content of any shortcode or worst. There are always abusers, plus the current way of doing things probably isn't preventing every bad uses of the API anyway.

Also, having their content removed because of invalidation is at the expense of the user. It's too fragile I think. But then it's only my opinion, and I have no clear proposal.

@vladoa
Copy link

vladoa commented Dec 13, 2019

Soo what's the status now?
I have a custom block and I'd want to be able to safely add a new CSS class or change the markup without fearing that it will break hundreds of pages.
What is the easiest way to go?

@eliot-akira
Copy link

@vladoa For now, it's recommended to use dynamic blocks, as per @chrisvanpatten 's comment.

I'd recommend using dynamic blocks over static blocks for these cases, with render_callback happening on the server. You can still have an inline edit representation of your block rendering with React in real time in the editor, but your save would be null and ultimately you'd use PHP to render the block content.

@jameskhamil
Copy link

jameskhamil commented Dec 13, 2019

Only WordPress could offer a modern React interface and then make you duplicate it in parallel with php, lol. Rube Goldberg machine.

@blindstuff
Copy link

Only WordPress could offer a modern React interface and then make you duplicate it in parallel with php, lol. Rube Goldberg machine.

It may sound silly, but our team chose to scrap blocks and return to regular implementation in WP because of this.

@badererliebert
Copy link

badererliebert commented Mar 20, 2024

Agreed. I don't want to use a dynamic block because I want to be saving static content, as an issue of page render speed and processor expense. If I am developing against static content, I don't want to use a dynamic block and call a PHP subroutine unnecessarily. Why should I be forced into a development flow that's different than my end use case, and then forced to refactor later, simply because the site has determined that the block changed? Yes, the block changed, I changed it. Delete what was there, and replace it with what is there. There really shouldn't be anything else to it than that. Super frustrating.

@badererliebert
Copy link

Previous comment stands, although I did find a workaround for development if anyone else continues to have this problem. I realized that when I develop blocks for headless sites it's not an issue since I'm not saving anything into the post body. So all I did for developing the block is deactivate the save function by commenting out the post save stuff and replacing it with return { ...useBlockProps.save() };. Now Wordpress doesn't see any differences in what's being saved to the html and you can develop your React component in peace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Extensibility The ability to extend blocks or the editing experience [Feature] Paste [Type] Question Questions about the design or development of the editor.
Projects
None yet
Development

No branches or pull requests