Fix issues with framework decorators (#79)

This PR fixes issues with several of the decorator implementations in
`framework` and adds unit tests to specify and validate their
functionality.

framework/src/lib/decorators/on.spec.ts

@@ -30,4 +30,18 @@ describe("@on(eventName)", () => {
 
 		expect(counter.count).toBe(3);
 	});
+
+	it("exhibits correct `this` handling when multiple instances are present", () => {
+		const counter1 = document.createElement("test-counter");
+		document.body.appendChild(counter1);
+
+		const counter2 = document.createElement("test-counter");
+		document.body.appendChild(counter2);
+
+		counter1.click();
+		counter2.click();
+
+		expect(counter1.count).toBe(1);
+		expect(counter2.count).toBe(1);
+	});
 });

framework/src/lib/decorators/on.ts

@@ -15,7 +15,7 @@ export function on(eventSelector: string) {
 		? eventSelector.split(":")
 		: [null, eventSelector];
 
-	let target = match (targetName, {
+	const target = match (targetName, {
 		"window": () => window as EventTarget,
 		"document": () => document as EventTarget,
 		_: () => null,
@@ -23,17 +23,35 @@ export function on(eventSelector: string) {
 
 	return <T extends LitElement>(proto: T, propName: keyof T, desc: PropertyDescriptor) => {
 		const handler = getMethodDescriptor(proto, propName)?.value ?? (() => {});
+		const $handler = Symbol(String(propName));
+
+		type Decorated = T & {
+			[$handler]: typeof handler;
+		}
 
 		prependRoutine(proto, "connectedCallback", function (this: T) {
-			target ??= this;
-			target.addEventListener(eventName, handler.bind(this));
+			assertType<Decorated>(this);
+			this[$handler] ??= handler.bind(this);
+
+			if (target) {
+				target.addEventListener(eventName, this[$handler]);
+			} else {
+				this.addEventListener(eventName, this[$handler]);
+			}
 		});
 
 		prependRoutine(proto, "disconnectedCallback", function (this: T) {
-			target ??= this;
-			target.removeEventListener(eventName, handler.bind(this));
+			assertType<Decorated>(this);
+
+			if (target) {
+				target.removeEventListener(eventName, this[$handler]);
+			} else {
+				this.removeEventListener(eventName, this[$handler]);
+			}
 		});
 
 		return desc;
 	}
 }
+
+function assertType<T>(value: unknown): asserts value is T {}

framework/src/lib/di/decorators.ts

@@ -1,4 +1,4 @@
-import { Ctor, Fn, Method } from "@storyteller/utility";
+import { Ctor, Fn } from "@storyteller/utility";
 import { LitElement } from "lit";
 
 import { appendRoutine, prependRoutine } from "../decorators/internal";
@@ -7,6 +7,7 @@ import { Token } from "./types";
 
 const $injector = Symbol("injector");
 const $$injector = Symbol("#injector");
+const $fulfillInjectionRequest = Symbol("fulfillInjectionRequest");
 
 export type ProviderOptions<T, E extends LitElement>
 	= Token<T>
@@ -41,6 +42,7 @@ export function provide<T, E extends LitElement>(options: ProviderOptions<T, E>)
 	type Decorated = E & {
 		[$$injector]?: Map<Token<any>, any> | undefined;
 		readonly [$injector]: Map<Token<any>, any>;
+		[$fulfillInjectionRequest](event: InjectionRequest<T>): void;
 	}
 
 	return (Target: Ctor<E>) => {
@@ -48,9 +50,7 @@ export function provide<T, E extends LitElement>(options: ProviderOptions<T, E>)
 		assertType<Decorated>(proto);
 
 		let descriptor = Object.getOwnPropertyDescriptor(proto, $injector);
-		let needsInit = false;
 		if (!($injector in proto) || !descriptor) {
-			needsInit = true;
 			Object.defineProperty(proto, $injector, {
 				get(this: Decorated) { return this[$$injector] ??= new Map() },
 				configurable: false,
@@ -73,17 +73,16 @@ export function provide<T, E extends LitElement>(options: ProviderOptions<T, E>)
 			this[$injector].set(token, value);
 			this.dispatchEvent(new DependencyProvision(token, value));
 
-			if (needsInit) {
-				const fulfill = fulfillInjectionRequest.bind(this);
-				this.addEventListener(DIEvent.InjectionRequest, fulfill as Method<HTMLElement>);
-			}
+			this[$fulfillInjectionRequest] ??= fulfillInjectionRequest.bind(this);
+			this.addEventListener(DIEvent.InjectionRequest, this[$fulfillInjectionRequest]);
 		});
 
-		appendRoutine(proto, "disconnectedCallback", function (this: Decorated): void {
+		prependRoutine(proto, "disconnectedCallback", function (this: Decorated): void {
 			const token = "token" in options ? options.token : options;
 			const value = this[$injector].get(token);
 
 			this.dispatchEvent(new ProviderRemoval(token, value));
+			this.removeEventListener(DIEvent.InjectionRequest, this[$fulfillInjectionRequest]);
 		});
 	}
 }
@@ -140,43 +139,45 @@ export function inject<T, E extends LitElement>(token: Token<T>) {
  */
 export function queryProviders<T, E extends LitElement>(token: Token<T>) {
 	return <K extends keyof E>(proto: E, key: K) => {
-		function onProvided(this: E, event: DependencyProvision<T>): void {
-			const array = this[key] ?? [];
-			assertType<E[K] & T[]>(array);
+		const $onProvided = Symbol(`onProvided:${String(key)}`);
+		const $onRemoved = Symbol(`onRemoved:${String(key)}`);
 
-			if (event.detail.token === token)
-				this[key] = array.concat(event.detail.value) as E[K];
+		type Decorated = E & {
+			[$onProvided](event: DependencyProvision<T>): void;
+			[$onRemoved](event: ProviderRemoval<T>): void;
 		}
 
-		function onRemoved(this: E, event: ProviderRemoval<T>): void {
+		assertType<Decorated>(proto);
+
+		function onProvided(this: Decorated, event: DependencyProvision<T>): void {
 			const array = this[key] ?? [];
-			assertType<E[K] & T[]>(array);
+			assertType<Decorated[K] & T[]>(array);
 
 			if (event.detail.token === token)
-				this[key] = array.filter(el => el !== event.detail.value) as E[K];
+				this[key] = array.concat(event.detail.value) as Decorated[K];
 		}
 
-		appendRoutine(proto, "connectedCallback", function (this: E): void {
-			this[key] = [] as E[K];
-			this.addEventListener(
-				DIEvent.DependencyProvision,
-				onProvided.bind(this) as Method<HTMLElement>,
-			);
-			this.addEventListener(
-				DIEvent.ProviderRemoval,
-				onRemoved.bind(this) as Method<HTMLElement>,
-			);
+		function onRemoved(this: Decorated, event: ProviderRemoval<T>): void {
+			const array = this[key] ?? [];
+			assertType<Decorated[K] & T[]>(array);
+
+			if (event.detail.token === token)
+				this[key] = array.filter(el => el !== event.detail.value) as Decorated[K];
+		}
+
+		appendRoutine(proto, "connectedCallback", function (this: Decorated): void {
+			this[key] = [] as Decorated[K];
+
+			this[$onProvided] ??= onProvided.bind(this);
+			this[$onRemoved] ??= onRemoved.bind(this);
+
+			this.addEventListener(DIEvent.DependencyProvision, this[$onProvided]);
+			this.addEventListener(DIEvent.ProviderRemoval, this[$onRemoved]);
 		});
 
-		appendRoutine(proto, "disconnectedCallback", function (this: E): void {
-			this.removeEventListener(
-				DIEvent.DependencyProvision,
-				onProvided.bind(this) as Method<HTMLElement>,
-			);
-			this.removeEventListener(
-				DIEvent.ProviderRemoval,
-				onRemoved.bind(this) as Method<HTMLElement>,
-			);
+		appendRoutine(proto, "disconnectedCallback", function (this: Decorated): void {
+			this.removeEventListener(DIEvent.DependencyProvision, this[$onProvided]);
+			this.removeEventListener(DIEvent.ProviderRemoval, this[$onRemoved]);
 		});
 	}
 }

framework/src/lib/di/di.spec.ts

@@ -0,0 +1,250 @@
+import { LitElement, html, render } from "lit";
+import { customElement, property } from "lit/decorators.js";
+import { createRef, ref } from "lit/directives/ref.js";
+
+import { provide, inject, queryProviders } from "./decorators";
+import { UniqueToken } from "./types";
+
+const TEST_TOKEN = UniqueToken.create<HTMLElement>();
+@customElement("test-token-provider")
+@provide(TEST_TOKEN)
+class TestTokenProvider extends LitElement {
+	override render = () => html`<slot></slot>`
+}
+
+@customElement("test-concrete-provider")
+@provide(TestConcreteProvider)
+class TestConcreteProvider extends LitElement {
+	override render = () => html`<slot></slot>`
+}
+
+const TEST_VALUE = UniqueToken.create<number>();
+@customElement("test-value-provider")
+@provide({
+	token: TEST_VALUE,
+	provide() { return this.value; }
+})
+class TestValueProvider extends LitElement {
+	@property({ type: Number })
+	value = 42;
+
+	override render = () => html`<slot></slot>`
+}
+
+@customElement("test-injector")
+class TestInjector extends LitElement {
+	@inject(TEST_TOKEN)
+	tokenProvider: unknown;
+
+	@inject(TestConcreteProvider)
+	concreteProvider?: TestConcreteProvider;
+
+	@inject(TEST_VALUE)
+	valueProvider?: number;
+
+	override render = () => html`<slot></slot>`;
+}
+
+@customElement("test-queryer")
+class TestQueryer extends LitElement {
+	@queryProviders(TEST_TOKEN)
+	tokenProviders: unknown[] = [];
+
+	@queryProviders(TestConcreteProvider)
+	concreteProviders: TestConcreteProvider[] = [];
+
+	@queryProviders(TEST_VALUE)
+	valueProviders: number[] = [];
+
+	override render = () => html`<slot></slot>`
+}
+
+describe("`@provide` and `@inject`", () => {
+	it("works with abstract tokens", () => {
+		const providerRef = createRef<TestTokenProvider>();
+		const injectorRef = createRef<TestInjector>();
+
+		render(html`
+			<test-token-provider ${ref(providerRef)}>
+				<test-injector ${ref(injectorRef)}></test-injector>
+			</test-token-provider>
+		`, document.body);
+
+		expect(providerRef.value).toBeInstanceOf(TestTokenProvider);
+		expect(injectorRef.value).toBeInstanceOf(TestInjector);
+		expect(injectorRef.value!.tokenProvider).toBe(providerRef.value);
+	});
+
+	it("works with concrete tokens", () => {
+		const providerRef = createRef<TestConcreteProvider>();
+		const injectorRef = createRef<TestInjector>();
+
+		render(html`
+			<test-concrete-provider ${ref(providerRef)}>
+				<test-injector ${ref(injectorRef)}></test-injector>
+			</test-concrete-provider>
+		`, document.body);
+
+		expect(providerRef.value).toBeInstanceOf(TestConcreteProvider);
+		expect(injectorRef.value).toBeInstanceOf(TestInjector);
+		expect(injectorRef.value!.concreteProvider).toBe(providerRef.value);
+	});
+
+	it("works with value tokens", () => {
+		const providerRef = createRef<TestValueProvider>();
+		const injectorRef = createRef<TestInjector>();
+
+		render(html`
+			<test-value-provider ${ref(providerRef)}>
+				<test-injector ${ref(injectorRef)}></test-injector>
+			</test-value-provider>
+		`, document.body);
+
+		expect(providerRef.value).toBeInstanceOf(TestValueProvider);
+		expect(injectorRef.value).toBeInstanceOf(TestInjector);
+		expect(injectorRef.value!.valueProvider).toBe(42);
+	});
+
+	it("works with multiple provider/injector instances", () => {
+		const parentProviderRef = createRef<TestValueProvider>();
+		const childProviderRef = createRef<TestValueProvider>();
+		const parentInjectorRef = createRef<TestInjector>();
+		const childInjectorRef = createRef<TestInjector>();
+
+		render(html`
+			<test-value-provider
+				${ref(parentProviderRef)}
+				.value=${420}
+			>
+				<test-injector ${ref(parentInjectorRef)}></test-injector>
+				<test-value-provider
+					${ref(childProviderRef)}
+					.value=${69}
+				>
+					<test-injector ${ref(childInjectorRef)}></test-injector>
+				</test-value-provider>
+			</test-value-provider>
+		`, document.body);
+
+		expect(parentProviderRef.value).toBeInstanceOf(TestValueProvider);
+		expect(childProviderRef.value).toBeInstanceOf(TestValueProvider);
+		expect(parentInjectorRef.value).toBeInstanceOf(TestInjector);
+		expect(childInjectorRef.value).toBeInstanceOf(TestInjector);
+
+		expect(parentInjectorRef.value!.valueProvider).toBe(420);
+		expect(childInjectorRef.value!.valueProvider).toBe(69);
+	});
+
+	it("works with nested providers", () => {
+		const tokenProviderRef = createRef<TestTokenProvider>();
+		const concreteProviderRef = createRef<TestConcreteProvider>();
+		const valueProviderRef = createRef<TestValueProvider>();
+		const injectorRef = createRef<TestInjector>();
+
+		render(html`
+			<test-token-provider ${ref(tokenProviderRef)}>
+				<test-concrete-provider ${ref(concreteProviderRef)}>
+					<test-value-provider ${ref(valueProviderRef)}>
+						<test-injector ${ref(injectorRef)}></test-injector>
+					</test-value-provider>
+				</test-concrete-provider>
+			</test-token-provider>
+		`, document.body);
+
+		expect(tokenProviderRef.value).toBeInstanceOf(TestTokenProvider);
+		expect(concreteProviderRef.value).toBeInstanceOf(TestConcreteProvider);
+		expect(valueProviderRef.value).toBeInstanceOf(TestValueProvider);
+		expect(injectorRef.value).toBeInstanceOf(TestInjector);
+
+		expect(injectorRef.value!.tokenProvider).toBe(tokenProviderRef.value);
+		expect(injectorRef.value!.concreteProvider).toBe(concreteProviderRef.value);
+		expect(injectorRef.value!.valueProvider).toBe(42);
+	});
+});
+
+describe("`@provide` and `@queryProviders`", () => {
+	it("works with abstract tokens", () => {
+		const queryerRef = createRef<TestQueryer>();
+		const providerRef = createRef<TestTokenProvider>();
+
+		render(html`
+			<test-queryer ${ref(queryerRef)}>
+				<test-token-provider ${ref(providerRef)}></test-token-provider>
+			</test-queryer>
+		`, document.body);
+
+		expect(queryerRef.value).toBeInstanceOf(TestQueryer);
+		expect(providerRef.value).toBeInstanceOf(TestTokenProvider);
+
+		expect(queryerRef.value?.tokenProviders).toEqual([providerRef.value]);
+	});
+
+	it("works with concrete tokens", () => {
+		const queryerRef = createRef<TestQueryer>();
+		const providerRef = createRef<TestConcreteProvider>();
+
+		render(html`
+			<test-queryer ${ref(queryerRef)}>
+				<test-concrete-provider ${ref(providerRef)}></test-concrete-provider>
+			</test-queryer>
+		`, document.body);
+
+		expect(queryerRef.value).toBeInstanceOf(TestQueryer);
+		expect(providerRef.value).toBeInstanceOf(TestConcreteProvider);
+
+		expect(queryerRef.value?.concreteProviders).toEqual([providerRef.value]);
+	});
+
+	it("works with value tokens", () => {
+		const queryerRef = createRef<TestQueryer>();
+		const providerRef = createRef<TestValueProvider>();
+
+		render(html`
+			<test-queryer ${ref(queryerRef)}>
+				<test-value-provider ${ref(providerRef)}></test-value-provider>
+			</test-queryer>
+		`, document.body);
+
+		expect(queryerRef.value).toBeInstanceOf(TestQueryer);
+		expect(providerRef.value).toBeInstanceOf(TestValueProvider);
+
+		expect(queryerRef.value?.valueProviders).toEqual([42]);
+	});
+
+	it("works with multiple providers", () => {
+		const queryerRef = createRef<TestQueryer>();
+
+		render(html`
+			<test-queryer ${ref(queryerRef)}>
+				<test-value-provider .value=${42}></test-value-provider>
+				<test-value-provider .value=${420}></test-value-provider>
+				<test-value-provider .value=${69}></test-value-provider>
+			</test-queryer>
+		`, document.body);
+
+		expect(queryerRef.value).toBeInstanceOf(TestQueryer);
+		expect(queryerRef.value!.valueProviders).toEqual([42, 420, 69]);
+	});
+
+	it("dynamically updates", () => {
+		const queryerRef = createRef<TestQueryer>();
+
+		render(html`
+			<test-queryer ${ref(queryerRef)}>
+				<test-value-provider .value=${42}></test-value-provider>
+				<test-value-provider .value=${420}></test-value-provider>
+				<test-value-provider .value=${69}></test-value-provider>
+			</test-queryer>
+		`, document.body);
+
+		expect(queryerRef.value).toBeInstanceOf(TestQueryer);
+		expect(queryerRef.value!.valueProviders).toEqual([42, 420, 69]);
+
+		const middleElement = queryerRef.value!.children.item(1) as TestValueProvider;
+		expect(middleElement).toBeInstanceOf(TestValueProvider);
+		middleElement.disconnectedCallback();
+		middleElement.remove();
+
+		expect(queryerRef.value!.valueProviders).toEqual([42, 69]);
+	});
+});