From b02c05209472df86792c35dbaf6084fddbc4f274 Mon Sep 17 00:00:00 2001 From: Joe Pea Date: Sun, 18 Apr 2021 20:15:46 -0700 Subject: [PATCH] simplify things: remove lowclass/Mixin and use #private fields such that we can output declaration files lowclass/Mixin and TypeScript "private" or "protected" access modifiers cause declaration output not to be possible, thus forcing us to point package.json types at src/index.ts instead of dist/index.d.ts, which can cause type errors in downstream consumers if they use different tsconfig.json settings when they type-check their applications (coupled with the fact that TypeScript will type-check library code if the library code types field does not point to declaration files, *even if* the user set `skipLibCheck` to `true`). Outputting declaration files and pointing `types` to them is the way to go for publishing libraries, but certain features of TypeScript do not work when declaration output is enabled, which is why we had to refactor. https://github.com/microsoft/TypeScript/issues/35822 Now that lume/cli is updated, npm test fails. Needs a fix. --- .prettierrc.js => .prettierrc.cjs | 1 - package.json | 6 ++-- src/Eventful.test.ts | 38 +++++++++++++++++++--- src/Eventful.ts | 53 +++++++++++++++++++------------ tsconfig.json | 4 --- 5 files changed, 70 insertions(+), 32 deletions(-) rename .prettierrc.js => .prettierrc.cjs (91%) diff --git a/.prettierrc.js b/.prettierrc.cjs similarity index 91% rename from .prettierrc.js rename to .prettierrc.cjs index dffccfe..9689f88 100644 --- a/.prettierrc.js +++ b/.prettierrc.cjs @@ -1,5 +1,4 @@ module.exports = { - tabWidth: 4, useTabs: true, semi: false, singleQuote: true, diff --git a/package.json b/package.json index e2911c3..fd75061 100644 --- a/package.json +++ b/package.json @@ -7,7 +7,7 @@ "homepage": "http://github.com/lume/eventful#readme", "type": "module", "main": "dist/index.js", - "types": "src/index.ts", + "types": "dist/index.d.ts", "exports COMMENT:": "This removes 'dist' from import statements, as well as replaces the 'main' field. See https://github.com/nodejs/node/issues/14970#issuecomment-571887546", "exports": { ".": "./dist/index.js", @@ -35,8 +35,8 @@ "lowclass": "^4.9.0" }, "devDependencies": { - "@lume/cli": "^0.3.0", - "prettier": "^1.19.1" + "@lume/cli": "^0.5.0", + "prettier": "^2.0.0" }, "repository": { "type": "git", diff --git a/src/Eventful.test.ts b/src/Eventful.test.ts index 35d68d2..a925345 100644 --- a/src/Eventful.test.ts +++ b/src/Eventful.test.ts @@ -1,4 +1,4 @@ -import Eventful from './Eventful.js' +import {Eventful} from './Eventful.js' let eventCount = 0 let eventCount2 = 0 @@ -6,7 +6,8 @@ let eventCount2 = 0 describe('Eventful', () => { describe('provides an event pattern', () => { it('triggers an event handler based on event names', () => { - const o = new Eventful() + const MyClass = Eventful() + const o = new MyClass() const eventHandler = () => { eventCount += 1 @@ -92,7 +93,8 @@ describe('Eventful', () => { }) it('passes event payloads to event handlers', () => { - const o = new Eventful() + class MyClass extends Eventful() {} + const o = new MyClass() let thePayload: string | number | undefined let thePayload2: number | undefined @@ -128,7 +130,8 @@ describe('Eventful', () => { }) it('allows callbacks to be paired with contexts with which to be called', () => { - const o = new Eventful() + class MyClass extends Eventful() {} + const o = new MyClass() let obj = {n: 0} const o1 = {} @@ -175,5 +178,32 @@ describe('Eventful', () => { expect(obj.n).toBe(4) }) + + it('allows us to check objects are instances of the mixin class or composed classes', () => { + const MyClass = Eventful() + class OtherClass extends Eventful() {} + class AnotherClass extends Eventful(Array) {} + + let o = new MyClass + expect(o).toBeInstanceOf(Eventful) + expect(o).toBeInstanceOf(MyClass) + expect(o).not.toBeInstanceOf(OtherClass) + expect(o).not.toBeInstanceOf(AnotherClass) + expect(o).not.toBeInstanceOf(Array) + + o = new OtherClass + expect(o).toBeInstanceOf(Eventful) + expect(o).not.toBeInstanceOf(MyClass) + expect(o).toBeInstanceOf(OtherClass) + expect(o).not.toBeInstanceOf(AnotherClass) + expect(o).not.toBeInstanceOf(Array) + + o = new AnotherClass + expect(o).toBeInstanceOf(Eventful) + expect(o).not.toBeInstanceOf(MyClass) + expect(o).not.toBeInstanceOf(OtherClass) + expect(o).toBeInstanceOf(AnotherClass) + expect(o).toBeInstanceOf(Array) + }) }) }) diff --git a/src/Eventful.ts b/src/Eventful.ts index a2788f6..e4cde04 100644 --- a/src/Eventful.ts +++ b/src/Eventful.ts @@ -1,16 +1,17 @@ -import {Mixin, MixinResult, Constructor} from 'lowclass' +import type {Constructor} from 'lowclass' -// TODO @trusktr, make strongly typed event args. Combine with stuff in Events.ts (or similar). +// TODO, make strongly typed event args. Combine with stuff in Events.ts (or similar). -// TODO @trusktr, Make sure emit will not attempt to call event handlers removed +// TODO, Make sure emit will not attempt to call event handlers removed // during emit (in place modification of listeners array during emit iteration // will try to access undefined after the end of the array). Possibly use // for..of with a Set instead, otherwise modify the iteration index manually. -// TODO @trusktr, an option to defer events, and batch them (so that 3 of the +// TODO, an option to defer events, and batch them (so that 3 of the // same event and payload triggers only one event instead of three) /** + * @mixin * @class Eventful - An instance of Eventful emits events that code can * subscribe to with callbacks. Events may optionally pass a payload to the * callbacks. @@ -41,12 +42,10 @@ import {Mixin, MixinResult, Constructor} from 'lowclass' * rectangle.off("resize", onResize) * ``` */ -export const Eventful = Mixin(EventfulMixin) -export interface Eventful extends InstanceType {} -export default Eventful +export function Eventful(Base: T = Object as any) { + class Eventful extends Base { + [isEventful] = isEventful -export function EventfulMixin(Base: T) { - class Eventful extends Constructor(Base) { /** * @method on - Register a `callback` to be executed any * time an event with name `eventName` is triggered by an instance of @@ -60,13 +59,13 @@ export function EventfulMixin(Base: T) { * @param {any} context - An optional context to call the callback with. Passing no context is the same as subscribing `callback` for a `context` of `undefined`. */ on(eventName: string, callback: Function, context?: any) { - let eventMap = this.__eventMap + let eventMap = this.#eventMap // @prod-prune if (typeof callback !== 'function') throw new Error('Expected a function in callback argument of Eventful#on.') - if (!eventMap) eventMap = this.__eventMap = new Map() + if (!eventMap) eventMap = this.#eventMap = new Map() let callbacks = eventMap.get(eventName) @@ -85,7 +84,7 @@ export function EventfulMixin(Base: T) { * @param {any} context - A context associated with `callback`. Not passing a `context` arg is equivalent to unsubscribing the `callback` for `context` of `undefined`. */ off(eventName: string, callback?: Function, context?: any) { - const eventMap = this.__eventMap + const eventMap = this.#eventMap if (!eventMap) return @@ -101,7 +100,7 @@ export function EventfulMixin(Base: T) { if (callbacks.length === 0) eventMap.delete(eventName) - if (eventMap.size === 0) this.__eventMap = null + if (eventMap.size === 0) this.#eventMap = null } /** @@ -120,7 +119,7 @@ export function EventfulMixin(Base: T) { * @param {data} any - The data that is passed to each callback subscribed to the event. */ emit(eventName: string, data?: any) { - const eventMap = this.__eventMap + const eventMap = this.#eventMap if (!eventMap) return @@ -140,12 +139,24 @@ export function EventfulMixin(Base: T) { } } - private __eventMap: Map> | null = null + #eventMap: Map> | null = null } - return Eventful as MixinResult + Eventful.prototype[isEventful] = isEventful + + return Eventful } +const isEventful = Symbol('isEventful') + +Object.defineProperty(Eventful, Symbol.hasInstance, { + value(obj: any): boolean { + if (!obj || typeof obj !== 'object') return false + if (!(isEventful in obj)) return false + return true + }, +}) + /** * @decorator * @function emits - This is a decorator that when used on a property in a @@ -171,7 +182,7 @@ function _emits(prototype: any, propName: string, descriptor: PropertyDescriptor if (!(prototype instanceof Eventful)) throw new TypeError('The @emits decorator in only for use on properties of classes that extend from Eventful.') - const vName = 'emits_' + propName + const vName = Symbol('@emits: ' + propName) // property decorators are not passed a descriptor (unlike decorators on accessors or methods) let calledAsPropertyDecorator = false @@ -226,7 +237,7 @@ function _emits(prototype: any, propName: string, descriptor: PropertyDescriptor } let initialValueIsSet = false - function emitEvent(this: Eventful) { + function emitEvent(this: EventfulInstance) { this.emit(eventName, propName) } @@ -257,7 +268,7 @@ function _emits(prototype: any, propName: string, descriptor: PropertyDescriptor originalSet!.call(this, newValue) // TODO should we defer the event, or not? Perhaps provide an option, and defer by default. - Promise.resolve().then(emitEvent.bind(this as Eventful)) + Promise.resolve().then(emitEvent.bind(this as EventfulInstance)) // emitEvent.call(this as Eventful) }, } @@ -265,7 +276,7 @@ function _emits(prototype: any, propName: string, descriptor: PropertyDescriptor set(newValue: any) { if (!initialValueIsSet) initialValueIsSet = true ;(this as any)[vName] = newValue - Promise.resolve().then(emitEvent.bind(this as Eventful)) + Promise.resolve().then(emitEvent.bind(this as EventfulInstance)) }, }), } @@ -279,3 +290,5 @@ function _emits(prototype: any, propName: string, descriptor: PropertyDescriptor // Weird, huh? // This will all change with updates to the ES decorators proposal, https://github.com/tc39/proposal-decorators } + +type EventfulInstance = InstanceType> diff --git a/tsconfig.json b/tsconfig.json index 6f3dce7..27c8cc8 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,7 +1,3 @@ { "extends": "./node_modules/@lume/cli/config/ts.config.json", - "compilerOptions": { - "declaration": false, // TODO, set back to true after removing class-factory mixins - "declarationMap": false - } }