symfony / webpack-encore-bundle

Symfony integration with Webpack Encore!
https://symfony.com/webpack-encore
MIT License
939 stars 83 forks source link

ResetAssetsEventListener breaks embedding controllers? #166

Closed stephanvierkant closed 2 years ago

stephanvierkant commented 2 years ago

115 breaks my application.

I'm using Bootstrap 3 and the content of one of the tabs is rendered using render():


<ul class="nav nav-tabs nav-justified nav-responsive" role="tablist">
   <!-- <li.... -->
</ul>
<div class="tab-content">
    <div role="tabpanel" class="tab-pane active" id="details">
        {{ render(url('user_details', {username: user.username})) }}
    </div>
</div>

Now this pull-down doesn't work anymore. I still can't figure out what's going wrong here, but I can tell the ResetAssetsEventListener plays a role here.

I'll try to investigate what the exact problem is, but any help would be appreciated.

Edit: I've found that the dropdown is opening and closing again directly. For some reason, the click handler on the dropdown menu is registered twice. I really don't know what's causing this, because the HTML code is exactly the same before and after installing the new version of this bundle.

zing-james commented 2 years ago

I've encountered a similar related issue. The ResetAssetsEventListener is getting called on every request in the stack and not just the main one. This is causing runtime.js to be included multiple times. This example is just to illustrate what is happening.

<html>
    <head>        
        {{ encore_entry_link_tags('app') }}
        {{ encore_entry_link_tags('test') }}
        {{ encore_entry_script_tags('app') }}

    <body>
        {{ render(controller('App\\Controller\\TestController::test')) }}
        {{ encore_entry_script_tags('test') }}
    </body>
</html>

We now end up with something like this:

<html>
    <head>
        <link rel="stylesheet" href="/build/app.css">
        <link rel="stylesheet" href="/build/test.css">
        <script src="/build/runtime.js"></script>
    <script src="/build/app.js"></script>
    </head> 
    <body>
    <script src="/build/runtime.js"></script>
    <script src="/build/test.js"></script>
    </body>
</html>

Where we use to end up with something more like this:

<html>
    <head>
        <link rel="stylesheet" href="/build/app.css">
        <link rel="stylesheet" href="/build/test.css">
        <script src="/build/runtime.js"></script>
    <script src="/build/app.js"></script>
    </head> 
    <body>
    <script src="/build/test.js"></script>
    </body>
</html>

I understand that this change was to fix side effects in other places due to the renderer remembering the assets that were already loaded, but now using the render method in twig has a different side-effect.

quentint commented 2 years ago

I can confirm.

When rendering fragments with {{ render(controller([...])) }} (or url()), the ResetAssetsEventListener::resetAssets method is invoked, and TagRenderer::$renderedFiles is reset, making later calls to {{ encore_entry_script_tags([...]) }} inject the <script src="/build/runtime.[something].js"></script>.

This "multi-runtime" issue makes the Encore scripts get called multiple times 🙁

A quick fix would be to replace ResetAssetsEventListener::resetAssets with:

public function resetAssets($e)
{
    if (!$e->isMainRequest()) {
        return;
    }
    foreach ($this->buildNames as $name) {
        $this->entrypointLookupCollection->getEntrypointLookup($name)->reset();
    }
}

But I haven't test this enough and I'm not sure this wouldn't break anything.

H4wKs commented 2 years ago

Using Symfony 4.4.X, and after upgrading to the last version of webpack-ecnore I have a the same issue, and I confirm that it's related to when you render a controller view inside of twig.

Downgrading symfony/webpack-encore-bundle from v1.14.0 to v1.13.2 fix the issue.

H4wKs commented 2 years ago

Ping @Warxcell @weaverryan

TarikAmine commented 2 years ago

We are stuck at 1.13.2 as well because of this, i made a pr if that helps https://github.com/symfony/webpack-encore-bundle/pull/172

weaverryan commented 2 years ago

172 is merged - try out https://github.com/symfony/webpack-encore-bundle/releases/tag/v1.14.1

This is... a tricky issue - I'm sorry it created so many problems. I'm not entirely convinced that #172 won't introduce its own, new problems. So let's see. If we still have issues, we may need to revert the ResetAssetsEventListener entirely.

Cheers!

dmaicher commented 2 years ago

For me the issue is fixed. Thanks @TarikAmine and @weaverryan

leroy0211 commented 4 months ago

We are still experiencing this issue. We are running version webpack-encore-bundle@1.7.0.

We have a twig file that executes render(controller()) and right after that, it will load the encore script tags. At that moment the runtime is being added again. Which causes events to be bounded twice.

[update: 15 hours later] We have figured it out, it wasn't related to render(controller()) but we wrapped fragments of html (including encore_entry_script_tags) in a twig cache. Therefor the cache contains the runtime, and eventually the runtime was added again because encore thought the runtime wasn't added yet.