diff --git a/src/decorator.ts b/src/decorator.ts index 7f9b53e..f154474 100644 --- a/src/decorator.ts +++ b/src/decorator.ts @@ -1,8 +1,12 @@ import { Class } from './types'; +import { flatten, protoChain, unique } from './util'; +import { getMixinsForClass } from './mixin-tracking'; + +type ObjectOfDecorators = { [key: string]: T[] }; export type PropertyAndMethodDecorators = { - property?: { [key: string]: PropertyDecorator[] }, - method?: { [key: string]: MethodDecorator[] }, + property?: ObjectOfDecorators, + method?: ObjectOfDecorators, }; type Decorators = { @@ -11,9 +15,79 @@ type Decorators = { instance?: PropertyAndMethodDecorators, }; -export const decorators: Map = new Map(); +const mergeObjectsOfDecorators = ( + o1: ObjectOfDecorators, + o2: ObjectOfDecorators, +): ObjectOfDecorators => { + const allKeys = unique([...Object.getOwnPropertyNames(o1), ...Object.getOwnPropertyNames(o2)]); + const mergedObject: ObjectOfDecorators = {}; + for (let key of allKeys) + mergedObject[key] = unique([...(o1?.[key] ?? []), ...(o2?.[key] ?? [])]); + return mergedObject; +}; + +const mergePropertyAndMethodDecorators = (d1: PropertyAndMethodDecorators, d2: PropertyAndMethodDecorators): PropertyAndMethodDecorators => ({ + property: mergeObjectsOfDecorators(d1?.property ?? {}, d2?.property ?? {}), + method: mergeObjectsOfDecorators(d1?.method ?? {}, d2?.method ?? {}), +}); + +const mergeDecorators = (d1: Decorators, d2: Decorators): Decorators => ({ + class: unique([...d1?.class ?? [], ...d2?.class ?? []]), + static: mergePropertyAndMethodDecorators(d1?.static ?? {}, d2?.static ?? {}), + instance: mergePropertyAndMethodDecorators(d1?.instance ?? {}, d2?.instance ?? {}), +}); + +const decorators: Map = new Map(); + +const findAllConstituentClasses = (...classes: Class[]): Class[] => { + const allClasses = new Set(); + const frontier = new Set([...classes]); + + while (frontier.size > 0) { + for (let clazz of frontier) { + const protoChainClasses = protoChain(clazz.prototype).map(proto => proto.constructor); + const mixinClasses = getMixinsForClass(clazz) ?? []; + const potentiallyNewClasses = [...protoChainClasses, ...mixinClasses] as Class[]; + const newClasses = potentiallyNewClasses.filter(c => !allClasses.has(c)); + for (let newClass of newClasses) + frontier.add(newClass); + + allClasses.add(clazz); + frontier.delete(clazz); + } + } + + return [...allClasses]; +}; + +export const deepDecoratorSearch = (...classes: Class[]): Decorators => { + const decoratorsForClassChain = + findAllConstituentClasses(...classes) + .map(clazz => decorators.get(clazz as Class)) + .filter(decorators => !!decorators) as Decorators[]; + + if (decoratorsForClassChain.length == 0) + return {}; + + if (decoratorsForClassChain.length == 1) + return decoratorsForClassChain[0]; + + return decoratorsForClassChain.reduce((d1, d2) => mergeDecorators(d1, d2)); +}; + +export const directDecoratorSearch = (...classes: Class[]): Decorators => { + const classDecorators = classes.map(clazz => getDecoratorsForClass(clazz)); + + if (classDecorators.length === 0) + return {}; + + if (classDecorators.length === 1) + return classDecorators[1]; + + return classDecorators.reduce((d1, d2) => mergeDecorators(d1, d2)); +}; -const getDecoratorsForClass = (clazz: Class) => { +export const getDecoratorsForClass = (clazz: Class) => { let decoratorsForClass = decorators.get(clazz); if (!decoratorsForClass) { decoratorsForClass = {}; @@ -75,5 +149,5 @@ export const decorate = ]); }) as T; diff --git a/src/mixin-tracking.ts b/src/mixin-tracking.ts index cd9b785..06294a8 100644 --- a/src/mixin-tracking.ts +++ b/src/mixin-tracking.ts @@ -1,10 +1,12 @@ -/** - * Keeps track of constituent classes for every mixin class created by ts-mixer. - */ -import {protoChain} from './util'; +import { protoChain } from './util'; +import { Class } from './types'; +// Keeps track of constituent classes for every mixin class created by ts-mixer. const mixins = new Map(); +export const getMixinsForClass = (clazz: Class) => + mixins.get(clazz); + export const registerMixins = (mixedClass: any, constituents: Function[]) => mixins.set(mixedClass, constituents); diff --git a/src/mixins.ts b/src/mixins.ts index 987f930..3314d94 100644 --- a/src/mixins.ts +++ b/src/mixins.ts @@ -2,7 +2,7 @@ import { proxyMix } from './proxy'; import { Class, Longest } from './types'; // TODO: need something more than just Longest: also forces all to be subset of longest import { settings } from './settings'; import { copyProps, hardMixProtos, softMixProtos } from './util'; -import { decorators, PropertyAndMethodDecorators } from './decorator'; +import { directDecoratorSearch, deepDecoratorSearch, PropertyAndMethodDecorators } from './decorator'; import { registerMixins } from './mixin-tracking'; function Mixin< @@ -237,19 +237,17 @@ function Mixin(...constructors: Class[]) { ); let DecoratedMixedClass: any = MixedClass; - for (let constructor of constructors) { - const classDecorators = decorators.get(constructor); - if (classDecorators) { - if (classDecorators.class) - for (let decorator of classDecorators.class) - DecoratedMixedClass = decorator(DecoratedMixedClass); - - if (classDecorators.static) - applyPropAndMethodDecorators(classDecorators.static, DecoratedMixedClass); - - if (classDecorators.instance) - applyPropAndMethodDecorators(classDecorators.instance, DecoratedMixedClass.prototype); - } + + if (settings.decoratorInheritance !== 'none') { + const classDecorators = settings.decoratorInheritance === 'deep' + ? deepDecoratorSearch(...constructors) + : directDecoratorSearch(...constructors); + + for (let decorator of classDecorators?.class ?? []) + DecoratedMixedClass = decorator(DecoratedMixedClass); + + applyPropAndMethodDecorators(classDecorators?.static ?? {}, DecoratedMixedClass); + applyPropAndMethodDecorators(classDecorators?.instance ?? {}, DecoratedMixedClass.prototype); } registerMixins(DecoratedMixedClass, constructors); @@ -269,7 +267,7 @@ const applyPropAndMethodDecorators = (propAndMethodDecorators: PropertyAndMethod if (methodDecorators) for (let key in methodDecorators) for (let decorator of methodDecorators[key]) - decorator(target, key, Object.getOwnPropertyDescriptor(target, key)); + decorator(target, key, Object.getOwnPropertyDescriptor(target, key)!); }; /** diff --git a/src/settings.ts b/src/settings.ts index 763f497..2320970 100644 --- a/src/settings.ts +++ b/src/settings.ts @@ -2,10 +2,12 @@ export type Settings = { initFunction: string | null, staticsStrategy: 'copy' | 'proxy', prototypeStrategy: 'copy' | 'proxy', + decoratorInheritance: 'deep' | 'direct' | 'none', }; export const settings: Settings = { initFunction: null, staticsStrategy: 'copy', prototypeStrategy: 'copy', + decoratorInheritance: 'deep', }; diff --git a/src/util.ts b/src/util.ts index 197fda3..265ddff 100644 --- a/src/util.ts +++ b/src/util.ts @@ -26,10 +26,10 @@ export const protoChain = (obj: object, currentChain: object[] = [obj]): object[ * Identifies the nearest ancestor common to all the given objects in their prototype chains. For most unrelated * objects, this function should return Object.prototype. */ -export const nearestCommonProto = (...objs: object[]): Function => { +export const nearestCommonProto = (...objs: object[]): object | undefined => { if (objs.length === 0) return undefined; - let commonProto = undefined; + let commonProto: object | undefined = undefined; const protoChains = objs.map(obj => protoChain(obj)); while (protoChains.every(protoChain => protoChain.length > 0)) { @@ -54,8 +54,8 @@ export const nearestCommonProto = (...objs: object[]): Function => { * flexible as updates to the source prototypes aren't captured by the mixed result. See softMixProtos for why you may * want to use that instead. */ -export const hardMixProtos = (ingredients: any[], constructor: Function, exclude: string[] = []): object => { - const base = nearestCommonProto(...ingredients); +export const hardMixProtos = (ingredients: any[], constructor: Function | null, exclude: string[] = []): object => { + const base = nearestCommonProto(...ingredients) ?? Object.prototype; const mixedProto = Object.create(base); // Keeps track of prototypes we've already visited to avoid copying the same properties multiple times. We init the @@ -88,5 +88,15 @@ export const hardMixProtos = (ingredients: any[], constructor: Function, exclude * changes made to the source prototypes will be reflected in the proxy-prototype, which may be desirable. */ export const softMixProtos = (ingredients: any[], constructor: Function): object => { - return proxyMix([...ingredients, { constructor }], null); + return proxyMix([...ingredients, { constructor }]); }; + +export const unique = (arr: T[]): T[] => + arr.filter((e, i) => arr.indexOf(e) == i); + +export const flatten = (arr: T[][]): T[] => + arr.length === 0 + ? [] + : arr.length === 1 + ? arr[0] + : arr.reduce((a1, a2) => [...a1, ...a2]); diff --git a/test/integration/class-validators.test.ts b/test/gh-issues/15.test.ts similarity index 100% rename from test/integration/class-validators.test.ts rename to test/gh-issues/15.test.ts diff --git a/test/gh-issues/28.test.ts b/test/gh-issues/28.test.ts new file mode 100644 index 0000000..266ea3f --- /dev/null +++ b/test/gh-issues/28.test.ts @@ -0,0 +1,45 @@ +import 'mocha'; +import { expect } from 'chai'; +import { forEachSettings } from '../util'; + +import { IsBoolean, IsIn, validate } from 'class-validator'; +import { decorate, Mixin } from '../../src'; + +describe('gh-issue #28', () => { + forEachSettings(() => { + it('should work', async () => { + class Disposable { + @decorate(IsBoolean()) // instead of @IsBoolean() + isDisposed: boolean = false; + } + + class Statusable { + @decorate(IsIn(['red', 'green'])) // instead of @IsIn(['red', 'green']) + status: string = 'green'; + } + + class Statusable2 { + @decorate(IsIn(['red', 'green'])) // instead of @IsIn(['red', 'green']) + other: string = 'green'; + } + + class ExtendedObject extends Mixin(Disposable, Statusable) { + } + + class ExtendedObject2 extends Mixin(Statusable2, ExtendedObject) { + } + + const extendedObject = new ExtendedObject2(); + extendedObject.status = 'blue'; + extendedObject.other = 'blue'; + // @ts-expect-error + extendedObject.isDisposed = undefined; + + const errors = await validate(extendedObject); + const errorProps = errors.map(error => error.property); + expect(errorProps).to.contain('status'); + expect(errorProps).to.contain('other'); + expect(errorProps).to.contain('isDisposed'); + }); + }); +}); diff --git a/test/integration/decorator-decorator.test.ts b/test/integration/decorator-decorator.test.ts deleted file mode 100644 index a5b498c..0000000 --- a/test/integration/decorator-decorator.test.ts +++ /dev/null @@ -1,35 +0,0 @@ -import 'mocha'; -import { expect } from 'chai'; - -import { IsBoolean, IsIn, validate } from 'class-validator'; -import { decorate, Mixin } from '../../src'; - -describe('@decorator(...)', () => { - xit('should work', async () => { - class Disposable { - @decorate(IsBoolean()) // instead of @IsBoolean() - isDisposed: boolean = false; - } - - class Statusable { - @decorate(IsIn(['red', 'green'])) // instead of @IsIn(['red', 'green']) - status: string = 'green'; - } - - class Statusable2 { - @decorate(IsIn(['red', 'green'])) // instead of @IsIn(['red', 'green']) - other: string = 'green'; - } - - class ExtendedObject extends Mixin(Disposable, Statusable) {} - - class ExtendedObject2 extends Mixin(Statusable2, ExtendedObject) {} - - const extendedObject = new ExtendedObject2(); - extendedObject.status = 'blue'; - extendedObject.other = 'blue'; - extendedObject.isDisposed = undefined; - - console.log(await validate(extendedObject)); - }); -}); diff --git a/test/integration/init-function.test.ts b/test/integration/init-function.test.ts index b8b5089..6dcf643 100644 --- a/test/integration/init-function.test.ts +++ b/test/integration/init-function.test.ts @@ -87,21 +87,21 @@ describe('Using an init function', () => { settings.initFunction = 'init'; class ClassA { - public initContextA = null; + public initContextA: any = null; protected init() { this.initContextA = this; } } class ClassB { - public initContextB = null; + public initContextB: any = null; protected init() { this.initContextB = this; } } class ClassC { - public initContextC = null; + public initContextC: any = null; protected init() { this.initContextC = this; } diff --git a/test/tsconfig.json b/test/tsconfig.json new file mode 100644 index 0000000..923cf8c --- /dev/null +++ b/test/tsconfig.json @@ -0,0 +1,7 @@ +{ + "extends": "../tsconfig.json", + "compilerOptions": { + "noEmit": true + }, + "include": ["**/*.ts"] +} diff --git a/test/unit/decorator.test.ts b/test/unit/decorator.test.ts index dd65cff..fa2d4b6 100644 --- a/test/unit/decorator.test.ts +++ b/test/unit/decorator.test.ts @@ -2,7 +2,8 @@ import 'mocha'; import { expect } from 'chai'; import { fake, assert } from 'sinon'; -import { decorate, decorators } from '../../src/decorator'; +import { Mixin } from '../../src'; +import { decorate, getDecoratorsForClass, deepDecoratorSearch } from '../../src/decorator'; describe('decorate', () => { const decorator1 = fake(); @@ -37,36 +38,121 @@ describe('decorate', () => { } it('should work for instance methods', () => { - expect(decorators.get(Foo).instance.method.bar).to.deep.equal([decorator2]); + expect(getDecoratorsForClass(Foo)?.instance?.method?.bar).to.deep.equal([decorator2]); assert.calledWithExactly(decorator2, Foo.prototype, 'bar', Object.getOwnPropertyDescriptor(Foo.prototype, 'bar')); }); it('should work for instance properties', () => { - expect(decorators.get(Bar).instance.property.bar).to.deep.equal([decorator4]); + expect(getDecoratorsForClass(Bar)?.instance?.property?.bar).to.deep.equal([decorator4]); assert.calledWith(decorator4, Bar.prototype, 'bar'); }); it('should work for static methods', () => { - expect(decorators.get(Bar).static.method.foo).to.deep.equal([decorator3]); + expect(getDecoratorsForClass(Bar)?.static?.method?.foo).to.deep.equal([decorator3]); assert.calledWithExactly(decorator3, Bar, 'foo', Object.getOwnPropertyDescriptor(Bar, 'foo')); }); it('should work for static properties', () => { - expect(decorators.get(Foo).static.property.FOO).to.deep.equal([decorator1]); + expect(getDecoratorsForClass(Foo)?.static?.property?.FOO).to.deep.equal([decorator1]); assert.calledWith(decorator1, Foo, 'FOO'); }); it('should work for multiple decorators on one field and retain application order', () => { - expect(decorators.get(Bar).instance.property.baz).to.deep.equal([decorator6, decorator5]); + expect(getDecoratorsForClass(Bar)?.instance?.property?.baz).to.deep.equal([decorator6, decorator5]); assert.calledWith(decorator6, Bar.prototype, 'baz', Object.getOwnPropertyDescriptor(Bar.prototype, 'baz')); assert.calledWith(decorator5, Bar.prototype, 'baz', Object.getOwnPropertyDescriptor(Bar.prototype, 'baz')); expect(decorator6.calledBefore(decorator5)).to.be.true; }); it('should work for class decorators', () => { - expect(decorators.get(Bar).class).to.deep.equal([decorator8, decorator7]); + expect(getDecoratorsForClass(Bar)?.class).to.deep.equal([decorator8, decorator7]); assert.calledWithExactly(decorator8, Bar); assert.calledWithExactly(decorator7, Bar); expect(decorator8.calledBefore(decorator7)).to.be.true; }); }); + +describe('getAllDecoratorsForHierarchy', () => { + const decorator1 = fake(); + const decorator2 = fake(); + const decorator3 = fake(); + const decorator4 = fake(); + const decorator5 = fake(); + const decorator6 = fake(); + const decorator7 = fake(); + const decorator8 = fake(); + + class Foo { @decorate(decorator1) method1(){} } + class Bar { @decorate(decorator2) method2(){} } + class SubFoo extends Foo{ @decorate(decorator3) method3(){} } + class SubBar extends Bar { @decorate(decorator4) method4(){} } + class FooBar extends Mixin(Foo, Bar) { @decorate(decorator5) method5(){} } + class SubFooSubBar extends Mixin(SubFoo, SubBar) { @decorate(decorator6) method6(){} } + class FooBarSubFooSubBar extends Mixin(FooBar, SubFooSubBar) { @decorate(decorator7) method7(){} } + class SubFooBarSubFooSubBar extends FooBarSubFooSubBar { @decorate(decorator8) method8(){} } + + it('should pick up first-level decorators', () => { + expect(deepDecoratorSearch(Foo)?.instance?.method).to.deep.equal({ + method1: [decorator1], + }); + + expect(deepDecoratorSearch(Bar)?.instance?.method).to.deep.equal({ + method2: [decorator2], + }); + }); + + it('should pick up single-inherited decorators', () => { + expect(deepDecoratorSearch(SubFoo)?.instance?.method).to.deep.equal({ + method1: [decorator1], + method3: [decorator3], + }); + + expect(deepDecoratorSearch(SubBar)?.instance?.method).to.deep.equal({ + method2: [decorator2], + method4: [decorator4], + }); + }); + + it('should pick up multi-inherited decorators', () => { + expect(deepDecoratorSearch(FooBar)?.instance?.method).to.deep.equal({ + method1: [decorator1], + method2: [decorator2], + method5: [decorator5], + }); + }); + + it('should pick up a mix of inherited decorators', () => { + expect(deepDecoratorSearch(SubFooSubBar)?.instance?.method).to.deep.equal({ + method1: [decorator1], + method2: [decorator2], + method3: [decorator3], + method4: [decorator4], + method6: [decorator6], + }); + }); + + it('should pick up a deep mix of inherited decorators', () => { + expect(deepDecoratorSearch(FooBarSubFooSubBar)?.instance?.method).to.deep.equal({ + method1: [decorator1], + method2: [decorator2], + method3: [decorator3], + method4: [decorator4], + method5: [decorator5], + method6: [decorator6], + method7: [decorator7], + }); + }); + + it('should pick a really deep mix of inherited decorators', () => { + expect(deepDecoratorSearch(SubFooBarSubFooSubBar)?.instance?.method).to.deep.equal({ + method1: [decorator1], + method2: [decorator2], + method3: [decorator3], + method4: [decorator4], + method5: [decorator5], + method6: [decorator6], + method7: [decorator7], + method8: [decorator8], + }); + }); +}); diff --git a/test/unit/util.test.ts b/test/unit/util.test.ts index 7620b4b..ee53615 100644 --- a/test/unit/util.test.ts +++ b/test/unit/util.test.ts @@ -1,7 +1,7 @@ import 'mocha'; import { expect } from 'chai'; -import { copyProps, protoChain, nearestCommonProto } from '../../src/util'; +import { copyProps, protoChain, nearestCommonProto, unique, flatten } from '../../src/util'; describe('copyProps', () => { // TODO: ... @@ -36,7 +36,7 @@ describe('nearestCommonProto', () => { it('should return undefined when no objects are passed', () => { expect(nearestCommonProto()).to.equal(undefined); }); - + it('should return undefined when objects share no common lineage (not even Object)', () => { const a = Object.create(null); const b = {}; @@ -100,3 +100,27 @@ describe('hardMixProtos', () => { describe('sofMixProtos', () => { // TODO: ... }); + +describe('unique', () => { + it('should leave already-dupe-free arrays alone', () => { + expect(unique([1, 2, 3])).to.deep.equal([1, 2, 3]); + }); + + it('should filter duplicates', () => { + expect(unique(['a', 'b', 'c', 'c', 'd'])).to.deep.equal(['a', 'b', 'c', 'd']); + }); +}); + +describe('flatten', () => { + it('should passthrough an empty array', () => { + expect(flatten([])).to.deep.equal([]); + }); + + it('should work with single nested array', () => { + expect(flatten([['a', 'b', 'c']])).to.deep.equal(['a', 'b', 'c']); + }); + + it('should work with multiple nested arrays', () => { + expect(flatten([[1, 2, 3], [4, 5, 6]])).to.deep.equal([1, 2, 3, 4, 5, 6]); + }); +}); diff --git a/tsconfig.json b/tsconfig.json index 18d7b2c..6fce8ee 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -9,7 +9,8 @@ "outDir": "dist", "declaration": true, "experimentalDecorators": true, - "emitDecoratorMetadata": true + "emitDecoratorMetadata": true, + "strictNullChecks": true }, "include": [ "src/**/*.ts"