288: Removes async, adds data attribute

[evanmwillhite]

Companion to fourkitchens/emulsify#288, this removes the troublesome async attribute and replaces it with a data attribute that is detected in JS by a new script in the companion PR.

[philwolstenholme]

I've updated #4 to include this change, hopefully this'll prevent any merge conflicts if this PR is merged first

[philwolstenholme]

@evanmwillhite I've given this a try, but I'm getting errors in my console to do with missing jQuery, because my component library scripts are trying to run before my global library (which contains jQuery) is being loaded in the footer (this is done before <script src="../../js/reload-scripts.js"></script> is mentioned, so I think this setup should be okay.

I think we could prevent this by doing some more data-attribute trickery, for example, instead of using

<script data-name="reload" src="/path-to-file.min.js></script>

We could use:

<script data-name="reload" data-src="/path-to-file.min.js></script>

And then use the JS in Emulsify to swap the data-src for an actual src. This way, the libraries won't actually be loaded until reload-scripts has a chance to run.

[evanmwillhite]

Great idea, @philwolstenholme! I've made this change here and it is working well for me (see latest on fourkitchens/emulsify#288). Can you verify on your end?

[philwolstenholme]

Hi @evanmwillhite , I tried to test this but the patch (https://patch-diff.githubusercontent.com/raw/drupal-pattern-lab/attach-library-twig-extension/pull/7.patch) wouldn't apply as I'm already applying #4 as a patch and they both change the same lines.

Would it be possible for you to review and merge in #4 ?

[philwolstenholme]

@evanmwillhite I wonder if we also need to do something to defer Emulsify's call to Drupal.attachBehaviors until all the script elements have had their data-src turned into a src, and then had their contents loaded so that Drupal behaviours have a chance to add the behaviour to Drupal.behaviors before the attachBehaviors function is called.

At the moment I'm doing something similar in my theme to what this PR proposes, but attachBehaviors (at the bottom of my _01-foot.twig file) is being called too soon.

I tried adding this to the end of reload-scripts.js:

if (
  typeof Drupal === 'object' &&
  typeof Drupal.attachBehaviors === 'function'
) {
  Drupal.attachBehaviors();
}

But it's still called too soon, presumably because even though the scripts.forEach will have run, the scripts will not necessarily have loaded?

Edit: for now I've done something very basic in _01-foot.twig:

  <!-- If using Drupal.behaviors, uncomment for them to work in Pattern Lab -->
  <script>Drupal.attachBehaviors();</script>
  <script>
    // A *very* basic way to account for the fact that libraries added in Pattern
    // Lab via a attach-library-twig-extension will be done after the above line
    // and pattern-lab/public/js/reload-scripts.js have been executed.
    // The timeout value can be adjusted if it is too generous or too conservative.

    // See https://github.com/drupal-pattern-lab/attach-library-twig-extension/pull/7
    setTimeout(function () {
      Drupal.attachBehaviors();
    }, 1000);
  </script>

[amazingrando]

Code and functional review pass.

[evanmwillhite]

@philwolstenholme I used some of your code here, but I wanted to see if I could avoid the timeout since it could be variable. I pinged you on the other one already - just wanted to note it here.