diff --git a/src/core/application.ts b/src/core/application.ts index e6f920e7..c9f9548d 100644 --- a/src/core/application.ts +++ b/src/core/application.ts @@ -6,8 +6,9 @@ import { Logger } from "./logger" import { Router } from "./router" import { Schema, defaultSchema } from "./schema" import { ActionDescriptorFilter, ActionDescriptorFilters, defaultActionDescriptorFilters } from "./action_descriptor" +import { WarningHandler } from "./warning_handler" -export class Application implements ErrorHandler { +export class Application implements ErrorHandler, WarningHandler { readonly element: Element readonly schema: Schema readonly dispatcher: Dispatcher @@ -93,7 +94,7 @@ export class Application implements ErrorHandler { // Logging - handleWarning(warning: string, message: string, detail: object) { + handleWarning(warning: string, message: string, detail: any) { if (this.warnings) { this.logger.warn(`%s\n\n%s\n\n%o`, message, warning, detail) } diff --git a/src/core/context.ts b/src/core/context.ts index a7967f2a..bd54a7fc 100644 --- a/src/core/context.ts +++ b/src/core/context.ts @@ -146,6 +146,6 @@ export class Context implements ErrorHandler, TargetObserverDelegate, OutletObse } handleElementMatchedNoValue(element: Element, token: Token, error?: Error) { - this.application.router.scopeObserver.elementMatchedNoValue(element, token, error) + this.application.router.handleWarningsWithDuplicates(element, token, error) } } diff --git a/src/core/guide.ts b/src/core/guide.ts index e96a2293..8cbc401f 100644 --- a/src/core/guide.ts +++ b/src/core/guide.ts @@ -1,14 +1,14 @@ -import { Logger } from "./logger" +import { WarningHandler } from "./warning_handler" export class Guide { - readonly logger: Logger + readonly warningHandler: WarningHandler readonly warnedKeysByObject: WeakMap> = new WeakMap() - constructor(logger: Logger) { - this.logger = logger + constructor(warningHandler: WarningHandler) { + this.warningHandler = warningHandler } - warn(object: any, key: string, message: string) { + warn(object: any, key: string, warning: string, message: string) { let warnedKeys: Set | undefined = this.warnedKeysByObject.get(object) if (!warnedKeys) { @@ -18,7 +18,7 @@ export class Guide { if (!warnedKeys.has(key)) { warnedKeys.add(key) - this.logger.warn(message, object) + this.warningHandler.handleWarning(warning, message, object) } } } diff --git a/src/core/router.ts b/src/core/router.ts index b04d1876..ecdb4aa9 100644 --- a/src/core/router.ts +++ b/src/core/router.ts @@ -5,18 +5,23 @@ import { Module } from "./module" import { Multimap } from "../multimap" import { Scope } from "./scope" import { ScopeObserver, ScopeObserverDelegate } from "./scope_observer" +import { Token } from "../mutation-observers" +import { Guide } from "./guide" +import { Action } from "./action" export class Router implements ScopeObserverDelegate { readonly application: Application - public scopeObserver: ScopeObserver + private scopeObserver: ScopeObserver private scopesByIdentifier: Multimap private modulesByIdentifier: Map + private guide: Guide constructor(application: Application) { this.application = application this.scopeObserver = new ScopeObserver(this.element, this.schema, this) this.scopesByIdentifier = new Multimap() this.modulesByIdentifier = new Map() + this.guide = new Guide(this.warningHandler) } get element() { @@ -27,8 +32,8 @@ export class Router implements ScopeObserverDelegate { return this.application.schema } - get logger() { - return this.application.logger + get warningHandler() { + return this.application } get controllerAttribute(): string { @@ -81,10 +86,25 @@ export class Router implements ScopeObserverDelegate { this.application.handleError(error, message, detail) } + handleWarningsWithDuplicates(element: Element, token: Token, error?: Error) { + if (!error) { + const parsed = Action.forToken(token, this.schema) + + if (!this.modules.map((c) => c.identifier).includes(parsed.identifier)) { + this.guide.warn( + element, + `identifier:${parsed.identifier}`, + `Warning connecting "${token.content}" to undefined controller "${parsed.identifier}"`, + `Warning connecting "${token.content}" to undefined controller "${parsed.identifier}"` + ) + } + } + } + // Scope observer delegate createScopeForElementAndIdentifier(element: Element, identifier: string) { - return new Scope(this.schema, element, identifier, this.logger) + return new Scope(this.schema, element, identifier, this.application) } scopeConnected(scope: Scope) { diff --git a/src/core/scope.ts b/src/core/scope.ts index de302807..6bd54f40 100644 --- a/src/core/scope.ts +++ b/src/core/scope.ts @@ -1,11 +1,11 @@ import { ClassMap } from "./class_map" import { DataMap } from "./data_map" import { Guide } from "./guide" -import { Logger } from "./logger" import { Schema } from "./schema" import { attributeValueContainsToken } from "./selectors" import { TargetSet } from "./target_set" import { OutletSet } from "./outlet_set" +import { WarningHandler } from "./warning_handler" export class Scope { readonly schema: Schema @@ -17,11 +17,11 @@ export class Scope { readonly classes = new ClassMap(this) readonly data = new DataMap(this) - constructor(schema: Schema, element: Element, identifier: string, logger: Logger) { + constructor(schema: Schema, element: Element, identifier: string, warningHandler: WarningHandler) { this.schema = schema this.element = element this.identifier = identifier - this.guide = new Guide(logger) + this.guide = new Guide(warningHandler) this.outlets = new OutletSet(this.documentScope, element) } @@ -55,6 +55,6 @@ export class Scope { private get documentScope(): Scope { return this.isDocumentScope ? this - : new Scope(this.schema, document.documentElement, this.identifier, this.guide.logger) + : new Scope(this.schema, document.documentElement, this.identifier, this.guide.warningHandler) } } diff --git a/src/core/scope_observer.ts b/src/core/scope_observer.ts index 3a10a188..e04de84c 100644 --- a/src/core/scope_observer.ts +++ b/src/core/scope_observer.ts @@ -2,8 +2,6 @@ import { ErrorHandler } from "./error_handler" import { Schema } from "./schema" import { Scope } from "./scope" import { Token, ValueListObserver, ValueListObserverDelegate } from "../mutation-observers" -import { Action } from "./action" -import { Router } from "./router" export interface ScopeObserverDelegate extends ErrorHandler { createScopeForElementAndIdentifier(element: Element, identifier: string): Scope @@ -18,7 +16,6 @@ export class ScopeObserver implements ValueListObserverDelegate { private valueListObserver: ValueListObserver private scopesByIdentifierByElement: WeakMap> private scopeReferenceCounts: WeakMap - private warnedSet: WeakMap constructor(element: Element, schema: Schema, delegate: ScopeObserverDelegate) { this.element = element @@ -27,7 +24,6 @@ export class ScopeObserver implements ValueListObserverDelegate { this.valueListObserver = new ValueListObserver(this.element, this.controllerAttribute, this) this.scopesByIdentifierByElement = new WeakMap() this.scopeReferenceCounts = new WeakMap() - this.warnedSet = new WeakMap() } start() { @@ -65,29 +61,6 @@ export class ScopeObserver implements ValueListObserverDelegate { } } - elementMatchedNoValue(element: Element, token: Token, error?: Error) { - if (!error) { - const parsed = Action.forToken(token, this.schema) - const router = this.delegate as Router - - if (!router.modules.map((c) => c.identifier).includes(parsed.identifier)) { - if (!this.warnedSet.has(element)) { - this.warnedSet.set(element, []) - } - - if (!this.warnedSet.get(element)?.includes(parsed.identifier)) { - router.application.handleWarning( - `Warning connecting identifier: ${parsed.identifier} action ${token.content}`, - `Warning connecting action ${token.content} with identifier: ${parsed.identifier}`, - { element, token } - ) - - this.warnedSet.get(element)?.push(parsed.identifier) - } - } - } - } - elementUnmatchedValue(element: Element, value: Scope) { const referenceCount = this.scopeReferenceCounts.get(value) if (referenceCount) { @@ -106,4 +79,6 @@ export class ScopeObserver implements ValueListObserverDelegate { } return scopesByIdentifier } + + elementMatchedNoValue() {} } diff --git a/src/core/target_set.ts b/src/core/target_set.ts index bd83e691..f8093fb6 100644 --- a/src/core/target_set.ts +++ b/src/core/target_set.ts @@ -80,6 +80,7 @@ export class TargetSet { this.guide.warn( element, `target:${targetName}`, + `Deprecated attributeName`, `Please replace ${attributeName}="${identifier}.${targetName}" with ${revisedAttributeName}="${targetName}". ` + `The ${attributeName} attribute is deprecated and will be removed in a future version of Stimulus.` ) diff --git a/src/core/warning_handler.ts b/src/core/warning_handler.ts new file mode 100644 index 00000000..f2e83483 --- /dev/null +++ b/src/core/warning_handler.ts @@ -0,0 +1,3 @@ +export interface WarningHandler { + handleWarning(warning: string, message: string, detail: any): void +} diff --git a/src/tests/modules/core/legacy_target_tests.ts b/src/tests/modules/core/legacy_target_tests.ts index 331035cc..6ec0bd6b 100644 --- a/src/tests/modules/core/legacy_target_tests.ts +++ b/src/tests/modules/core/legacy_target_tests.ts @@ -20,6 +20,7 @@ export default class LegacyTargetTests extends ControllerTestCase(TargetControll async setupApplication() { super.setupApplication() + this.application.warnings = true this.application.logger = Object.create(console, { warn: { value: () => this.warningCount++, diff --git a/src/tests/modules/core/warning_tests.ts b/src/tests/modules/core/warning_tests.ts index e303d5f5..c4129c0e 100644 --- a/src/tests/modules/core/warning_tests.ts +++ b/src/tests/modules/core/warning_tests.ts @@ -90,12 +90,21 @@ export default class WarningTests extends ApplicationTestCase { this.assert.equal(this.mockLogger.warns.length, 2) this.assert.deepEqual( - this.mockLogger.warns.map((warn) => ({ message: warn.message })), + this.mockLogger.warns + .map((warn) => ({ message: warn.message, warning: warn.warning })) + .sort((warn) => warn.warning), [ - { message: "Warning connecting action non-existing-controller#found with identifier: non-existing-controller" }, { + warning: + 'Warning connecting "non-existing-controller#found" to undefined controller "non-existing-controller"', message: - "Warning connecting action non-existing-controller#not-found with identifier: non-existing-controller", + 'Warning connecting "non-existing-controller#found" to undefined controller "non-existing-controller"', + }, + { + warning: + 'Warning connecting "non-existing-controller#not-found" to undefined controller "non-existing-controller"', + message: + 'Warning connecting "non-existing-controller#not-found" to undefined controller "non-existing-controller"', }, ] )