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

enqueue_block_assets hook loads scripts twice #53590

Open
wpsoul opened this issue Aug 11, 2023 · 20 comments · May be fixed by #54250
Open

enqueue_block_assets hook loads scripts twice #53590

wpsoul opened this issue Aug 11, 2023 · 20 comments · May be fixed by #54250
Assignees
Labels
[Feature] Blocks Overall functionality of blocks [Status] In Progress Tracking issues with work in progress [Status] Needs More Info Follow-up required in order to be actionable. [Type] Enhancement A suggestion for improvement.

Comments

@wpsoul
Copy link

wpsoul commented Aug 11, 2023

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

@jordesign jordesign added [Type] Enhancement A suggestion for improvement. [Feature] Blocks Overall functionality of blocks Needs Testing Needs further testing to be confirmed. labels Aug 14, 2023
@jorgefilipecosta
Copy link
Member

jorgefilipecosta commented Aug 15, 2023

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?

@jorgefilipecosta jorgefilipecosta added [Status] Needs More Info Follow-up required in order to be actionable. and removed Needs Testing Needs further testing to be confirmed. labels Aug 15, 2023
@ellatrix
Copy link
Member

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

@wpsoul
Copy link
Author

wpsoul commented Aug 16, 2023

@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
Copy link
Member

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
Copy link
Member

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

@wpsoul
Copy link
Author

wpsoul commented Aug 17, 2023

@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
Copy link
Member

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
Copy link
Author

wpsoul commented Aug 19, 2023

@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
Copy link

github-actions bot commented Sep 4, 2023

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.

@github-actions github-actions bot added the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Sep 4, 2023
@ellatrix ellatrix linked a pull request Sep 7, 2023 that will close this issue
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Sep 7, 2023
@ellatrix
Copy link
Member

ellatrix commented Sep 7, 2023

@wpsoul I created #54250. Does that work for you?

@github-actions github-actions bot removed the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Sep 8, 2023
@github-actions
Copy link

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.

@github-actions github-actions bot added the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Sep 23, 2023
@wpsoul
Copy link
Author

wpsoul commented Sep 24, 2023

@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 github-actions bot removed the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Sep 25, 2023
@github-actions
Copy link

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.

@github-actions github-actions bot added the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Oct 10, 2023
@wpsoul
Copy link
Author

wpsoul commented Nov 20, 2023

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

@github-actions github-actions bot removed the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Nov 21, 2023
Copy link

github-actions bot commented Dec 7, 2023

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.

@github-actions github-actions bot added the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Dec 7, 2023
@DamChtlv
Copy link

DamChtlv commented Dec 21, 2023

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' );
    }
}
@github-actions github-actions bot removed the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Dec 26, 2023
@ellatrix
Copy link
Member

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

CreativeDive commented Apr 26, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks [Status] In Progress Tracking issues with work in progress [Status] Needs More Info Follow-up required in order to be actionable. [Type] Enhancement A suggestion for improvement.
6 participants