[proposal] Removing unused stylesheets and scripts in head

[joelmoss]

So I understand that any new stylesheets and scripts in the head of the new page will be appended to the current head. Which is great, but what's not so great, is that these appended assets stick around even when not needed. So if I have some page specific CSS or JS, they will persist way beyond the page they are used.

I also understand that the recommended solution to this is to include these page specific assets in the body, instead of the head.

This is however, not ideal. Particularly when it comes to styles, as styles are render blocking assets, resulting in a flash of unstyled content when loading a new page that includes CSS within the body.

I've been playing around and have found what I believe to be a better solution that allows me to include assets in the head, but cleans up any unused or page specific assets.

After Turbo merges the head, the stylesheets and scripts within the new and current snapshots are compared. Any current asset not found in the new snapshot is removed from the head. Something like this...

removeOldStylesheets() {
  const currentSheets = this.currentHeadSnapshot.getElementsMatchingType("stylesheet")
  const newSheets = this.newHeadSnapshot.getElementsMatchingType("stylesheet")

  currentSheets.forEach((element) => {
    if (!this.isCurrentElementInElementList(element, newSheets)) {
      document.head.removeChild(element)
    }
  })
}

This works nicely in my [limited] testing, but not so great if caching is enabled - I need to dig deeper into that. But before I do so, I wanted to get some feedback on this idea, and ensure that I've not overlooked anything.

Thanks, and any and all feedback is appreciated.

[afcapel]

Removing unused stylesheets is a great idea because the browser automatically updates the associated styles without needing a full page reload.

Removing JS is trickier. The JS state is not going to change after removing the outdated <script> tag. In that case we always want a full page reload, which is what data-turbo-track="reload" does.

But just being able to pick up new styles without a full page reload is a nice feature, will accept a PR implementing that 👍

[joelmoss]

Happy to send a PR, but I couldn't get it working with caching enabled. When I called document.head.removeChild(element), the page took a second or two to load.

Would appreciate some pointers there.

[joelmoss]

draft PR at #1088

[scuml]

Here's a solution that uses much less code, based on a head refactor I made a PR on. #832

scuml@78d8f52

The issue remains with the cache you mentioned (#1082 (comment)) is still at play here. I'm familiarizing myself with the cache mechanism now to see what can be done to fix that.

The code can be further optimized once the cached snapshot issue is figured out as the mergeHead content looks like it can be replaced with the newly available idiomorph:

Idiomorph.morph(this.currentSnapshot.headSnapshot.element, this.newSnapshot.headSnapshot.element, {
      callbacks: {
        beforeNodeAdded: this.#shouldAddElement,
        beforeNodeMorphed: this.#shouldMorphElement,
        beforeNodeRemoved: this.#shouldRemoveElement
      }}
    );
  #shouldAddElement = (node) => {
    return true
  }
  #shouldMorphElement = (oldNode, newNode) => {
  }
  #shouldRemoveElement = (node) => {
    return !node.hasAttribute("turbo-progress-bar");
  }

[joelmoss]

looks like we have a solution just merged from #1128. Not yet tried it though.

[afcapel]

Right, #1128 closes this.