humanmade / altis-cms

CMS Module for Altis
https://www.altis-dxp.com/resources/docs/core/
46 stars 5 forks source link

Enqueued script warning for `wp-editor` on widgets screen #482

Open robindevitt opened 2 years ago

robindevitt commented 2 years ago

Steps to reproduce:

  1. Login to the site
  2. Either view the widgets page or view widgets in the customiser
  3. View query Monitor for the following error:

    wp_enqueue_script() was called incorrectly. "wp-editor" script should not be enqueued together with the new widgets editor (wp-edit-widgets or wp-customize-widgets). Please see Debugging in WordPress for more information. (This message was added in version 5.8.0.)

Expected to see: No Error

A search for wp-editor returns it in the following plugins:

Related PR's

roborourke commented 2 years ago

Need to list out all plugins that use this and carry out necessary refactoring, making reliance on wp-editor optional or conditional depending on context.

robindevitt commented 2 years ago

Starting to look into this and it appears that Yoast SEO had changes done and the only issued plugins are

robindevitt commented 2 years ago

I have dug through this and starting to pull my hair out a little.

I have figured I need to focus on a single module at a time as that makes the most amount of sense and once the solution is figured out for the one, it's easier to roll out a fix for the rest.

I have made the change manually to ElasticPress until I have the solution and can then submit an issue to the original repo.

I have gone and disabled the 3 modules : workflow, cms and analytics.

Viewing the widgets screen I no longer see the enqueued script warning for wp-editor.

I have then enabled the cms module as this module has a single instance of wp-editor and confirm the error returns when the module is enabled.

I think I was being pretty hopeful thinking to remove the wp-editor from the wp_enqueue_script array of dependants within the module would "solve" the issue to come degree, but no luck there, the issue continues to display.

Within Query Monitor it has the following for showing the error, may I be looking in the wrong spot initially?

Screenshot 2022-02-15 at 10.13.51.png

I am trying to figure out if there is something else I should be looking for that would resolve the issue.

robindevitt commented 2 years ago

Figured out cloning some of the modules into the extra-packages folder and then running composer symlink didn't link all the folders and noticeably the one I was working in

roborourke commented 2 years ago

@robindevitt might have to double check the name of the cloned repo, ensure the directory names match the <package> part of the directory names in vendor/<owner>/<package>, including lowercasing etc..

roborourke commented 2 years ago

Didn't know you could disable the CMS module tbh as that's WordPress!

From what I recall seeing there are 2 things in the CMS module that generate this warning:

Start by disabling the reusable blocks function so you can focus on one thing at a time.

Have you found anything from searching the web about this warning? We're using the hook enqueue_block_editor_assets to load the branding JS. Is there something you can do to check the current context the editor is in to determine whether the JS needs to be enqueued?

What are the differences between wp-editor and wp-edit-widgets, does the code need to be modified to use the appropriate methods or hooks from those packages depending on the context?

robindevitt commented 2 years ago

I'm disabling the cms module with the below int he config which seems to disable functionality like authorship despite it being enabled.

"cms" : {
    "enabled": false,
    "authorship": true,
}

I'm starting to looking into the differences between wp-editor, wp-block-editor and will include wp-edit-widgets.

I found that using wp-block-editor instead of wp-editor does stop the issue showing but would prefer to do a bit more research into what exactly the differences are.

Most of the research I have come across is mostly returning other support queries for plugins and nothing concrete to work from.

Based from your comment and my latest findings late the afternoon I'll dig into it a bit deeper tomorrow.

roborourke commented 2 years ago

Rather than try disabling the CMS module wholesale just switch off the sub-components that are causing the problems one by one as you go through the errors.

Does wp-block-editor provide the same components and method as the wp-editor package?

Not sure if you've seen these docs but they detail what each JS package provides:

It looks like we'll need to factor in the new edit-site and edit-navigation packages too with WP 5.9.

robindevitt commented 2 years ago

I came across this article where it says don't use @wordpress/editor, but doesn't give any feedback as to what should be used instead.

Don’t use @wordpress/editor Many legacy widgets call the wp.editor.initialize() JavaScript function to instantiate a TinyMCE editor. If a plugin or block uses the @wordpress/editor package and enqueues wp-editor as a script dependency, this will re-define the wp.editor global, often resulting in a wp.editor.initialize is undefined error.

robindevitt commented 2 years ago

I tried to tackle this issue within the AWS Analytics Plugin and got stuck, given this is a bug with a should have label, I'm going to move away from it for now.

Some notes of what I have looked into/ discoverd.

I identified all locations where the wp-editor dependency is mentioned.

I started with trying to resolve this enqueued script first.

I found the original posted error message is being generated when both wp-editor and/or wp-edit-post dependencies are used.

Removing both, breaks the A/B test functionality, with the following note:

Your site doesn’t include support for the "altis/ab-test" block. You can leave this block intact, convert its content to a Custom HTML block, or remove it entirely.

Replacing wp-editor with wp-edit-widgets and keeping wp-edit-post shows the error reported initially.

Keeping wp-edit-widgets and removing wp-edit-post, presents the following error note and the following errors in the console :

This block has encountered an error and cannot be previewed.

Screenshot 2022-02-28 at 14.40.26.png

I was using the dependencies on the widget area using the below code:

global $pagenow;

$dependencies = [
    'wp-plugins',
    'wp-blocks',
    'wp-i18n',
    'wp-components',
    'wp-html-entities',
    'altis-analytics-xb-data',
];

if ( $pagenow === 'widgets.php' ) {
    array_push( $dependencies, 'wp-edit-post', 'wp-edit-widgets' );
} else {
    array_push( $dependencies, 'wp-edit-post', 'wp-editor' );
}

wp_enqueue_script(
    'altis-experiments-features-blocks-ab-test',
    Utils\get_asset_url( 'blocks/ab-test.js' ),
    $dependencies,
    null
);

I have a feeling that we might need to have an A/B Test block for pages & posts and then another for widgets if the dependencies can't be adjusted.

roborourke commented 2 years ago

If the code has a direct dependency on functions provided by the wp-editor package (namely getCurrentPost() as indicated in the error messages above then you might be right about needing an alternative for widgets. I would aim to make the the components composable though so there's minimal rewriting to do.

Did you find out if there's an equivalent to getCurrentPost() provided by the wp-edit-widgets package? Or did you look at the code that uses getCurrentPost() to see how it might be refactored?