🇨🇦Canada @ambient.impact

Updated summary to reflect that we now have core patching in CI.

Marking 🐛 Turbo: Drupal messages are lost if Turbo does a full reload Active item as completed.

Updated the status of a bunch of items and rewrote a few things to reflect current state of various fixes.

I've created a new branch (3414538-what-if-attachments-processor) in the issue fork to try and implement a decorated attachments processor, but I can't yet get it to work. The main motivation here is to no longer rely on ajaxPageState because outputting it on every page results in certain generated URLs that have a query string (e.g. pager, admin/config compact toggle link, etc.) also including the Ajax page state in their URL, which in turn causes some weird behaviour with those links where Turbo doesn't kick in during history back/forward navigation, in addition to looking messy and ugly.

There's something I'm not understanding and not doing correctly with the library diffing, and without a clear indicator of what libraries are being added and when, it's hard to pin down. I think this would need some kind of debugging output when libraries are added to hopefully spot what's going wrong, but I'm tired and need to do other stuff right now.

Marking as fixed as a result of patching Turbo.

Added item for 🌱 Turbo: Tracking issue of core and contrib behaviours that need fixes to correctly detach and then attach Active .

ambient.impact → created an issue.

🤔🤔🤔 🐛 Logic error in Drupal's lazy load for asset aggregation Active will definitely be helpful for this. Will have to experiment with 10.4 when I get a chance.

I have mixed feelings about the stale file threshold: I can see it being useful for some smaller sites and it would be nice to have cache clears not wipe out aggregates, but on the other hand, I feel like it's treating a symptom and not the root cause; the root cause usually being that it's the tool someone reaches for when they don't know how to use update hooks, the Cache API (tag invalidation, etc.), or the appropriate service to clear/rebuild a specific thing they need. That's specifically why I created the Rebuilder module → , which can rebuild just aggregated assets among several other things.

ambient.impact → created an issue.

I'm very interested in seeing how this works out because making aggregation work - especially for JavaScript - was a nightmare for RefreshLess 2.x: 🐛 Turbo: JS aggregation causes the RefreshLess JavaScript to evaluate more and more times on navigation Active

This particular improvement might be less relevant to that project since it was more the additive part that had to be implemented, but anything core can do to make the library system even more predictable would be awesome.

Also, this makes me wonder if we get core to provide a list of all possible aggregates given the known dependency trees, could we then use that as part of a post-deploy process to proactively/preemptively build all those aggregates? That would be awesome from a performance point of view.

ambient.impact → created an issue.

After some significant work and testing, I think I've basically fixed FOUC under most circumstances.

Initial port works quite well. Setting to postponed for refining using ideas from NProgress.

Thanks! Also, I just discovered that a more thorough visible/hidden check was added to core at some point so I've updated the merge request to use that.

I recreated the change on 11.x, but since I can't edit MR !10210 (and I'm tired), I created a new one. Can someone close MR !10210 for me?

ambient.impact → created an issue.

ambient.impact → created an issue.

ambient.impact → created an issue.

My original implementation of dark mode toggling was primarily CSS using @media (prefers-color-scheme: dark) {}, which not only would have avoided having to store anything by default, but also removed the additional problems with having to check storage with JavaScript, i.e. no chance of brief flashes of light mode unless you used an inline script which is a problem with CSP, etc. See #3161904-27: Auto Darkmode option → for my concerns at the time.

That's what I get for having the tab open with the form state saved. 🤦

Right you are. Thanks!

Also fixed some typos in my summary.

An alternative if you don't want to install Ultimate Cron just to disable Warmer cron is the Hux module → 's ability to replace a hook implementation like we did on Omnipedia.

I can confirm that I'm also seeing this problem where a class that only uses ReplaceOriginalHook didn't get discovered no matter how many container rebuilds or full cache clears I tried, and I thought I was losing my mind until it dawned on me to search the issue queue. I can also confirm that adding a #[Hook('...')] to the class does seem to cause it to be discovered.

There's a module for that: https://www.drupal.org/project/image_style_warmer →

Added screenshots.

ambient.impact → created an issue.

Added videos.

Added screenshots.

ambient.impact → created an issue.

Ah, I see I'm not the only one that was utterly confused by the cryptic "LoadJS" error. It turns out the library did this to keep the size down, which I personally feel was a bad move because it's so confusing and non-helpful. I opened a pull request to change it, but since they still want to keep the size down, they're only willing to throw the bundle ID as the error message rather than a fully descriptive thing that says why it was thrown. Glad to see this is being handled on the Drupal side.

Not to dunk on the author of that even more, but I'm in agreement with @fathershawn, and would also highlight:

An attractive future direction might be to re-implement Htmx in React:

• The server sends a JSON blob that React converts into virtual DOM
  components.

...which baffles me because it sounds like this person doesn't understand the core philosophy behind htmx and similar approaches. Sending a JSON blob for React to convert into a virtual DOM has nothing to do with htmx, and it's in fact entirely antithetical to htmx as I understand it.

Even after applying the patch from the merge request and clearing caches, I'm still getting the fatal error. Is there a step I'm forgetting to get this to work?

Added Turbo to HTMX guide link.

It's worth taking a quick look at Turbo, if only to get a sense of what we would have to recreate with HTMX. I did a bit of poking around the HTMX docs just now and realized they have a guide for migrating from Hotwire Turbo, so that'll be handy.

@fathershawn Most of that I'm aware of. I think the biggest challenge in implementing the current functionality in HTMX is that we'll have to re-implement a lot of stuff that Hotwire Turbo does out of the box, but building on your module will help get there a bit faster when I (or someone reading this?) have the time and energy for it.

My impression is that HTMX seems to more of a low level toolkit that you build with - would you say that's accurate?

ambient.impact → created an issue.

If someone is willing to implement it in a robust and reliable way, I'd be happy to review and merge it. I can see the value of it, but my time and energy are limited right now so it's not a priority for me.

ambient.impact → created an issue.

I took a quick look but Pinto seems to require more set up than I would like. I feel that it's missing what I love about Hux: I can just install Hux, drop one or more classes into src/Hooks with concise PHP attributes, and it automagically works. I don't know enough about the inner workings of Drupal's theme system, so maybe that's just not currently possible, but I feel like Hux has the winning formula from a DX perspective.

Doing a bit of research on this and came across the Preprocess module → , which seems pretty straightforward to use until core or someone else comes up with a better solution. It's not quite as elegant as Hux, but it's not terrible either.

Moved 📌 Turbo: Drastically simplify installation for end users Active to postponed as immediate goal is achieved.

We still need a proper build process as detailed in the issue summary but the commits above are good enough for now and accomplish the immediate goal.

Moved 🌱 Turbo: Tracking issue of core and contrib behaviours that need fixes to correctly detach and then attach Active to ongoing from blocking.

Marked the core/misc/announce.js item as complete.

Added 📌 Turbo: Drastically simplify installation for end users Active to blocking issues.

ambient.impact → created an issue.

Removed 📌 Turbo: Configuration and form for toggling Turbo features Active as it's not really relevant to a minimum viable implementation right now, though might be done down the road.

ambient.impact → created an issue.

Added 🐛 Turbo: Drupal messages are lost if Turbo does a full reload Active .

Removed 🐛 Turbo: Remove behaviours from Drupal.behaviors when Turbo removes their Active from issue summary.

Added postponed issues and ongoing issues lists, and moved linked issues where appropriate.

Removed some lingering old stuff that's no longer relevant.

Significant restructure of issue summary; moved the old stuff to a section at the end as archived notes.

Going to close this as won't fix because there's no realistic way we could do this without breaking various things.

Moved 📌 Turbo: Automated tests Active to non-blocking.

Added item for Drupal\refreshless_turbo\Value\RefreshlessRequest.

Added list item for page cache request policy.

Removed session item (not actually true) and added page cache request policy item.

Added References section with links to the HTMX module.

Added a few remaining tasks from my comments.

Moved 🐛 Turbo: JS aggregation causes the RefreshLess JavaScript to evaluate more and more times on navigation Active and 🐛 Turbo: Drupal messages are lost if Turbo does a full reload Active to blocking issues.

Link to 📌 Turbo: Automated tests Active for test details.

Added CSS and JS aggregation list items.

Changed ⌛️ emoji to ⏳ as the former is a finished hourglass while the latter is what we want: an in-progress hourglass.

Reordered "Remaining tasks" and "Follow up tasks", and moved some stuff between them where they make more sense.

Updated status of a few items.

ambient.impact → created an issue.

Whoops, it looks like I accidentally replaced the issue summary with my comment. One of the maintainers should revert that if possible. 😬

Minor grammar fix.

Added remaining tasks and follow up tasks.

Heavy rewrite of issue summary reflecting current understanding of the problem and solution.

Added 🐛 Turbo: Drupal messages are lost if Turbo does a full reload Active to non-blocking tasks.

Updating title as we now know what needed to be done and why.

ambient.impact → created an issue.

In my case, I have a couple of sites with complex content that makes heavy use of the caching API (cache tags and cache contexts) and we try to avoid clearing the cache unless absolutely necessary. We even have a custom drush.yml with this:

command:

  updatedb:
    options:
      # Don't clear the cache by default to avoid unnecessary work.
      #
      # @see https://www.bounteous.com/insights/2020/03/12/automated-drupal-deployment-and-rollback-recipes/#don%E2%80%99t-clear-all-caches-instead-selectively-clear-what-you-need
      cache-clear: 0

Which I suspect may be part of the reason we also got hit with this Drupal\config_split\Plugin\ConfigFilter\SplitFilter error, despite following the 2.0.0 instructions and had drupal/config_filter required explicitly. Thankfully, this happened on a staging site so I could just restore a database back up and try a few things. What ended up working in our case to avoid a full cache clear was:

1. Upgrade Config Split in a dev environment; update database; export config; commit/push.
2. On the target site (staging/production) do drush pm:uninstall config_split
3. Deploy the updated codebase and config files.
4. drush -y updb and then drush -y config:import (our deployment process does this automatically)
5. Config Split 2.x is now installed and using the 2.x config without a problem.

The workaround I've committed is okay for now, but still results in brief flashes of incorrect specificity, so I'm going to set this as postponed for the time being.

Linking screenshots in issue summary.

Added screenshots demonstrating the issue.

Alright, so I think I have it mostly working. It should now be aggregating and
attaching only what's not already on the page, with the following caveats/problems yet to be resolved:

• Our module JavaScript is set to preprocess: false for now because it doesn't seem possible to group ourselves into a separate aggregate; see ✨ [PP-1] Allow setting aggregation groups for js files in library definitions Postponed . Ideally, we would be grouped into one aggregate that contains just the refreshless_turbo/refreshless and refreshless_turbo/turbo libraries so that we could take advantage of core's minification and gzipping.
• We have to special case the refreshless_turbo/refreshless and refreshless_turbo/turbo libraries to ensure Turbo is always attached even with the additive libraries logic, but never attached or aggregated more than once because bad things happen when Turbo loads and evaluates itself a second time.
• There's some duplicate logic between the middleware and our decorated asset resolver that I'm not happy with but I couldn't get it work well enough without it. I'll probably rewrite some of it into an attachments processor (see the HTMX module's HtmxResponseAttachmentsProcessor) to hopefully remove the duplication.
• This re-introduces the same CSS specificity symptoms as seen in 🐛 Turbo: CSS files merged into the on Turbo navigation always appended instead of preserving full page load order, causing CSS specificity issues Active but now when aggregation is enabled as well: the cause now seems to be that the base theme's (Claro) CSS for dropbuttons and vertical tabs is additively aggregated (since they're defined in separate libraries and not on every page) while Gin compiles dropbuttons into its big gin.css that gets attached to every page and uses a fixed weight value on its tabs.css library definition rather than listing Claro's as a dependency. The best course of action may be to rework the additive aggregation so that it only applies to JavaScript and not libraries as a whole, leaving CSS to aggregate as it did before, non-additively; this may be doable via our decorated asset resolver in the getCssAssets() method by checking if the current request is a Turbo request and then temporarily removing the already loaded libraries, calling $this->decorated->getCssAssets(), then restoring the already loaded libraries before returning the CSS assets.
• It's unclear if this needs a cache context because this seems to be working with caching, but my gut is saying that we should have a cache context and set up a clear distinction between a full page load and a Turbo/additive load, because there's a bunch of stuff in the code that just YOLOs it without a care which might backfire in the future.

So it still needs work but some progress has been made. The hardest part of this is working without an easy way to visualize what libraries are being (additively) aggregated and attached to a page. Setting up some tests for that would help, but I would also want to implement console logging of what new libraries get added, much like the 8.x-1.x branch has, and may something a bit more visual would help as well.

Added section discussing the HTMX contrib module → .

Merged and setting back to active. Thank you, bot.