philw_ [Yesterday at 3:02 PM]
Has anyone run into an Emulsify issue with adding libraries that contain external JS files? components/_twig-components/functions/pl_attach-library.function.php tries to attach all the library files as async script tags, but it prepends the src value with a /, so external srcs are broken, here’s an example:
<script async="" src="/https://cdnjs.cloudflare.com/ajax/libs/popper.js/1.14.3/umd/popper.min.js"></script>
31 replies danny_englander [23 hours ago] There's a chance it might be related to this? drupal-pattern-lab/attach-library-twig-extension#1 GitHub Drupal.js issue with adding script with async attribute · Issue #1 · drupal-pattern-lab/attach-library-twig-extension It appears the method of loading Drupal.js and attaching libraries that is currently recommended in Emulsify does not play well with how we're adding the scripts with the async attribute. We wi...
evanmwillhite [23 hours ago] hadn't yet, but not too surprised as this Twig extension is extremely new. This is different from that one above, so I've added a new issue drupal-pattern-lab/attach-library-twig-extension#3 GitHub External scripts · Issue #3 · drupal-pattern-lab/attach-library-twig-extension When adding an external script, the extension incorrectly prepends it with a trailing slash like <script async="" src="/https://cdnjs.cloudflare.com/ajax/libs/popper.js/1.14.3/umd...
(edited)
philw_ [23 hours ago] Thanks @evanmwillhite, I’ve commented on that ticket with some more info
evanmwillhite [23 hours ago] Awesome, thank you!
philw_ [23 hours ago] @evanmwillhite If I’m modifying a Twig_SimpleFunction do I need to reload PL each time for it to pick up on the changes?
evanmwillhite [23 hours ago]
you do. You might be able to get away with running gulp pl:full instead of rerunning the start command
philw_ [23 hours ago] @evanmwillhite I think there’s also a bug in that function where only the first JS file actually gets a script element produced, I think maybe because $output gets wiped out by each iteration of the forEach loop, then only passed to $stringLoader->render once
philw_ [23 hours ago] I’m just having a play around with it now as well as the external scripts issue, if I fix it I can raise a PR against drupal-pattern-lab/attach-library-twig-extension#3
evanmwillhite [23 hours ago] :disappointed: OK, I can add that one too. Sorry - if you're able to fix any of these, would you like me to add you as a contributor so you can submit PR's directly?
philw_ [23 hours ago] Yes please, I’ll let you know how I get on
philw_ [23 hours ago] I’ll raise the second issue to save you doing it 🙂
evanmwillhite [23 hours ago] what is your github username?
philw_ [23 hours ago]
It’s philwolstenholme
evanmwillhite [23 hours ago] invite sent!
evanmwillhite [23 hours ago] Thanks and thanks for your patience on this piece. If we have to, we can go back to the method we used before it but I would love to fix these issues as I feel like it is a better approach overall. I should be able to devote some time either tomorrow or next week myself to the project
philw_ [23 hours ago] Here’s a PR for the external paths issue: drupal-pattern-lab/attach-library-twig-extension#4 GitHub Fix for external JS paths being broken by attach_library by philwolstenholme · Pull Request #4 · drupal-pattern-lab/attach-library-twig-extension A quick attempt at fixing #3 Only prefixes JS paths with a / is the JS is not type: external
philw_ [23 hours ago] How is that function brought into Emulsify @evanmwillhite? Is it copied in during the install process? I’m trying to work out the best way to patch this locally
evanmwillhite [23 hours ago] Awkwardly... because we wanted to offer it to the community outside of Pattern Lab like our other Twig functions, we copy them over in this script https://github.com/fourkitchens/emulsify/blob/develop/scripts/twig_functions.sh GitHub fourkitchens/emulsify Emulsify: Pattern Lab + Drupal 8. Contribute to fourkitchens/emulsify development by creating an account on GitHub.
evanmwillhite [23 hours ago] The good is that this will get the latest on install.
evanmwillhite [23 hours ago] If you want to make changes though, I would just remove from the gitignore file and commit it to your repo.
philw_ [23 hours ago] Ah yes, that’s a good idea
philw_ [23 hours ago] :thumbsup:
philw_ [22 hours ago] @evanmwillhite here’s a second PR for the ‘only the first JS file is output’ issue: drupal-pattern-lab/attach-library-twig-extension#6 GitHub Alter when the list of script tags is returned to make sure we can handle libraries with more than one JS file by philwolstenholme · Pull Request #6 · drupal-pattern-lab/attach-library-twig-extension This seems to fix #5 for me
evanmwillhite [22 hours ago] Thanks @philw_!
philw_ [22 hours ago] No problem! 🙂
philw_ [22 hours ago] I’m inevitably going to be asking this channel a lot of questions over the next few weeks as I’m starting on my first PL and Emulsify project, so it’s only fair to contribute back where I can 👍 (edited)
evanmwillhite [22 hours ago] I also added you to emulsify and emulsify-gulp in case you need to create PRs against those more easily as well
philw_ [22 hours ago] Oh great, thank you
evanmwillhite [22 hours ago] thank YOU!
bjlewis2 [22 hours ago] @philw_ ++ for contributing back!