diff --git a/packages/@stimulus/core/src/application.ts b/packages/@stimulus/core/src/application.ts index dc17ee0f..df52be88 100644 --- a/packages/@stimulus/core/src/application.ts +++ b/packages/@stimulus/core/src/application.ts @@ -13,6 +13,7 @@ export class Application implements ErrorHandler { readonly router: Router logger: Logger = console debug: boolean = false + warnings: boolean = true static start(element?: Element, schema?: Schema): Application { const application = new Application(element, schema) @@ -71,6 +72,14 @@ export class Application implements ErrorHandler { return context ? context.controller : null } + // Warning handling + + handleWarning(warning: string, message: string, detail: object) { + if (this.warnings) { + this.logger.warn(`%s\n\n%s\n\n%o`, message, warning, detail) + } + } + // Error handling handleError(error: Error, message: string, detail: object) { diff --git a/packages/@stimulus/core/src/binding_observer.ts b/packages/@stimulus/core/src/binding_observer.ts index 1456cd21..6356054c 100644 --- a/packages/@stimulus/core/src/binding_observer.ts +++ b/packages/@stimulus/core/src/binding_observer.ts @@ -59,8 +59,18 @@ export class BindingObserver implements ValueListObserverDelegate { private connectAction(action: Action) { const binding = new Binding(this.context, action) + const { controller } = binding.context + const method = (controller as any)[action.methodName] + this.bindingsByAction.set(action, binding) this.delegate.bindingConnected(binding) + + if (typeof method != "function") { + this.context.handleWarning( + `Action "${action.toString()}" references undefined method "${action.methodName}" on controller "${action.identifier}"`, + `connecting action "${action.toString()}"` + ) + } } private disconnectAction(action: Action) { @@ -92,4 +102,12 @@ export class BindingObserver implements ValueListObserverDelegate { elementUnmatchedValue(element: Element, action: Action) { this.disconnectAction(action) } + + elementMatchedNoValue(token: Token) { + const action = Action.forToken(token) + this.context.handleWarning( + `Action "${token.content}" references undefined controller "${action.identifier}"`, + `connecting action "${token.content}"` + ) + } } diff --git a/packages/@stimulus/core/src/context.ts b/packages/@stimulus/core/src/context.ts index 49b8f6a6..2605fa4d 100644 --- a/packages/@stimulus/core/src/context.ts +++ b/packages/@stimulus/core/src/context.ts @@ -6,12 +6,14 @@ import { ErrorHandler } from "./error_handler" import { Module } from "./module" import { Schema } from "./schema" import { Scope } from "./scope" +import { TargetGuide } from "./target_guide" import { ValueObserver } from "./value_observer" export class Context implements ErrorHandler { readonly module: Module readonly scope: Scope readonly controller: Controller + readonly targetGuide: TargetGuide private bindingObserver: BindingObserver private valueObserver: ValueObserver @@ -19,6 +21,7 @@ export class Context implements ErrorHandler { this.module = module this.scope = scope this.controller = new module.controllerConstructor(this) + this.targetGuide = new TargetGuide(this.scope, this.controller) this.bindingObserver = new BindingObserver(this, this.dispatcher) this.valueObserver = new ValueObserver(this, this.controller) @@ -86,7 +89,13 @@ export class Context implements ErrorHandler { this.application.handleError(error, `Error ${message}`, detail) } - // Debug logging + // Logging + + handleWarning(warning: string, message: string, detail: object = {}) { + const { identifier, controller, element } = this + detail = Object.assign({ identifier, controller, element }, detail) + this.application.handleWarning(warning, `Warning ${message}`, detail) + } logDebugActivity = (functionName: string, detail: object = {}): void => { const { identifier, controller, element } = this diff --git a/packages/@stimulus/core/src/router.ts b/packages/@stimulus/core/src/router.ts index 8db0c92b..60cda1d3 100644 --- a/packages/@stimulus/core/src/router.ts +++ b/packages/@stimulus/core/src/router.ts @@ -59,6 +59,7 @@ export class Router implements ScopeObserverDelegate { unloadIdentifier(identifier: string) { const module = this.modulesByIdentifier.get(identifier) + if (module) { this.disconnectModule(module) } @@ -84,10 +85,17 @@ export class Router implements ScopeObserverDelegate { } scopeConnected(scope: Scope) { - this.scopesByIdentifier.add(scope.identifier, scope) - const module = this.modulesByIdentifier.get(scope.identifier) + const { identifier, element } = scope + this.scopesByIdentifier.add(identifier, scope) + const module = this.modulesByIdentifier.get(identifier) if (module) { module.connectContextForScope(scope) + } else { + this.application.handleWarning( + `Element references undefined controller "${identifier}"`, + `Warning connecting controller "${identifier}"`, + { identifier, element } + ) } } diff --git a/packages/@stimulus/core/src/scope_observer.ts b/packages/@stimulus/core/src/scope_observer.ts index 85f9746f..541b4f0c 100644 --- a/packages/@stimulus/core/src/scope_observer.ts +++ b/packages/@stimulus/core/src/scope_observer.ts @@ -71,6 +71,8 @@ export class ScopeObserver implements ValueListObserverDelegate { } } + elementMatchedNoValue(token: Token) {} + private fetchScopesByIdentifierForElement(element: Element) { let scopesByIdentifier = this.scopesByIdentifierByElement.get(element) if (!scopesByIdentifier) { diff --git a/packages/@stimulus/core/src/target_guide.ts b/packages/@stimulus/core/src/target_guide.ts new file mode 100644 index 00000000..12692644 --- /dev/null +++ b/packages/@stimulus/core/src/target_guide.ts @@ -0,0 +1,123 @@ +import { Controller } from "./controller" +import { Constructor } from "./constructor" +import { Scope } from "./scope" +import { TargetDescriptor } from "./target_properties" +import { readInheritableStaticArrayValues } from "./inheritable_statics" + +export class TargetGuide { + readonly scope: Scope + readonly controller: Controller + readonly definedTargets: Array + + constructor(scope: Scope, controller: Controller) { + this.scope = scope + this.controller = controller + this.definedTargets = readInheritableStaticArrayValues(this.controller.constructor as Constructor, "targets") + + this.searchForUndefinedTargets() + } + + get identifier() { + return this.scope.identifier + } + + get targets() { + return this.scope.targets + } + + get registeredControllers(): Array { + return this.controller.application.router.modules.map((c) => c.identifier) + } + + controllerRegistered(controllerName: string): Boolean { + return this.registeredControllers.includes(controllerName) + } + + targetDefined(targetName: string): Boolean { + return this.definedTargets.includes(targetName) + } + + private getAllTargets(element: Element): Array { + const attribute = `data-${this.identifier}-target` + const selector = `[${attribute}]` + + return Array.from(element.querySelectorAll(selector)).map((element: Element) => { + const target = element.getAttribute(attribute) + return { + identifier: this.identifier, + target, + element, + attribute, + legacy: false + } + }) + } + + private getAllLegacyTargets(element: Element): Array { + const attribute = "data-target" + const selector = `[${attribute}]` + + return Array.from(element.querySelectorAll(selector)).map((element: Element) => { + const value = element.getAttribute(attribute) + const parts = value ? value.split(".") : [] + return { + identifier: parts[0], + target: parts[1], + element, + attribute, + legacy: true + } + }) + } + + private searchForUndefinedTargets() { + const { element } = this.scope + + const targets: Array = [ + ...this.getAllTargets(element), + ...this.getAllLegacyTargets(element) + ] + + targets.forEach((descriptor) => { + const { identifier, attribute, target, legacy, element } = descriptor + + if (identifier && target) { + this.handleWarningForUndefinedTarget(descriptor) + } else if (identifier && !target) { + this.controller.context.handleWarning( + `The "${attribute}" attribute of the Element doesn't include a target. Please specify a target for the "${identifier}" controller.`, + `connecting target for "${identifier}"`, + { identifier, target, attribute, element } + ) + } else if (legacy && !target) { + this.controller.context.handleWarning( + `The "${attribute}" attribute of the Element doesn't include a value. Please specify a controller and target value in the right format.`, + `connecting target`, + { identifier, target, attribute, element } + ) + } + }) + } + + private handleWarningForUndefinedTarget(descriptor: TargetDescriptor) { + const { identifier, target, element, attribute } = descriptor + + if (identifier === this.identifier) { + if (!this.targetDefined(target as string)) { + this.controller.context.handleWarning( + `Element references undefined target "${target}" for controller "${identifier}". Make sure you defined the target "${target}" in the "static targets" array of your controller.`, + `connecting target "${identifier}.${target}"`, + { identifier, target, element, attribute } + ) + } + } else { + if (!this.controllerRegistered(identifier as string)) { + this.controller.context.handleWarning( + `Target "${target}" references undefined controller "${identifier}". Make sure you registered the "${identifier}" controller.`, + `connecting target "${identifier}.${target}"`, + { identifier, target, element, attribute } + ) + } + } + } +} diff --git a/packages/@stimulus/core/src/target_properties.ts b/packages/@stimulus/core/src/target_properties.ts index 8aad1af6..e02fd921 100644 --- a/packages/@stimulus/core/src/target_properties.ts +++ b/packages/@stimulus/core/src/target_properties.ts @@ -10,6 +10,14 @@ export function TargetPropertiesBlessing(constructor: Constructor) { }, {} as PropertyDescriptorMap) } +export type TargetDescriptor = { + identifier: string | null + target: string | null + attribute: string + legacy: Boolean + element: Element +} + function propertiesForTargetDefinition(name: string) { return { [`${name}Target`]: { diff --git a/packages/@stimulus/core/src/tests/cases/application_test_case.ts b/packages/@stimulus/core/src/tests/cases/application_test_case.ts index cc516cf0..0649d925 100644 --- a/packages/@stimulus/core/src/tests/cases/application_test_case.ts +++ b/packages/@stimulus/core/src/tests/cases/application_test_case.ts @@ -16,6 +16,7 @@ export class ApplicationTestCase extends DOMTestCase { try { this.setupApplication() this.application.start() + this.application.warnings = false await super.runTest(testName) } finally { this.application.stop() diff --git a/packages/@stimulus/mutation-observers/src/tests/cases/value_list_observer_tests.ts b/packages/@stimulus/mutation-observers/src/tests/cases/value_list_observer_tests.ts index 242dfb7c..7b1852c3 100644 --- a/packages/@stimulus/mutation-observers/src/tests/cases/value_list_observer_tests.ts +++ b/packages/@stimulus/mutation-observers/src/tests/cases/value_list_observer_tests.ts @@ -103,6 +103,10 @@ export default class ValueListObserverTests extends ObserverTestCase implements this.recordCall("elementMatchedValue", element, value.id, value.token.content) } + elementMatchedNoValue(token: Token) { + this.recordCall("elementMatchedNoValue", token) + } + elementUnmatchedValue(element: Element, value: Value) { this.recordCall("elementUnmatchedValue", element, value.id, value.token.content) } diff --git a/packages/@stimulus/mutation-observers/src/value_list_observer.ts b/packages/@stimulus/mutation-observers/src/value_list_observer.ts index f1cf28be..2ba2fb94 100644 --- a/packages/@stimulus/mutation-observers/src/value_list_observer.ts +++ b/packages/@stimulus/mutation-observers/src/value_list_observer.ts @@ -4,6 +4,7 @@ export interface ValueListObserverDelegate { parseValueForToken(token: Token): T | undefined elementMatchedValue(element: Element, value: T): void elementUnmatchedValue(element: Element, value: T): void + elementMatchedNoValue(token: Token): void } interface ParseResult { @@ -54,6 +55,8 @@ export class ValueListObserver implements TokenListObserverDelegate { if (value) { this.fetchValuesByTokenForElement(element).set(token, value) this.delegate.elementMatchedValue(element, value) + } else { + this.delegate.elementMatchedNoValue(token) } } diff --git a/packages/@stimulus/polyfills/index.js b/packages/@stimulus/polyfills/index.js index 5601543d..30f9808a 100644 --- a/packages/@stimulus/polyfills/index.js +++ b/packages/@stimulus/polyfills/index.js @@ -1,6 +1,7 @@ import "core-js/es/array/find" import "core-js/es/array/find-index" import "core-js/es/array/from" +import "core-js/es/array/includes" import "core-js/es/map" import "core-js/es/object/assign" import "core-js/es/promise"