WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.51k stars 4.2k forks source link

unregisterBlockType not working on domReady and never for core-embed blocks #27607

Open markhowellsmead opened 3 years ago

markhowellsmead commented 3 years ago

Describe the bug The documentation indicates that we should use the following code to unregister blocks.

import { unregisterBlockType } from '@wordpress/blocks';
import domReady from '@wordpress/dom-ready';

domReady(function () {
    unregisterBlockType('core/verse');
});

This has no effect. Using the same function on window load works, however. This has been the case for at least the last couple of Core releases, if not more. (Sorry, I have no statistics for older Core versions.)

import { unregisterBlockType } from '@wordpress/blocks';

window.addEventListener('load', function () {
    unregisterBlockType('core/verse');
});

Additionally, using unregisterBlockType for core-embed block types never has any effect.

unregisterBlockType('core/verse'); // works
unregisterBlockType('core-embed/twitter'); // doesn't work

To reproduce Steps to reproduce the behavior:

  1. Use the code above
  2. Try to insert a verse or Twitter embed block

Expected behavior

Editor version (please complete the following information):

Desktop (please complete the following information):

talldan commented 3 years ago

@markhowellsmead The embed block subtypes are now registered using block variations. Something like wp.blocks.unregisterBlockVariation( 'core/embed', 'twitter' ) should do the trick: https://developer.wordpress.org/block-editor/packages/packages-blocks/#unregisterBlockVariation

markhowellsmead commented 3 years ago

Thanks @talldan. Using unregisterBlockType on 'core/embed' also seems to work, removing all embed blocks. The problem with the event timing remains.

markhowellsmead commented 3 years ago

I case anyone else comes across this issue: using unregisterBlockVariation to remove all of the core/embed block variations still allows the user to paste in a URL, which is then converted automatically by the root core/embed block. (The base variation in the core/embed block cannot be unregistered using unregisterBlockVariation.)

E.g. if the Twitter block variation is unregistered, the user can still add a Twitter embed to the site. In order to avoid all embeds, you need to use unregisterBlockType on the entire core/embed block.

talldan commented 3 years ago

Right—you could probably use a php function to remove support for the oembed provider in addition to the variation if you just wanted to remove one type of embed completely. Maybe this function: https://developer.wordpress.org/reference/functions/wp_oembed_remove_provider/

markhowellsmead commented 3 years ago

Ah, there's a good idea!

ptbello commented 3 years ago

The bug also applies to unregisterBlockStyle since the root cause is @wordpress/dom-ready not working properly any more. Thanks @markhowellsmead for pointing out the window.addEventListener('load', function () {}) workaround

markhowellsmead commented 3 years ago

Concur with @ptbello: I also use the same load event listener for unregisterBlockStyle.

ptbello commented 3 years ago

@markhowellsmead actually I just switched to document.addEventListener('DOMContentLoaded', function () {}) which is slightly faster and avoids the "flash of unregistered block styles"

markhowellsmead commented 3 years ago

Me too, now ;) I'm guessing that domReady should essentially be the same as DOMContentLoaded.

strarsis commented 3 years ago

Related/very similar issue: https://github.com/WordPress/gutenberg/issues/25330

kubiqsk commented 2 years ago

actually, everything works on wp.domReady but it's very important to use correct dependency Currently it's a little bit tricky, like:

$dependencies = array( 'wp-blocks', 'wp-dom-ready' );
if( is_object( get_current_screen() ) ){
    if( get_current_screen()->id == 'site-editor' ){
        $dependencies[] = 'wp-edit-site';
    }elseif( get_current_screen()->id == 'widgets' ){
        $dependencies[] = 'wp-edit-widgets';
    }else{
        $dependencies[] = 'wp-edit-post';
    }
}else{
    $dependencies[] = 'wp-edit-post';
}
wp_enqueue_script(  'xxx', plugins_url( 'js/script' . ( defined('SCRIPT_DEBUG') && SCRIPT_DEBUG ? '' : '.min' ) . '.js', __FILE__ ), $dependencies, 1, false );
markhowellsmead commented 2 years ago

This remains an issue. The suggestion by @kubiqsk works.

markhowellsmead commented 1 year ago

This still isn't working without the non-standard workaround.

onetrev commented 1 year ago

EDIT -> DO NOT USE SUGGESTION BELOW! It breaks stuff.

One thing I wish @ryanwelcher that was better documented is that if you want to unregisterBlockType to take effect in the Site Editor (which if you are removing styles elsewhere I'd think you'd want it removed here too) then you must also add wp-edit-site to the list of dependencies.

In other words you need something like this, which is taken from the Block Editor Handbook, but with wp-edit-site added:

function myguten_enqueue() {
    wp_enqueue_script(
        'myguten-script',
        plugins_url( 'myguten.js', __FILE__ ),
        array( 'wp-blocks', 'wp-dom-ready', 'wp-edit-post', 'wp-edit-site' ),
        filemtime( plugin_dir_path( __FILE__ ) . '/myguten.js' )
    );
}
add_action( 'enqueue_block_editor_assets', 'myguten_enqueue' );
markhowellsmead commented 8 months ago

This issue appears to be resolved in the Block Editor, but still does not work in the Site Editor. cc. @WordPress/outreach.

markhowellsmead commented 8 months ago

My dependencies are defined by the Dependency Extraction Webpack Plugin. It seems unnecessarily complicated to have to break into this logic manually.

kraftner commented 8 months ago

Also I can confirm it is very finicky because of load order, loading speed, cache and whatnot. It works sometimes, and sometimes not. Especially when testing on local it works more often because network is instant, but on live it randomly breaks.

For me the very ugly hack of sticking with domready() but then adding a 200ms timeout has reliably worked for years. :scream_cat:

Unfortunately I also wasn't able to reproduce the core issue in a reliable, testable way. Hence I haven't added a comment here yet although lingering for years. But now that "this is solved" lies in the air I just felt like I had to say something. Although I understand if I'll be ignored since, like I said, I am not (yet) ably to reproduce this in a reliable, testable way.

swissspidy commented 8 months ago

Agree with Mark here. The default experience when using the recommended workflows & following regular documentation is not great. And just importing something from @wordpress/block-editor to make this work isn't a nice workaround.

onetrev commented 8 months ago

I'm back-flipping! I agree, this actually isn't solved. Also, disregard my tip above, you should NOT add wp-edit-site to your list of dependencies. For one it breaks stuff / causes weird glitches in the Block Editor. For one, the "Apply Styles Globally" UI appears unexpectedly (and not displayed properly) in the Block Editor if you do. I gave bad bad advice above. 🤦

Screenshot 2024-03-05 111945

The actual workaround is to check if you are in the Block Editor or Site Editor as suggested in this comment. I agree with @swissspidy this is still an overly complicated fix. Probably the better option, but still not straightforward is this workaround by @t-hamano.

It's also feels like this is a bug when your Block Variations added with the exact same code (dependencies) works in the Site Editor, whereas your Block Styles do not.