Only file JavaScript assets with preprocessing enabled can be optimized.

I'm facing the same issue. I don't understand the current logic. If the library doesn't want to be preprocessed, why is it automatically minified by default? In that case it makes no sense in preprocess: false.

I believe the issue lies in the logic implemented in two methods: \Drupal\Core\Asset\JsCollectionOptimizerLazy::optimizeGroup and \Drupal\Core\Asset\JsCollectionOptimizer::optimize.

The \Drupal\Core\Asset\JsCollectionOptimizer::optimize respects asset group preprocess status:

          if (!$js_group['preprocess']) {
            $uri = $js_group['items'][0]['data'];
            $js_assets[$order]['data'] = $uri;
          }

The \Drupal\Core\Asset\JsCollectionOptimizerLazy::optimizeGroup function optimizes the group regardless of any conditions. It doesn't have a condition and simply moves on to the optimization process.

This method should looks like:

  public function optimizeGroup(array $group): string {
    $data = '';
    $current_license = FALSE;

    // No preprocessing, single JS asset: just use the existing URI.
    if ($group['type'] === 'file' && !$group['preprocess']) {
      $data = file_get_contents($group['items'][0]['data']);
    }
    else {
      foreach ($group['items'] as $js_asset) {
        // Ensure license information is available as a comment after
        // optimization.
        if ($js_asset['license'] !== $current_license) {
          $data .= "/* @license " . $js_asset['license']['name'] . " " . $js_asset['license']['url'] . " */\n";
        }
        $current_license = $js_asset['license'];
        // Optimize this JS file, but only if it's not yet minified.
        if (isset($js_asset['minified']) && $js_asset['minified']) {
          $data .= file_get_contents($js_asset['data']);
        }
        else {
          $data .= $this->optimizer->optimize($js_asset);
        }
        // Append a ';' and a newline after each JS file to prevent them from
        // running together.
        $data .= ";\n";
      }
    }

    // Remove unwanted JS code that causes issues.
    return $this->optimizer->clean($data);
  }->optimizer->clean($data);
  }

I created the MR with the fix and it works with the google_tag module. However, I'm still not sure about this part of the code: $data = file_get_contents($group['items'][0]['data']);. The same code occurs in the methodDrupal\Core\Asset\JsCollectionOptimizer::optimize. It seems that there may be more than one 'item' in the group, and in such a case, some content may be missing. Need some advice here is that safe or not.

It also fails when using libraries with external dependencies, but with a slightly different exception.

Only file JavaScript assets can be optimized.

E.g. of such library:

example:
  version: 1
  remote: https://example.com
  license:
    name: Example
    url: https://example.com/license
    gpl-compatible: true
  js:
    js/init.js: { }
    //example.com/script.js: { preprocess: false }

The suggested solution won't work in this case because the type here is external, which is handled in the method \Drupal\Core\Asset\JsCollectionOptimizer::optimize, but not in the method \Drupal\Core\Asset\JsCollectionOptimizerLazy::optimizeGroup. But for now, I’m out of ideas on how to solve this issue for external libraries. The result of the lazy method should be optimized JavaScript.

For those who have encountered this problem, I suggest explicitly setting the following in the code: {preprocess: false, minified: true} for such JavaScript files.

It's also worth mentioning that the solution from MR and Drupal\Core\Asset\JsCollectionOptimizer::optimize does not include injecting library license information.

Can the issue summary be updated to include any proposed solution and other relevant sections from the issue template.

Previously was tagged for tests may be good to get those written and may help guide the fix.

I can confirm this message keeps on filling our logs on larger projects.

It appears this is also affecting the Stripe module.

We're seeing this too. Its odd that it would skip "minified" but not "preprocess = false" here if the optimize function is just going to throw an exception for "preprocess = false": https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...

Just reporting in that we are seeing this frequent visitor in our logs too.

Same issue here. A larger project fills the log with this error message.

This indeed still fills up the logs and is confusing. This is nothing that should be at exception level, I think. For us, it means, that monitoring sees this as (kind of critical) error like the application is broken somewhere. We should really fix this, or at least reduce the severity for that reason. Especially because the site owner can't do anything about it?

anybody → changed the visibility of the branch 3416508-only-file-javascript to hidden.

Same as in #14 - using external js, not minified.
Works for me:
- using pach from MR
- using local copy of the external js file, with {preprocess: false, minified: false}

Could we change the exception to an assert() maybe? This seems like it should be a libraries.yml validation step (which we don't really have).

The MR looks reasonable to me though.

Adjusted the issue summary and replaced the exception statements with "assert()" statements. Do we really need tests for this, since it is pretty straight forward?

Also should we create a follow-up issue, regarding #10 🐛 Only file JavaScript assets with preprocessing enabled can be optimized. Active :

The suggested solution won't work in this case because the type here is external, which is handled in the method \Drupal\Core\Asset\JsCollectionOptimizer::optimize, but not in the method \Drupal\Core\Asset\JsCollectionOptimizerLazy::optimizeGroup. But for now, I’m out of ideas on how to solve this issue for external libraries. The result of the lazy method should be optimized JavaScript.

?

Seems like a pretty valid bug for test coverage.

But pipeline has issues.

Using a patch from the MR. The last commit:
"ef07f56b - Use asserts instead of if cases"
breaks my site. I had to revert it.

@szato what was the error / backtrace message?

@grevil

The website encountered an unexpected error. Try again later.

AssertionError: Only file JavaScript assets can be optimized. in assert() (line 30 of core/lib/Drupal/Core/Asset/JsOptimizer.php).
Drupal\Core\Asset\JsOptimizer->optimize() (Line: 184)
Drupal\Core\Asset\JsCollectionOptimizerLazy->optimizeGroup() (Line: 185)
Drupal\system\Controller\AssetControllerBase->deliver()
call_user_func_array() (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 638)
Drupal\Core\Render\Renderer->executeInRenderContext() (Line: 121)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext() (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 181)
Symfony\Component\HttpKernel\HttpKernel->handleRaw() (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle() (Line: 53)
Drupal\Core\StackMiddleware\Session->handle() (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle() (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle() (Line: 116)
Drupal\page_cache\StackMiddleware\PageCache->pass() (Line: 90)
Drupal\page_cache\StackMiddleware\PageCache->handle() (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle() (Line: 124)
Drupal\cloudflare\CloudFlareMiddleware->handle() (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle() (Line: 36)
Drupal\Core\StackMiddleware\AjaxPageState->handle() (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle() (Line: 741)
Drupal\Core\DrupalKernel->handle() (Line: 19)

Uninstalled cloudflare, same error

The website encountered an unexpected error. Try again later.

AssertionError: Only file JavaScript assets can be optimized. in assert() (line 30 of core/lib/Drupal/Core/Asset/JsOptimizer.php).
Drupal\Core\Asset\JsOptimizer->optimize() (Line: 184)
Drupal\Core\Asset\JsCollectionOptimizerLazy->optimizeGroup() (Line: 185)
Drupal\system\Controller\AssetControllerBase->deliver()
call_user_func_array() (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 638)
Drupal\Core\Render\Renderer->executeInRenderContext() (Line: 121)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext() (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 181)
Symfony\Component\HttpKernel\HttpKernel->handleRaw() (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle() (Line: 53)
Drupal\Core\StackMiddleware\Session->handle() (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle() (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle() (Line: 116)
Drupal\page_cache\StackMiddleware\PageCache->pass() (Line: 90)
Drupal\page_cache\StackMiddleware\PageCache->handle() (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle() (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle() (Line: 36)
Drupal\Core\StackMiddleware\AjaxPageState->handle() (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle() (Line: 741)
Drupal\Core\DrupalKernel->handle() (Line: 19)

Thank you, @szato! I am an idiot and forgot to negate the conditions... my apologies. Should work again now!

@grevil,

I can confirm: the actual MR patch works for me (with the last commit fix).

Pipeline is green, maybe the Draft status should be removed from the MR?

Pipeline is green now!

Tested the MR patch. Solved my issue, moving to RTBC.

For external JS, I had to move (make a copy) the external JS to local. I think we need a follow-up issue as it was mentioned in #22.

The change doesn't look right to me - how does a single, non-preprocessed file end up getting run through the optimizer? Only files that end up in asset aggregates can be optimized, which implies 'preprocess: true' (although we should change that to 'aggregate: true' at some point). Feels like it should prevent getting to this point earlier rather than returning the same file from the optimizer.

I do think this needs tests, so at least moving back for that, this might help clarify my confusion above.

We are seeing this error in our logs despite having both preprocess: false and minified: true in our library definition.