WordPress / gutenberg

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

enqueue_block_assets hook loads scripts twice #53590

Open wpsoul opened 1 year ago

wpsoul commented 1 year ago

Description

Latest update gives us option to load some scripts and styles in Iframe, which is great.

The problem is that it's loading everything twice in the Site editor - outside iframe and inside iframe. This makes some scripts execute twice and break them

Is it possible to load only in an iframe on the Site editor? I think it can be correct if we check if current page has iframed content and if yes, then we load only in an iframe, if not, we load outside iframe

Step-by-step reproduction instructions

  1. Use enqueue_block_assets and add wp_enqueue_script
  2. Open Site editor
  3. Check the page source code. The script is added to the page content.
  4. Add console.log('test') to script
  5. We see that it's triggered twice on the Site editor pages

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

jorgefilipecosta commented 1 year ago

Hello @wpsoul, your diligence in bringing up this behavior is appreciated.

I've successfully replicated the behavior. Here's the code snippet I used:

function test_script(){
    wp_enqueue_script(
        'foo',
        get_template_directory_uri() . '/test.js',
        array(),
        '1.0.0',
    );
}

add_action( 'enqueue_block_assets', 'test_script' );

test.js file:
alert('test');

The alert occurs twice – once inside the iframe and once outside.

I'm uncertain if it's viable to exclusively execute block asset scripts within the iframe, considering that some might interact with the editor. @ellatrix, what are your thoughts? Would it be plausible to confine the script execution to the iframe?

ellatrix commented 1 year ago

Yes, this is expected. What script are you loading in the iframe that can only be executed once?

wpsoul commented 1 year ago

@ellatrix

I think all scripts must be executed only once on page. Otherwise, highly likely they will have conflicts and bugs. At least, we must have hook which will work only in iframe (like we have now enqueue_block_editor_assets which works outside Iframe)

ellatrix commented 1 year ago

It is executed once per page: the iframe is a completely different document, a bit like a preview tab. That said, you are correct that enqueue_block_assets shouldn't be loaded in the editor if there's an iframe, it shouldn't be necessary. Right now it's there for backward compatibility, but we can check how much it is needed and for what. We could consider a compat script to clone styling if we detect editor UI selectors, like we do for enqueue_block_editor_assets in the opposite direction. For scripts it's harder. If plugins are currently relying on it for something in the top window (editor UI), then things will break. They shouldn't be using enqueue_block_assets, but it worked in the past because there was no iframe boundary.

ellatrix commented 1 year ago

So for these reasons I am curious why it's a problem. What is your script doing?

wpsoul commented 1 year ago

@ellatrix I use GSAP library for animations. And yes, it's a problem, because I found very strange behaviour. First, script library is loaded in iframe and executed. Then, if I trigger something from block, script library which is outside iframe is used (I guess, it's because block's scripts are loaded outside iframe), it gives me double execution and wrong animations.

Why we don't have hook for iframe? I think it's logical, because Iframe has another window, using ref and ownerWindow doesn't help because in current point, when we use this, we point to element inside Iframe, but library is loaded outside iframe and this makes conflicts.

They shouldn't be using enqueue_block_assets

Yes, but in current point we don't have anything other to load scripts inside iframe

ellatrix commented 1 year ago

Unfortunately that library seems to be closed source, so I cannot test. Is the problem merely loading the library script, or are you doing some initialisation (your script) twice inside and outside the iframe? If it's initialisation, you could check for the presence of body.editor-styles-wrapper in the window and only initialise if that element element exists?

I will look into removing enqueue_block_assets from the top window (outside the iframe), but that doesn't help you right now of course.

wpsoul commented 1 year ago

@ellatrix GSAP is free library, only some addons are premium. Problem is that scripts are loading twice. Inside block I use ref, then detect window and document from ref and send it to script. Everything is working but library has also Scroll features and detecting window object is calculated in core files, so I can't send it to script. Also, script is executed twice on page.

Is any technical problem to add hook which is working only in iframe?

github-actions[bot] commented 1 year ago

Help us move this issue forward. This issue is being marked stale since it has no activity after 15 days of requesting more information. Please add info requested so we can help move the issue forward. Note: The triage policy is to close stale issues that need more info and no response after 2 weeks.

ellatrix commented 1 year ago

@wpsoul I created https://github.com/WordPress/gutenberg/pull/54250. Does that work for you?

github-actions[bot] commented 11 months ago

Help us move this issue forward. This issue is being marked stale since it has no activity after 15 days of requesting more information. Please add info requested so we can help move the issue forward. Note: The triage policy is to close stale issues that need more info and no response after 2 weeks.

wpsoul commented 11 months ago

@ellatrix Hi. Thank you for PR. Maybe I am testing it in wrong way, but what PR does - it removes everything which is registered via enqueue_block_assets. Currently I can't see scripts in iframe and in not iframed editor

github-actions[bot] commented 11 months ago

Help us move this issue forward. This issue is being marked stale since it has no activity after 15 days of requesting more information. Please add info requested so we can help move the issue forward. Note: The triage policy is to close stale issues that need more info and no response after 2 weeks.

wpsoul commented 9 months ago

@ellatrix I think in 6.4 this is not fixed, still see scripts outside iframe

github-actions[bot] commented 9 months ago

Help us move this issue forward. This issue is being marked stale since it has no activity after 15 days of requesting more information. Please add info requested so we can help move the issue forward. Note: The triage policy is to close stale issues that need more info and no response after 2 weeks.

DamChtlv commented 8 months ago

This issue shouldn't be closed, it's still a problem. I saw the PR but it doesn't seems correct as mentionned.

Couldn't we use a new conditional helper like is_admin() which could be is_editor() maybe and only be true when inside the iframe context? _(or maybe a new parameter of enqueue_block_assets action)_ This way, we could use enqueue_block_assets action easily to load only style / script inside or outside iframe. Something like:

add_action( 'enqueue_block_assets', function( $editor_iframe ) {
    if ( $editor_iframe ) {
        wp_enqueue_style( 'my-iframe-style' );
    }
    // or
    if ( is_editor() ) {
        wp_enqueue_style( 'my-iframe-style' );
    }
}
ellatrix commented 5 months ago

There's a PR open that isn't merged yet, I will need to have another look at what the blockers where, and it needs testing.

CreativeDive commented 4 months ago

Because I don't understand it exactly, is there a reason why block assets are loaded in the main frame and iframe? I would say no. When the editor iframe is loaded, block assets should only be loaded there and not a second time in the main frame, because they play no role there. Every time I debug scripts, I have the console logs twice, which is a bit annoying.

CreativeDive commented 4 months ago

We have two types of block assets that are loaded in the frontend and editor “script” and “style” according to block.json, right? However, enqueuing is handled via block.json, so we have no chance of changing this behavior.

CreativeDive commented 4 months ago

And if you want to add styles/scripts (NOT block type related) using add_action( 'admin_enqueue_scripts' then you get a message like this:

Bildschirmfoto 2024-04-26 um 11 12 57

This makes no sense to me, because I want to add admin scripts/styles which should be affect the whole screen, not the iframe document and not blocks only.

nextend commented 3 weeks ago

This is what worked for me:

<?php

function enqueue_my_block_assets() {

    wp_enqueue_style('my-style', 'src');
}

add_action('enqueue_block_assets', 'enqueue_my_block_assets');

add_action('admin_enqueue_scripts', function () {
    remove_action('enqueue_block_assets', 'enqueue_my_block_assets');

    //wp_dequeue_style('my-style');
    wp_deregister_style('my-style');
});