-
Notifications
You must be signed in to change notification settings - Fork 426
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
disconnect() not called if outlet is accessed in connect() #763
Comments
Hi. Looks like the problem occurs because of Outlet#get. So when you load outlet inside of parent controller there is no associated controller for child outlet element and stimulus tries to connect controller with element before show warning message function getControllerAndEnsureConnectedScope(controller: Controller, element: Element, outletName: string) {
let outletController = getOutletController(controller, element, outletName)
if (outletController) return outletController
controller.application.router.proposeToConnectScopeForElementAndIdentifier(element, outletName)
...
} https://github.com/hotwired/stimulus/blob/main/src/core/outlet_properties.ts#L21 |
I'm not sure that's the right track. At least I'm not seeing the warning in the console 🤔 Edit: It indeed seems to be related. In a similar situation I do get the warning. But I haven't investigated yet what the difference to the example in the issue is and why I'm not seeing the warning there. |
please check this function getControllerAndEnsureConnectedScope(controller: Controller, element: Element, outletName: string) {
let outletController = getOutletController(controller, element, outletName)
if (outletController) return outletController
controller.application.router.proposeToConnectScopeForElementAndIdentifier(element, outletName)
outletController = getOutletController(controller, element, outletName)
if (outletController) return outletController
}
function propertiesForOutletDefinition(name: string) {
const camelizedName = namespaceCamelize(name)
return {
[`${camelizedName}Outlet`]: {
get(this: Controller) {
const outletElement = this.outlets.find(name)
const selector = this.outlets.getSelectorForOutletName(name)
if (outletElement) {
const outletController = getControllerAndEnsureConnectedScope(this, outletElement, name)
if (outletController) return outletController
throw new Error(
`The provided outlet element is missing an outlet controller "${name}" instance for host controller "${this.identifier}"`
)
}
...
}
by my opinion calling outlet inside |
Mh, I can't reproduce the case where I saw the
@ildarkayumov do you see that error when running the provided example? Or can you provide an example which causes the error to be thrown? I did find out one more detail: The example also works as expected if <!-- pacing child here also works 💥 -->
<div id="child" data-controller="child"></div>
<div id="parent" data-controller="parent" data-parent-child-outlet="#child">
<button data-action="click->parent#listChildren">List children</button>
</div> Regarding the question if accessing outlets in
If we follow that, then it should be documented, should throw an error and not work 50:50 depending on the structure of the HTML. However I'd argue that it should work. Accessing child controller properties to set an initial state (either in the parent controller or in the child controller) seems like a legitimate use case to me. The current documentation for outlets does exactly that: https://stimulus.hotwired.dev/reference/outlets#definitions // chat_controller.js
export default class extends Controller {
static outlets = [ "user-status" ]
connect () {
this.userStatusOutlets.forEach(status => ...)
}
} @marcoroth maybe you want to chime in? 😇 |
Well, I am able to reproduce the |
@sebastianludwig yes, you are right - there is a bug. |
it also affects other setups e.g. when an action on the parent controller is triggered fast enough |
I noticed that accessing an outlet in
connect()
causes the reference count of that controller to be 2 instead of 1 after start has finished. That means all actions which under normal circumstances cause the controller to be disconnected (like removing thedata-controller
attribute) don't cause the disconnect anymore.The following code snippet illustrates the problem
Setting a breakpoint in
ScopeObserver.elementMatchedValue()
and reloading the page you can see that it is hit twice forchild
. Clicking the remove button does not triggerdisconnect()
to be called. Everything works as expected with the marked line commented out.The text was updated successfully, but these errors were encountered: