Skip to content

Commit

Permalink
Ensure Scope is connected before accessing outlets (#648)
Browse files Browse the repository at this point in the history
* Ensure `Scope` is connected before accessing outlets
* Add tests for accessing outlets before they are initialized
* Also test connect order
  • Loading branch information
marcoroth committed Jun 19, 2023
1 parent 823cfee commit eda0f9d
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 26 deletions.
71 changes: 45 additions & 26 deletions src/core/outlet_properties.ts
Expand Up @@ -10,26 +10,42 @@ export function OutletPropertiesBlessing<T>(constructor: Constructor<T>) {
}, {} as PropertyDescriptorMap)
}

function getOutletController(controller: Controller, element: Element, identifier: string) {
return controller.application.getControllerForElementAndIdentifier(element, identifier)
}

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 outlet = this.outlets.find(name)

if (outlet) {
const outletController = this.application.getControllerForElementAndIdentifier(outlet, name)
if (outletController) {
return outletController
} else {
throw new Error(
`Missing "${this.application.schema.controllerAttribute}=${name}" attribute on outlet element for "${this.identifier}" 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}"`
)
}

throw new Error(`Missing outlet element "${name}" for "${this.identifier}" controller`)
throw new Error(
`Missing outlet element "${name}" for host controller "${this.identifier}". Stimulus couldn't find a matching outlet element using selector "${selector}".`
)
},
},

Expand All @@ -39,16 +55,15 @@ function propertiesForOutletDefinition(name: string) {

if (outlets.length > 0) {
return outlets
.map((outlet: Element) => {
const controller = this.application.getControllerForElementAndIdentifier(outlet, name)
if (controller) {
return controller
} else {
console.warn(
`The provided outlet element is missing the outlet controller "${name}" for "${this.identifier}"`,
outlet
)
}
.map((outletElement: Element) => {
const outletController = getControllerAndEnsureConnectedScope(this, outletElement, name)

if (outletController) return outletController

console.warn(
`The provided outlet element is missing an outlet controller "${name}" instance for host controller "${this.identifier}"`,
outletElement
)
})
.filter((controller) => controller) as Controller[]
}
Expand All @@ -59,11 +74,15 @@ function propertiesForOutletDefinition(name: string) {

[`${camelizedName}OutletElement`]: {
get(this: Controller) {
const outlet = this.outlets.find(name)
if (outlet) {
return outlet
const outletElement = this.outlets.find(name)
const selector = this.outlets.getSelectorForOutletName(name)

if (outletElement) {
return outletElement
} else {
throw new Error(`Missing outlet element "${name}" for "${this.identifier}" controller`)
throw new Error(
`Missing outlet element "${name}" for host controller "${this.identifier}". Stimulus couldn't find a matching outlet element using selector "${selector}".`
)
}
},
},
Expand Down
10 changes: 10 additions & 0 deletions src/core/router.ts
Expand Up @@ -75,6 +75,16 @@ export class Router implements ScopeObserverDelegate {
}
}

proposeToConnectScopeForElementAndIdentifier(element: Element, identifier: string) {
const scope = this.scopeObserver.parseValueForElementAndIdentifier(element, identifier)

if (scope) {
this.scopeObserver.elementMatchedValue(scope.element, scope)
} else {
console.error(`Couldn't find or create scope for identifier: "${identifier}" and element:`, element)
}
}

// Error handler delegate

handleError(error: Error, message: string, detail: any) {
Expand Down
4 changes: 4 additions & 0 deletions src/core/scope_observer.ts
Expand Up @@ -42,6 +42,10 @@ export class ScopeObserver implements ValueListObserverDelegate<Scope> {

parseValueForToken(token: Token): Scope | undefined {
const { element, content: identifier } = token
return this.parseValueForElementAndIdentifier(element, identifier)
}

parseValueForElementAndIdentifier(element: Element, identifier: string): Scope | undefined {
const scopesByIdentifier = this.fetchScopesByIdentifierForElement(element)

let scope = scopesByIdentifier.get(identifier)
Expand Down
6 changes: 6 additions & 0 deletions src/tests/controllers/outlet_controller.ts
Expand Up @@ -19,6 +19,7 @@ export class OutletController extends BaseOutletController {
alphaOutletDisconnectedCallCount: Number,
betaOutletConnectedCallCount: Number,
betaOutletDisconnectedCallCount: Number,
betaOutletsInConnect: Number,
gammaOutletConnectedCallCount: Number,
gammaOutletDisconnectedCallCount: Number,
namespacedEpsilonOutletConnectedCallCount: Number,
Expand Down Expand Up @@ -46,11 +47,16 @@ export class OutletController extends BaseOutletController {
alphaOutletDisconnectedCallCountValue = 0
betaOutletConnectedCallCountValue = 0
betaOutletDisconnectedCallCountValue = 0
betaOutletsInConnectValue = 0
gammaOutletConnectedCallCountValue = 0
gammaOutletDisconnectedCallCountValue = 0
namespacedEpsilonOutletConnectedCallCountValue = 0
namespacedEpsilonOutletDisconnectedCallCountValue = 0

connect() {
this.betaOutletsInConnectValue = this.betaOutlets.length
}

alphaOutletConnected(_outlet: Controller, element: Element) {
if (this.hasConnectedClass) element.classList.add(this.connectedClass)
this.alphaOutletConnectedCallCountValue++
Expand Down
45 changes: 45 additions & 0 deletions src/tests/modules/core/outlet_order_tests.ts
@@ -0,0 +1,45 @@
import { ControllerTestCase } from "../../cases/controller_test_case"
import { OutletController } from "../../controllers/outlet_controller"

const connectOrder: string[] = []

class OutletOrderController extends OutletController {
connect() {
connectOrder.push(`${this.identifier}-${this.element.id}-start`)
super.connect()
connectOrder.push(`${this.identifier}-${this.element.id}-end`)
}
}

export default class OutletOrderTests extends ControllerTestCase(OutletOrderController) {
fixtureHTML = `
<div data-controller="alpha" id="alpha1" data-alpha-beta-outlet=".beta">Search</div>
<div data-controller="beta" id="beta-1" class="beta">Beta</div>
<div data-controller="beta" id="beta-2" class="beta">Beta</div>
<div data-controller="beta" id="beta-3" class="beta">Beta</div>
`

get identifiers() {
return ["alpha", "beta"]
}

async "test can access outlets in connect() even if they are referenced before they are connected"() {
this.assert.equal(this.controller.betaOutletsInConnectValue, 3)

this.controller.betaOutlets.forEach((outlet) => {
this.assert.equal(outlet.identifier, "beta")
this.assert.equal(Array.from(outlet.element.classList.values()), "beta")
})

this.assert.deepEqual(connectOrder, [
"alpha-alpha1-start",
"beta-beta-1-start",
"beta-beta-1-end",
"beta-beta-2-start",
"beta-beta-2-end",
"beta-beta-3-start",
"beta-beta-3-end",
"alpha-alpha1-end",
])
}
}

0 comments on commit eda0f9d

Please sign in to comment.