-
Notifications
You must be signed in to change notification settings - Fork 423
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ensure Scope
is connected before accessing outlets
#648
Ensure Scope
is connected before accessing outlets
#648
Conversation
Would it be possible to include a test scenario that exercises this behavior? |
yeah, I'm working on that, that's also why the PR is a draft 🙈 I just wanted to get this posted so that people can see the progress on it while being as transparent as possible about it |
Still wondering what this would do with lazy controllers ? Just delay the finish of the initial controller a lot ? |
@jorismak I would imagine that the order how they are feteched is different. It would just pause the connect() of the initial controller at the point where you try to access the outlets when the outlet controller wasn't connected already. But I don't see how it would have a negative impact or a different behavior compared to how it worked before. Otherwise it would be more than appreciated if you could report back how the dev-build works out for you! 🙏🏼 |
…re-scope-is-connected-before-accessing-outlets
b94a694
to
15a3675
Compare
The good news is that this is also working with lazy-loading. There isn't really a way we can test the lazy-loading behavior when controllers are referencing unloaded/unregistered outlet controllers in Since the (lazy)-loading of Stimulus controllers via Importmaps can't be controlled and is up to the device/browser/network connection it's possible that there might be some cases where the controllers aren't loading in time and it would still reference an empty outlets array when trying to access There are a few ways to avoid this from happening:
|
This is awesome @marcoroth . Will there be a new version cut soon that includes this? I think the last release was in November of last year if package.json is right? 8ab6e59 |
Hey @zachfeldman, yeah that's right. We are working to get a new release out soon. There are still some open things we want to get resolved first though. If you need this fix right now, you can use the latest dev-builds with: yarn add @hotwired/stimulus@https://github.com/hotwired/dev-builds/archive/refs/tags/@hotwired/stimulus/2a3603c.tar.gz I hope this helps. Thanks for your understanding! 🙏🏼 |
@marcoroth awesome thanks so much for the update and instructions on how to run the bleeding edge! We're using the built in function to fire when outlets are run as a workaround for now but are looking forward to a release landing with this feature soon. Thanks for all that you do for Stimulus!! |
Hmm, I still seem to seeing this issue (stimulus-rails 1.2.2 which appears to have this fix from https://github.com/hotwired/stimulus/releases/tag/v3.2.2) when referencing outlets from the parent's connect some of the time. I'll try to put together a minimal example and confirm it's not due to something else I'm doing 🤔 |
Yeah, I think there still might be some edge cases where we cannot 100% guarantee that your code is running in the right order when you are using import-maps, especially combined with lazy-loading, just because of the nature of the technology. |
fwiw I'm using import maps and the particular example isn't lazy loaded. I'm confused as to why the callback fires if the controllers aren't yet instantiated. Is there a lower level function I could be using manually as a work around? I'm trying to grok it now. Will update if I find anything. 😅 Appreciate all your efforts, btw. |
Which callbacks are you referring to? The The best way to make sure all outlet controllers are available is to not access the outlets in But I'm afraid that it would be best if you could provide us with a minimal reproduction setup to be certain. |
@marcoroth I just faced the same issue. I am accessing an outlet in the I am happy to debug this together in the next days, just reach me out via slack. I think it makes sense...I also understand why this happens. I guess I have to rethink the logic behind my controller. It makes completely sense that within the |
The fix does not seem to work in conjunction with stimulus-rails. I circumvented this by exporting all controllers in import "@hotwired/turbo-rails";
import { Application } from "@hotwired/stimulus";
import * as Controllers from "controllers";
// Setup stimulus application
const application = Application.start();
application.debug = true;
window.Stimulus = application;
// Register all stimulus controllers
Object.keys(Controllers).forEach((key) => {
const name = (key[0].toLowerCase() + key.slice(1))
.replace("Controller", "")
.replace(/([A-Z])/g, "-$1")
.toLowerCase();
application.register(name, Controllers[key]);
}); |
This pull request ensures that the associated scope of an outlet controller is connected before evaluating and returning the outlet controllers.
The outlet getter returned nothing/raised an error when it was accessed within the
connect()
function of the host controller and if the controller elements were placed in a certain order in the DOM.The example below demonstrates this issue. If one tried to access the
this.resultOutlets
in theconnect()
action of thesearch
host controller it wouldn't return them. The elements with theresult
controllers weren't initialized/connected yet because they appear after the element with thesearch
controller in the DOM.Previously the connect order for this example looked like:
With this pull request the order looks like this:
Which allows that outlets can now be accessed in the
connect()
action of the host controller independent of in which order they might appear in the DOM.You can test this pull request with the following dev-build:
Resolves #618