Contrib.social

Detect external library assets rather than requiring manual type: external definition

Open on Drupal.org →
Created on 16 June 2023, almost 2 years ago
Updated 9 August 2023, over 1 year ago

Problem/Motivation

The default 'type' for library assets is always "file" not matter what.

Current logic in \Drupal\Core\Asset\LibraryDiscoveryParser::buildByExtension():

          // By default, all library assets are files.
          if (!isset($options['type'])) {
            $options['type'] = 'file';
          }
          if ($options['type'] == 'external') {
            $options['data'] = $source;
          }
          // Determine the file asset URI.
          else {
            if ($source[0] === '/') {
              // An absolute path maps to DRUPAL_ROOT / base_path().
              if ($source[1] !== '/') {
                $source = substr($source, 1);
                // Non core provided libraries can be in multiple locations.
                if (strpos($source, 'libraries/') === 0) {
                  $path_to_source = $this->librariesDirectoryFileFinder->find(substr($source, 10));
                  if ($path_to_source) {
                    $source = $path_to_source;
                  }
                }
                $options['data'] = $source;
              }
              // A protocol-free URI (e.g., //cdn.com/example.js) is external.
              else {
                $options['type'] = 'external';
                $options['data'] = $source;
              }
            }
            // A stream wrapper URI (e.g., public://generated_js/example.js).
            elseif ($this->streamWrapperManager->isValidUri($source)) {
              $options['data'] = $source;
            }
            // A regular URI (e.g., http://example.com/example.js) without
            // 'external' explicitly specified, which may happen if, e.g.
            // libraries-override is used.
            elseif ($this->isValidUri($source)) {
              $options['type'] = 'external';
              $options['data'] = $source;
            }
            // By default, file paths are relative to the registering extension.
            else {
              $options['data'] = $path . '/' . $source;
            }
          }

Steps to reproduce

  1. Add a custom.libraries.yml definition like this:
    external:
  js:
    http://www.example.com/test.js: {}
  2. Test that the library is not defined as external:
    $library = \Drupal::service('library.discovery')->getLibraryByName('custom', 'external');
// You will see that $library['js'][0]['type'] is set to 'file'

Proposed resolution

I propose that we reduce a burden when defining libraries by checking if the asset path passes UrlHelper::isExternal() and if so, use a type of "external" and not require everyone to manually define external URLs with { type: external }. This can also simplify the logic below by skipping some steps since we're not setting external assets with type "file".

Remaining tasks

User interface changes

None

API changes

External library assets no longer need { type: external }. If the asset URI passes UrlHelper::isExternal the external type is automatically applied.

Data model changes

None

Release notes snippet

External library assets are now automatically detected as external and no longer need { type: external } to be set in the library definition.

✨ Feature request
Status

Needs work

Version

11.0 🔥

Component
Asset library  →

Last updated 3 days ago

No maintainer
Created by

🇺🇸United States dave reid Nebraska USA

Live updates comments and jobs are added and updated live.
  • Needs change record

    A change record needs to be drafted before an issue is committed. Note: Change records used to be called change notifications.

Sign in to follow issues

Comments & Activities

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024