hotwired/stimulus-rails

stimulus-loading Race Condition

tleish opened this issue · 11 comments

In trying to use importmap with stimulus `stimulus-loading' and I've run into a challenge with race conditions.

For example, say I have the following code.

// send_controller.js
export default class extends Controller {
  connect() {
    this.dispatch('connected')
  }
}
// receive_controller.js
export default class extends Controller {
  catch() {
    console.log('catch');
  }
}
<div data-controller="receive send"
     data-action="send:connected->receive#catch"></div>
  1. send controller dispatches a connected event on connect()
  2. send controller event runs action receive#catch

In the case of importmap, sometimes the send controller loads and executes before receive controller loaded. The event is dispatched but nothing happens because the receive controller is not loaded.

I've explored other options:

Option 1: turbo:load@window->receive#catch

This works for visits, but not not if the content is loaded via turbo-frame or a FORM turbo-stream response with lazyLoadControllersFrom

Option 2: turbo:load@window->receive#catch turbo:frame-load@window->receive#catch

This works for visits and turbo-frame, but still not for FORM turbo-stream response with lazyLoadControllersFrom.

I thought perhaps I could tap into the solution outlined in #57, but the PR is closed.

Is there a solution I'm not aware of that works with visit, turbo-frame and turbo-stream scenarios? Perhaps an event which indicates that all the stimulus controllers have been loaded and ready to execute? Should stimulus hold off on executing javascript until controllers finish loading?

dhh commented

Hmm, not an easy problem to solve, because lazy loading is loading things dynamically. So some controllers might have been loaded already, and good to go. Others only load when new HTML appears that have them in there.

Seems like what we need is essentially turbo:finish or something like that which captures all forms of turbo loading. Do explore that in Turbo 👍

Do explore that in Turbo

The dynamic loading of the stimulus controllers occurs in the stimulus-loading.js
library in this codebase. Would it then make more sense to include the logic in the library which dynamically loads the stimulus controllers?

Basing it off this closed PR created by @seanpdoyle, the following modifications dispatches an event stimulus:ready which solves the issue for us:

function loadController(name, under, application) {
  if (!(name in registeredControllers)) {
+   registeredControllers[name] = false
    import(controllerFilename(name, under))
      .then(module => registerController(name, module, application))
-     .catch(error => console.error(`Failed to autoload controller: ${name}`, error))
+     .catch(error => importError(error, name))
+     .finally(()  => dispatchWhenReady())
  }
}

function registerController(name, module, application) {
- if (!(name in registeredControllers)) {
+ if (!registeredControllers[name]) {
    application.register(name, module.default)
    registeredControllers[name] = true
  }
}

+ function importError(error, name) {
+   delete registeredControllers[name]
+   console.error(`Failed to autoload controller: ${name}`, error)
+ }

+ function dispatchWhenReady() {
+   if(Object.values(registeredControllers).every(Boolean)) {
+     const controllers = Object.keys(registeredControllers);
+     const event = new CustomEvent('stimulus:ready', { detail: { controllers }, bubbles: true });
+     document.dispatchEvent(event);
+   }
+ }
dhh commented

Actually, yes, I do like that option too. I was thinking of turbo:finish, which I also think we should explore. But I'd take the stimulus:ready event too. That's a good idea.