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>
- send controller dispatches a connected event on
connect()
- 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?
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);
+ }
+ }
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.