themesberg / themesberg/flowbite

Flowbite's handling of Turbo is using the wrong strategy

Open
#796 39 comments 15 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
HTML
Stars
9.4k
Forks
862
PR merge metrics
No merged PRs in 30d

Description

tl;dr: The current approach for integrating Turbo with Flowbite is using a mismatched strategy. As far as I can tell, it will only ever work for a certain subset of Turbo-enabled web applications. Events don't work for every case and should be replaced with MutationObserver.


Details

Sorry for the long wall of text, but I believe this is necessary to understand the problem.

I spend some hours over the weekend to make Flowbite work with Turbo, but had no actual success. As soon as I used more than just the page cache feature, simple visits or most-of-the-page Turbo frames, I encountered dead ends.

The unpublished fix in https://github.com/themesberg/flowbite/pull/760 did help at first, but it then revealed a huge conceptual problem, which I think cannot be tackled with the current approach.

Let me explain 😃 Most previous issues regarding Turbo were about missing initialization on frame navigations or after stream actions, or about broken re-initialization: https://github.com/themesberg/flowbite/issues/450, https://github.com/themesberg/flowbite/pull/760, https://github.com/themesberg/flowbite/issues/698, https://github.com/themesberg/flowbite/issues/697, https://github.com/themesberg/flowbite/issues/454

They are fixed now, but they all are centered around a certain aspect:

Some component instances need to be initialized or reinitialized after a Turbo operation.

For a simple application, where Turbo is used mostly as a page cache, this is all you need.

But as soon as you start using Turbo in a more fine-grained approach, with many frames and stream actions, you will also encounter the opposite problem:

Some component instances must not be reinitialized after a Turbo operation.

There are no reported issues regarding this topic yet, besides the odd "no, this fix still doesn't work for me" comments.
(Most likely because fine-grained decomposed Turbo pages utilize stream actions, which didn't work until https://github.com/themesberg/flowbite/pull/760.)

Example

You have a modal, make it visible, and then update a completely unrelated portion of the DOM, either via frame navigation or a stream action. Then the subsequent initFlowbite/initModals will re-initialize all modals, even the one currently visible.

This leads to an inconsistent state of the page, because the new instance of Modal thinks, it is hidden, but the
DOM nodes of the modal still have the TailwindCSS classes, which make it visible. The backdrop is removed, too, so you are
left with kind of a "zombie" modal, not fully open, not fully closed.

Depending on the app (or the component) that might not be a problem, e.g. when no Modal is visible during a Turbo operation. Or when it is actually the modal, which is replaced during that operation, because then you get "fresh" HTML with the correct classes.

In apps where this actually is a problem, the idea to make initFlowbite/initModals idempotent (as proposed in https://github.com/themesberg/flowbite/issues/119#issuecomment-1703815359) only would help in those cases, where components are not replaced. In other words, in such cases where the DOM nodes of a component get replaced by a Turbo operation with HTML of the same component, then that component must actually be reinitialized, and an idempotent init function would wrongly skip it.

Why just Flowbite?

The reason why other frameworks don't have these problems is that their implementation of a component instance manager stores the instances together with the DOM node element, in Foundation indirectly using jQuery's data, for example. (See https://github.com/foundation/foundation-sites/blob/develop/js/foundation.core.plugin.js, https://github.com/jquery/jquery/blob/main/src/data/Data.js)

So on those frameworks, if any operation of Turbo replaces a part of the DOM, any affected component instances normally are gone as well, as opposed to Flowbite, where the DOM nodes and their respective component instances are loosely coupled only through an ID string. I like Flowbite's approach better, but it requires a more fine-grained approach, as I explain below.

The wrong strategy

The core of the problem lies in the event-based approach coupled with a full (re-)initialization after turbo:load, turbo:frame-load and now the custom turbo:after-stream-render event.

Turbo's fine-grained modifications of the DOM and the current catch-all approach of (re-)initializing everything on the page conceptually don't match.

A better strategy

The proper solution is to only (re-)initialize components in those parts of the DOM which actually have been changed.

I wrote a simple proof-of-concept handler for Turbo which utilizes MutationObserver and - as far as I can tell - only requires superficial modifications to the current Flowbite JS code:

const observer = new MutationObserver((mutationList, observer) => {
    const modals = window.FlowbiteInstances.getInstances('Modal') // Hack, because `instances` is not exported
    for (const mutation of mutationList) {
        // Destroy components from removed DOM nodes
        for (const root of mutation.removedNodes) {
            const ids = collectIDs(root) // Implemented elsewhere
            const removed = ids.map(id => modals[id]).filter(Boolean)
            removed.forEach(modal => modal.destroyAndRemoveInstance())
        }

        // Initialize components from added DOM nodes
        for (const root of mutation.addedNodes) {
            if (root.nodeType !== Node.ELEMENT_NODE) {
                continue // Most often a #text node
            }
            initModals(root) // <== New parameter required in Flowbite initializers
        }
    }
})

observer.observe(document.body, { childList: true, subtree: true })

As you can see, the overall principle is not that complicated, but it requires some modifications to the init* functions, most notably adding an optional parameter to specify which subtree to initialize.

The required changes look like this:

export function initModals(root = document) { // <== New optional parameter
    // initiate modal based on data-modal-target
    const targets = Array.from(root.querySelectorAll('[data-modal-target]'))
    // This part is ugly: `querySelectorAll` only considers child nodes and since a non-document node could already
    // be the component target, we must explicitly check the root node, too.
    if (root.matches('[data-modal-target]')) {
        targets.push(root)
    }
    targets.forEach(function ($triggerEl) {
        var modalId = $triggerEl.getAttribute('data-modal-target');
        var $modalEl = document.getElementById(modalId); //
        // ...
    }
    // ...
}

I tested this extensively with modals, and it seems to work much better:

  • New nodes/modals are initialized.
  • Replaced nodes/modals are destroyed then initialized again.
  • Removed nodes/modals are destroyed.
  • Unchanged nodes/modals remain untouched.

Most likely, my code can be optimized and perhaps there are some edge cases I didn't consider, especially in the other components I didn't test.

The beauty of this solution is that it is almost completely agnostic to Turbo and should work with every other library which modifies the DOM in a similar fashion.

The only Turbo-specific thing is the event name for the initial page load:

document.addEventListener('turbo:load', () => {
    initFlowbite()
    observeChanges()
})

On the other hand, I'm not sure how my solution behaves in conjunction with JS frameworks like Vue or React.
I can imagine there might be conflicts, but this might also make some Vue wrappers obsolete, especially if their only purpose is fine-grained initialization.

But I do have some experience with Stimulus:
Before coming to this solution, I used a Stimulus controller for manual initialization (i.e. explicitly calling new Modal) to tackle the problem described above. This worked well, but the observer approach made it obsolete, which is not surprising, since Stimulus also uses MutationObserver. (It now only provides an "open modal after fame load" feature that I wanted, since that specific feature was removed from Flowbite https://github.com/themesberg/flowbite/pull/141#issuecomment-1925815453 )

As much as https://github.com/themesberg/flowbite/pull/760 was pivotal in finding a proper solution, I must admit that the Turbo developers were right in not adding that event. I once asked for a similar feature in order to apply the same "sledgehammer" approach (https://github.com/hotwired/turbo/pull/425#issuecomment-1246358996 😌)

Final words

Thank you for reading this. I'm hoping my proposal can be integrated into Flowbite. The changes should be small, albeit at numerous locations, i.e. all init functions. They should not affect non-Turbo users at all, too, but they would provide proper support for all Turbo-related use cases.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Review the current event-based initialization around turbo:load, turbo:frame-load, and turbo:after-stream-render, then inspect the init* functions and FlowbiteInstances lifecycle. Use the MutationObserver proof of concept as the starting point. Done means added and replaced nodes initialize correctly, removed nodes are destroyed, and unchanged components remain untouched.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, tailwindcss
Domain
frontend, web-dev
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.