Currently, tacjs only loads tarteaucitron.js vie the libraries API, then this JS files loads additional JS and CSS files.
This means these files don't benefit from Drupal optimizations (aggregation, caching, etc.).

We could use the useExternalJs and useExternalCss settings to prevent tarteaucitron from loading these files, then make Drupal load them.

CommentFileSizeAuthor
#7 interdiff_5-7.txt681 bytesprudloff
#7 tacjs-3158826-7.patch2.66 KBprudloff
#5 interdiff_2-5.txt2.4 KBboulaffasae
#5 tacjs-3158826-5.patch2.75 KBboulaffasae
#2 tacjs-3158826-2.patch2.28 KBprudloff

Issue fork tacjs-3158826

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:

About issue forks

Comments

prudloff created an issue. See original summary.

prudloff’s picture

prudloff commented
Status: Active » Needs work
StatusFileSize
new tacjs-3158826-2.patch2.28 KB

The attached patch makes Drupal load the following files:

  • tarteaucitron.services.js
  • css/tarteaucitron.css
  • the approriate language file

In order to only load the language file we need, it forces tarteaucitron's language to the current Drupal langague. (This has the added benefit of making sure the website and tarteaucitron always use the same language.)

The main remaining issue is that will only work correctly if useExternalJs and useExternalCss are enabled.
Should we force-enable them for everyone?
But then what about other legitimate use cases of useExternalCss (like people who want to use their own custom CSS)?

boulaffasae’s picture

boulaffasae commented

Hi prudloff,

Yes tarteaucitron currently use asynchronous loading, which is more optimized than serving an aggregated file. If one day the library added support for loading files with <script /> & <link /> we could update our code source accordingly.

prudloff’s picture

prudloff commented

Well, technically tarteaucitron already supports that. That's what the useExternalJs option is for, so you can load tarteaucitron.services.js any other way you want.

I understand why tarteaucitron (as a general library not tied to a specific CMS) loads script like that by default, but I also think Drupal modules should load every resource with the Drupal libraries system (so that other modules can apply various optimizations to these resources).

boulaffasae’s picture

boulaffasae commented
StatusFileSize
new tacjs-3158826-5.patch2.75 KB
new interdiff_2-5.txt2.4 KB

I made some changes in your patch if it's okey :). Please review my patch & let's test this together before applying patch to the 3.11 version :)

  1. Drupal provide a drupalSettings.path.currentLanguage globally, we can use this instead of 'lang' => $lang.
  2. And I replaced component by theme, Wanna do a Call of Duty 1 vs 1 to decide ?

The main remaining issue is that will only work correctly if useExternalJs and useExternalCss are enabled.
Should we force-enable them for everyone?
But then what about other legitimate use cases of useExternalCss (like people who want to use their own custom CSS)?

We need just to enable useExternalCss/useExternalJs by default so that Drupal optimize & load required css & js. For people who want to use their own custom CSS, Drupal have a great solution to overridde assets.

libraries-override was added as a way for themes to manipulate assets (CSS/JS) provided by libraries. This includes removing or replacing assets, as well as removing a library altogether.

- Libraries can now be overridden and extended by themes

libraries-override:
  # Replace an asset with another.
  tacjs/tarteaucitron.js:
    css:
      theme:
        assets/vendor/tarteaucitron.js/css/tarteaucitron.css: css/custom-tarteaucitron.css
boulaffasae’s picture

boulaffasae commented
Status: Needs work » Needs review
prudloff’s picture

prudloff commented
StatusFileSize
new tacjs-3158826-7.patch2.66 KB
new interdiff_5-7.txt681 bytes

The language file has to be loaded after the library (because it accesses the tarteaucitron object).

Apart from that, everything seems to work correctly.

  • boulaffasae committed d3385ab on 8.x-3.x 
    Issue #3158826 by prudloff, boulaffasae: Allow Drupal to optimize...
boulaffasae’s picture

boulaffasae commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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

mademo made their first commit to this issue’s fork.