Problem/Motivation

SDC lets you alter the auto-generated Drupal library using libraryOverrides in the component definition file. It will then treat the paths for CSS and JS as relative to the component library folder, so it can be found later on. This process does not check if the provided URL is external or not. This leads to broken URLs in the front-end.

Temporary workaround

Manually define a library that contains this external JS/CSS, and add a dependency to it in your component instead.

Steps to reproduce

Add the following to your component definition:

libraryOverrides:
  js:
    https://drupal.org/fake-dependency/index.min.js: { }
  css:
    component:
      https://drupal.org/fake-dependency/styles.css: { }

Then render the component normally.

You will observe that the browser is trying to request: https://d10.ddev.site/en/admin/config/modules/contrib/cl_devel/components/component-details/https://drupal.org/fake-dependency/index.min.js

Error message displayed

Proposed resolution

The code that builds the path relative to the component directory should check if the path is external or not, in both CSS and JS.

Remaining tasks

  • Write the fix.
  • Add an automated test for the fix.
CommentFileSizeAuthor
sdc-external-url-error.png237.31 KBe0ipso

Issue fork drupal-3357382

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

e0ipso created an issue. See original summary.

e0ipso’s picture

Issue summary: View changes

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

e0ipso’s picture

Robert Ngo made their first commit to this issue’s fork.

robert ngo’s picture

I tried a fix using `UrlHelper::isExternal()` to detect external JS.
Also, should we have a mirror issue for external CSS?

robert ngo’s picture

Status: Active » Needs review
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Tested using #3365497: Create new SDC component for Umami Banner and confirmed the issue by adding

libraryOverrides:
  js:
    https://cdn.jsdelivr.net/npm/docson@2.1.0/lib/index.min.js: {}

Applying the MR I get Uncaught ReferenceError: exports is not defined
Which I think is unrelated so the error from this ticket appears to have been resolved.

e0ipso’s picture

Title: Unable to override library auto-definition to add external JS » Unable to override library auto-definition to add external CSS & JS
Issue summary: View changes
Status: Reviewed & tested by the community » Needs work

I updated the IS to reflect that we also need this for CSS. Thanks @Robert Ngo for pointing it out in #7.

robert ngo’s picture

I've revised the fake-dependency URLs and added the case for external CSS.
It's ready to retest now.

robert ngo’s picture

Status: Needs work » Needs review
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Same test from #9 but using css option too.
Confirmed the bug and the MR fixed the console error.

  • lauriii committed 575c5331 on 11.x
    Issue #3357382 by Robert Ngo, e0ipso, smustgrave: Unable to override...

  • lauriii committed 0d0b1864 on 10.1.x
    Issue #3357382 by Robert Ngo, e0ipso, smustgrave: Unable to override...
lauriii’s picture

Version: 11.x-dev » 10.1.x-dev
Status: Reviewed & tested by the community » Fixed

Committed 575c533 and pushed to 11.x. Thanks! Also cherry-picked to 10.1.x.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.