Unverified Commit d3457677 authored by Alex Pott's avatar Alex Pott
Browse files

Issue #3494354 by catch, mstrelan, acbramley, smustgrave:...

Issue #3494354 by catch, mstrelan, acbramley, smustgrave: AssetResolver::getJsAssets cache id doesn't consider attached settings, leads to ajax issues

(cherry picked from commit dc6bc439)
parent 72642535
Loading
Loading
Loading
Loading
Loading
+22 −23
Original line number Diff line number Diff line
@@ -286,7 +286,8 @@ protected function getJsSettingsAssets(AttachedAssetsInterface $assets) {
   * {@inheritdoc}
   */
  public function getJsAssets(AttachedAssetsInterface $assets, $optimize, ?LanguageInterface $language = NULL) {
    if (!$assets->getLibraries() && !$assets->getSettings()) {
    $asset_settings = $assets->getSettings();
    if (!$assets->getLibraries() && !$asset_settings) {
      return [[], []];
    }
    if (!isset($language)) {
@@ -309,14 +310,14 @@ public function getJsAssets(AttachedAssetsInterface $assets, $optimize, ?Languag

    // If all the libraries to load contained only CSS, there is nothing further
    // to do here, so return early.
    if (!$libraries_to_load && !$assets->getSettings()) {
    if (!$libraries_to_load && !$asset_settings) {
      return [[], []];
    }

    // Add the theme name to the cache key since themes may implement
    // hook_library_info_alter(). Additionally add the current language to
    // support translation of JavaScript files via hook_js_alter().
    $cid = 'js:' . $theme_info->getName() . ':' . $language->getId() . ':' . Crypt::hashBase64(serialize($libraries_to_load)) . (int) (count($assets->getSettings()) > 0) . (int) $optimize;
    $cid = 'js:' . $theme_info->getName() . ':' . $language->getId() . ':' . Crypt::hashBase64(serialize($libraries_to_load)) . ':' . (int) $optimize;

    if ($cached = $this->cache->get($cid)) {
      [$js_assets_header, $js_assets_footer, $settings, $settings_in_header] = $cached->data;
@@ -389,32 +390,30 @@ public function getJsAssets(AttachedAssetsInterface $assets, $optimize, ?Languag
        $js_assets_footer = $collection_optimizer->optimize($js_assets_footer, $libraries_to_load);
      }

      // If the core/drupalSettings library is being loaded or is already
      // loaded, get the JavaScript settings assets, and convert them into a
      // single "regular" JavaScript asset.
      $libraries_to_load = $this->getLibrariesToLoad($assets);
      $settings_required = in_array('core/drupalSettings', $libraries_to_load) || in_array('core/drupalSettings', $this->libraryDependencyResolver->getLibrariesWithDependencies($assets->getAlreadyLoadedLibraries()));
      $settings_have_changed = count($libraries_to_load) > 0 || count($assets->getSettings()) > 0;

      // Initialize settings to FALSE since they are not needed by default. This
      // distinguishes between an empty array which must still allow
      // hook_js_settings_alter() to be run.
      $settings = FALSE;
      if ($settings_required && $settings_have_changed) {
      // Always build settings from js libraries. They may or may not be
      // used later depending on whether the core/drupalSettings library is
      // requested.
      $settings = $this->getJsSettingsAssets($assets);
      // Allow modules to add cached JavaScript settings.
      $this->moduleHandler->invokeAllWith('js_settings_build', function (callable $hook, string $module) use (&$settings, $assets) {
        $hook($settings, $assets);
      });
      }
      $settings_in_header = in_array('core/drupalSettings', $header_js_libraries);
      $this->cache->set($cid, [$js_assets_header, $js_assets_footer, $settings, $settings_in_header], CacheBackendInterface::CACHE_PERMANENT, ['library_info']);
    }

    if ($settings !== FALSE) {
    // If the core/drupalSettings library is being loaded or is already
    // loaded, get the JavaScript settings assets, and convert them into a
    // single "regular" JavaScript asset. But only if there are settings to
    // add. Do the quickest checks first.
    $process_settings = FALSE;
    if (count($libraries_to_load) > 0 || count($asset_settings) > 0) {
      $process_settings = in_array('core/drupalSettings', $libraries_to_load) || in_array('core/drupalSettings', $this->libraryDependencyResolver->getLibrariesWithDependencies($assets->getAlreadyLoadedLibraries()));
    }
    if ($process_settings) {
      // Attached settings override both library definitions and
      // hook_js_settings_build().
      $settings = NestedArray::mergeDeepArray([$settings, $assets->getSettings()], TRUE);
      $settings = NestedArray::mergeDeepArray([$settings, $asset_settings], TRUE);
      // Allow modules and themes to alter the JavaScript settings.
      $this->moduleHandler->alter('js_settings', $settings, $assets);
      $this->themeManager->alter('js_settings', $settings, $assets);
+22 −0
Original line number Diff line number Diff line
@@ -467,4 +467,26 @@ public function testAddJsFileWithQueryString(): void {
    $this->assertStringContainsString('<script src="' . str_replace('&', '&amp;', $this->fileUrlGenerator->generateString('core/modules/system/tests/modules/common_test/querystring.js?arg1=value1&arg2=value2')) . '&amp;' . $query_string . '"></script>', $rendered_js, 'JavaScript file with query string gets version query string correctly appended.');
  }

  /**
   * Test settings can be loaded even when libraries are not.
   */
  public function testAttachedSettingsWithoutLibraries(): void {
    $assets = new AttachedAssets();

    // First test with no libraries will return no settings.
    $assets->setSettings(['test' => 'foo']);
    $js = $this->assetResolver->getJsAssets($assets, TRUE, \Drupal::languageManager()->getCurrentLanguage())[1];
    $this->assertArrayNotHasKey('drupalSettings', $js);

    // Second test with a warm cache.
    $js = $this->assetResolver->getJsAssets($assets, TRUE, \Drupal::languageManager()->getCurrentLanguage())[1];
    $this->assertArrayNotHasKey('drupalSettings', $js);

    // Now test with different settings when drupalSettings is already loaded.
    $assets->setSettings(['test' => 'bar']);
    $assets->setAlreadyLoadedLibraries(['core/drupalSettings']);
    $js = $this->assetResolver->getJsAssets($assets, TRUE, \Drupal::languageManager()->getCurrentLanguage())[1];
    $this->assertSame('bar', $js['drupalSettings']['data']['test']);
  }

}