From 30979fec961f3abeb406a78ec966d70ec5cfd627 Mon Sep 17 00:00:00 2001 From: Danny McGee Date: Thu, 7 Mar 2024 21:44:55 -0500 Subject: [PATCH] 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 | 14 ++ framework/src/lib/decorators/on.ts | 28 ++- framework/src/lib/di/decorators.ts | 71 +++---- framework/src/lib/di/di.spec.ts | 250 ++++++++++++++++++++++++ 4 files changed, 323 insertions(+), 40 deletions(-) create mode 100644 framework/src/lib/di/di.spec.ts diff --git a/framework/src/lib/decorators/on.spec.ts b/framework/src/lib/decorators/on.spec.ts index 430456a..94866fa 100644 --- a/framework/src/lib/decorators/on.spec.ts +++ b/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); + }); }); diff --git a/framework/src/lib/decorators/on.ts b/framework/src/lib/decorators/on.ts index dbd4f5b..c3a66a1 100644 --- a/framework/src/lib/decorators/on.ts +++ b/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 (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(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(this); + + if (target) { + target.removeEventListener(eventName, this[$handler]); + } else { + this.removeEventListener(eventName, this[$handler]); + } }); return desc; } } + +function assertType(value: unknown): asserts value is T {} diff --git a/framework/src/lib/di/decorators.ts b/framework/src/lib/di/decorators.ts index 6674c96..c49de19 100644 --- a/framework/src/lib/di/decorators.ts +++ b/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 = Token @@ -41,6 +42,7 @@ export function provide(options: ProviderOptions) type Decorated = E & { [$$injector]?: Map, any> | undefined; readonly [$injector]: Map, any>; + [$fulfillInjectionRequest](event: InjectionRequest): void; } return (Target: Ctor) => { @@ -48,9 +50,7 @@ export function provide(options: ProviderOptions) assertType(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(options: ProviderOptions) 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); - } + 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(token: Token) { */ export function queryProviders(token: Token) { return (proto: E, key: K) => { - function onProvided(this: E, event: DependencyProvision): void { - const array = this[key] ?? []; - assertType(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): void; + [$onRemoved](event: ProviderRemoval): void; } - function onRemoved(this: E, event: ProviderRemoval): void { + assertType(proto); + + function onProvided(this: Decorated, event: DependencyProvision): void { const array = this[key] ?? []; - assertType(array); + assertType(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, - ); - this.addEventListener( - DIEvent.ProviderRemoval, - onRemoved.bind(this) as Method, - ); + function onRemoved(this: Decorated, event: ProviderRemoval): void { + const array = this[key] ?? []; + assertType(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, - ); - this.removeEventListener( - DIEvent.ProviderRemoval, - onRemoved.bind(this) as Method, - ); + appendRoutine(proto, "disconnectedCallback", function (this: Decorated): void { + this.removeEventListener(DIEvent.DependencyProvision, this[$onProvided]); + this.removeEventListener(DIEvent.ProviderRemoval, this[$onRemoved]); }); } } diff --git a/framework/src/lib/di/di.spec.ts b/framework/src/lib/di/di.spec.ts new file mode 100644 index 0000000..400807a --- /dev/null +++ b/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(); +@customElement("test-token-provider") +@provide(TEST_TOKEN) +class TestTokenProvider extends LitElement { + override render = () => html`` +} + +@customElement("test-concrete-provider") +@provide(TestConcreteProvider) +class TestConcreteProvider extends LitElement { + override render = () => html`` +} + +const TEST_VALUE = UniqueToken.create(); +@customElement("test-value-provider") +@provide({ + token: TEST_VALUE, + provide() { return this.value; } +}) +class TestValueProvider extends LitElement { + @property({ type: Number }) + value = 42; + + override render = () => html`` +} + +@customElement("test-injector") +class TestInjector extends LitElement { + @inject(TEST_TOKEN) + tokenProvider: unknown; + + @inject(TestConcreteProvider) + concreteProvider?: TestConcreteProvider; + + @inject(TEST_VALUE) + valueProvider?: number; + + override render = () => html``; +} + +@customElement("test-queryer") +class TestQueryer extends LitElement { + @queryProviders(TEST_TOKEN) + tokenProviders: unknown[] = []; + + @queryProviders(TestConcreteProvider) + concreteProviders: TestConcreteProvider[] = []; + + @queryProviders(TEST_VALUE) + valueProviders: number[] = []; + + override render = () => html`` +} + +describe("`@provide` and `@inject`", () => { + it("works with abstract tokens", () => { + const providerRef = createRef(); + const injectorRef = createRef(); + + render(html` + + + + `, 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(); + const injectorRef = createRef(); + + render(html` + + + + `, 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(); + const injectorRef = createRef(); + + render(html` + + + + `, 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(); + const childProviderRef = createRef(); + const parentInjectorRef = createRef(); + const childInjectorRef = createRef(); + + render(html` + + + + + + + `, 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(); + const concreteProviderRef = createRef(); + const valueProviderRef = createRef(); + const injectorRef = createRef(); + + render(html` + + + + + + + + `, 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(); + const providerRef = createRef(); + + render(html` + + + + `, 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(); + const providerRef = createRef(); + + render(html` + + + + `, 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(); + const providerRef = createRef(); + + render(html` + + + + `, 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(); + + render(html` + + + + + + `, document.body); + + expect(queryerRef.value).toBeInstanceOf(TestQueryer); + expect(queryerRef.value!.valueProviders).toEqual([42, 420, 69]); + }); + + it("dynamically updates", () => { + const queryerRef = createRef(); + + render(html` + + + + + + `, 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]); + }); +});