Skip to content

Commit

Permalink
refactor(Mixin): trace prototype chain, allow construction
Browse files Browse the repository at this point in the history
  • Loading branch information
ckohen committed Sep 3, 2024
1 parent 8355229 commit 1846ab6
Show file tree
Hide file tree
Showing 11 changed files with 243 additions and 58 deletions.
33 changes: 31 additions & 2 deletions packages/structures/__tests__/Mixin.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { describe, test, expect } from 'vitest';
import type { APIData } from './mixinClasses.js';
import { Mixed } from './mixinClasses.js';
import { Mixed, MixedWithExtended } from './mixinClasses.js';

describe('Mixin function', () => {
const data: APIData = {
Expand All @@ -20,6 +20,35 @@ describe('Mixin function', () => {
expect(instance.getId()).toBe(data.id);
expect(instance.getProperty1()).toBe(data.property1);
expect(instance.getProperty2()).toBe(data.property2);
expect(instance.getProperties()).toEqual({ property1: data.property1, property2: data.property2 });
expect(instance.getProperties()).toEqual({
property1: data.property1,
property2: data.property2,
});
});

test('Mixed with extended class has all getters', () => {
const instance = new MixedWithExtended(data);
expect(instance.id).toBe(data.id);
expect(instance.property1).toBe(data.property1);
expect(instance.property2).toBe(data.property2);
expect(instance.isExtended).toBe(true);
});

test('Mixed with extended class has all methods', () => {
const instance = new MixedWithExtended(data);
expect(instance.getId()).toBe(data.id);
expect(instance.getProperty1()).toBe(data.property1);
expect(instance.getProperty2()).toBe(data.property2);
expect(instance.getProperties()).toEqual({
property1: data.property1,
property2: data.property2,
});
});

test('Mixed class calls construct methods on construct', () => {
const instance1 = new Mixed(data);
const instance2 = new MixedWithExtended(data);
expect(instance1.constructCalled).toBe(true);
expect(instance2.constructCalled).toBe(true);
});
});
12 changes: 9 additions & 3 deletions packages/structures/__tests__/Structure.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ describe('Base Structure', () => {
beforeEach(() => {
// @ts-expect-error Structure constructor is protected
struct = new Structure(data);
// @ts-expect-error Structure.DataTemplate is protected
Structure.DataTemplate = {};
});

test('Data reference is not identical (clone via Object.assign)', () => {
Expand All @@ -16,8 +18,10 @@ describe('Base Structure', () => {
});

test('Remove properties via template (constructor)', () => {
// @ts-expect-error Structure.DataTemplate is protected
Structure.DataTemplate = { set removed(_) {} };
// @ts-expect-error Structure constructor is protected
const templatedStruct: Structure<typeof data> = new Structure(data, { template: { set removed(_) {} } });
const templatedStruct: Structure<typeof data> = new Structure(data);
expect(templatedStruct[kData].removed).toBe(undefined);
// Setters still exist and pass "in" test unfortunately
expect('removed' in templatedStruct[kData]).toBe(true);
Expand All @@ -37,10 +41,12 @@ describe('Base Structure', () => {
});

test('Remove properties via template (_patch)', () => {
// @ts-expect-error Structure.DataTemplate is protected
Structure.DataTemplate = { set removed(_) {} };
// @ts-expect-error Structure constructor is protected
const templatedStruct: Structure<typeof data> = new Structure(data, { template: { set removed(_) {} } });
const templatedStruct: Structure<typeof data> = new Structure(data);
// @ts-expect-error Structure#patch is protected
templatedStruct._patch({ removed: false }, { template: { set removed(_) {} } });
templatedStruct._patch({ removed: false });
expect(templatedStruct[kData].removed).toBe(undefined);
// Setters still exist and pass "in" test unfortunately
expect('removed' in templatedStruct[kData]).toBe(true);
Expand Down
29 changes: 27 additions & 2 deletions packages/structures/__tests__/mixinClasses.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { MixinTypes } from '../src/Mixin.js';
import { Mixin } from '../src/Mixin.js';
import { Structure } from '../src/Structure.js';
import { kData } from '../src/utils/symbols.js';
import { kData, kMixinConstruct } from '../src/utils/symbols.js';

export interface APIData {
id: string;
Expand Down Expand Up @@ -34,8 +34,14 @@ export class MixinProperty1 {
}
}

export interface MixinProperty2<Omitted extends keyof APIData | '' = ''> extends Base<Omitted> {}
export interface MixinProperty2<Omitted extends keyof APIData | '' = ''> extends Base<Omitted> {
constructCalled: boolean;
}
export class MixinProperty2 {
public [kMixinConstruct]() {
this.constructCalled = true;
}

public get property2() {
return this[kData].property2;
}
Expand All @@ -44,6 +50,12 @@ export class MixinProperty2 {
return this.property2;
}
}
export class ExtendedMixinProperty2 extends MixinProperty2 {
// eslint-disable-next-line @typescript-eslint/class-literal-property-style
public get isExtended() {
return true;
}
}

export interface Mixed extends MixinTypes<Base, [MixinProperty1, MixinProperty2]> {}
export class Mixed extends Base {
Expand All @@ -53,3 +65,16 @@ export class Mixed extends Base {
}

Mixin(Mixed, [MixinProperty1, MixinProperty2]);

export interface MixedWithExtended extends MixinTypes<Base, [MixinProperty1, ExtendedMixinProperty2]> {}
export class MixedWithExtended extends Base {
public getProperties() {
return {
property1: this.property1,
property2: this.property2,
};
}
}

// Intentiontally don't directly mix Property 2
Mixin(MixedWithExtended, [MixinProperty1, ExtendedMixinProperty2]);
17 changes: 12 additions & 5 deletions packages/structures/__tests__/types/Mixin.test-d.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,20 @@
import { expectNotType, expectType } from 'tsd';
import { expectTypeOf } from 'vitest';
import type { MixinTypes } from '../../src/Mixin.js';
import type { kMixinConstruct } from '../../src/utils/symbols.js';
import type { MixinProperty1, Base, MixinProperty2 } from '../mixinClasses.js';

declare const extendsNoOmit: Omit<MixinProperty1, keyof Base>;
declare const extendsOmitProperty1: Omit<MixinProperty1<'property1'>, keyof Base>;
declare const extendsBothNoOmit: Omit<MixinProperty1 & MixinProperty2, keyof Base>;
declare const extendsBothOmitProperty1: Omit<MixinProperty1<'property1'> & MixinProperty2<'property1'>, keyof Base>;
declare const extendsBothOmitBoth: Omit<MixinProperty1<'property1'> & MixinProperty2<'property2'>, keyof Base>;
declare const extendsNoOmit: Omit<MixinProperty1, keyof Base | typeof kMixinConstruct>;
declare const extendsOmitProperty1: Omit<MixinProperty1<'property1'>, keyof Base | typeof kMixinConstruct>;
declare const extendsBothNoOmit: Omit<MixinProperty1 & MixinProperty2, keyof Base | typeof kMixinConstruct>;
declare const extendsBothOmitProperty1: Omit<
MixinProperty1<'property1'> & MixinProperty2<'property1'>,
keyof Base | typeof kMixinConstruct
>;
declare const extendsBothOmitBoth: Omit<
MixinProperty1<'property1'> & MixinProperty2<'property2'>,
keyof Base | typeof kMixinConstruct
>;

expectType<MixinTypes<Base, [MixinProperty1]>>(extendsNoOmit);
expectType<MixinTypes<Base<'property1'>, [MixinProperty1<'property1'>]>>(extendsOmitProperty1);
Expand Down
112 changes: 96 additions & 16 deletions packages/structures/src/Mixin.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { Structure } from './Structure.js';
import { DataTemplatePropetyName, OptimizeDataPropertyName, type Structure } from './Structure.js';
import type { kData } from './utils/symbols.js';
import { kMixinConstruct } from './utils/symbols.js';
import type { CollapseUnion, MergePrototypes } from './utils/types.js';

export type Mixinable<ClassType> = new (...args: unknown[]) => ClassType;
Expand Down Expand Up @@ -37,26 +38,105 @@ export function Mixin<DestinationClass extends typeof Structure<unknown>>(
destination: DestinationClass,
mixins: Mixinable<MixinBase<DestinationClass['prototype']>>[],
) {
for (const mixin of mixins) {
const originalDescriptors = Object.getOwnPropertyDescriptors(
mixin.prototype as MixinBase<DestinationClass['prototype']>,
);
const usingDescriptors: { [prop: string]: PropertyDescriptor } = {};
for (const [prop, descriptor] of Object.entries(originalDescriptors)) {
if (['constructor'].includes(prop)) {
continue;
}
const dataTemplates: Record<string, unknown>[] = [];
const dataOptimizations: ((data: unknown) => void)[] = [];
const constructors: ((data: Partial<unknown>) => void)[] = [];

for (const mixin of mixins) {
// The entire prototype chain, in reverse order, since we want to copy it all
const prototypeChain: MixinBase<DestinationClass['prototype']>[] = [];
let extendedClass = mixin;
while (extendedClass.prototype !== undefined) {
if (
typeof descriptor.get !== 'undefined' ||
typeof descriptor.set !== 'undefined' ||
typeof descriptor.value === 'function'
DataTemplatePropetyName in extendedClass &&
typeof extendedClass.DataTemplate === 'object' &&
// eslint-disable-next-line no-eq-null, eqeqeq
extendedClass.DataTemplate != null
) {
usingDescriptors[prop] = descriptor;
dataTemplates.push(extendedClass.DataTemplate as Record<string, unknown>);
}

prototypeChain.unshift(extendedClass.prototype);
extendedClass = Object.getPrototypeOf(extendedClass);
}

for (const prototype of prototypeChain) {
// Synboled data isn't traversed by Object.entries, we can handle it here
if (prototype[kMixinConstruct]) {
constructors.push(prototype[kMixinConstruct]);
}

// Copy instance methods and setters / getters
const originalDescriptors = Object.getOwnPropertyDescriptors(prototype);
const usingDescriptors: { [prop: string]: PropertyDescriptor } = {};
for (const [prop, descriptor] of Object.entries(originalDescriptors)) {
// Drop constructor
if (['constructor'].includes(prop)) {
continue;
}

// Special case for optimize function, we want to combine these
if (prop === OptimizeDataPropertyName) {
if (typeof descriptor.value !== 'function') return;
dataOptimizations.push(descriptor.value);
continue;
}

// Shouldn't be anything other than these without being instantiated, but just in case
if (
typeof descriptor.get !== 'undefined' ||
typeof descriptor.set !== 'undefined' ||
typeof descriptor.value === 'function'
) {
usingDescriptors[prop] = descriptor;
}
}

Object.defineProperties(destination.prototype, usingDescriptors);
}

// Set the function to call any mixed constructors
if (constructors.length > 0) {
Object.defineProperty(destination.prototype, kMixinConstruct, {
writable: true,
enumerable: false,
configurable: true,
// eslint-disable-next-line func-name-matching
value: function _mixinConstructors(data: Partial<unknown>) {
for (const construct of constructors) {
construct.call(this, data);
}
},
});
}

// Combine all optimizations into a single function
if (dataOptimizations.length > 0) {
const baseOptimize = Object.getOwnPropertyDescriptor(destination, OptimizeDataPropertyName);
if (baseOptimize && typeof baseOptimize.value === 'function') {
dataOptimizations.unshift(baseOptimize.value);
}

Object.defineProperty(destination.prototype, OptimizeDataPropertyName, {
writable: true,
enumerable: false,
configurable: true,
// eslint-disable-next-line func-name-matching
value: function _optimizeData(data: unknown) {
for (const optimization of dataOptimizations) {
optimization.call(this, data);
}
},
});
}

Object.defineProperties(destination.prototype, usingDescriptors);
// Copy the properties (setters) of each mixins template to the destinations template
if (dataTemplates.length > 0) {
destination[DataTemplatePropetyName] ??= {};
for (const template of dataTemplates) {
Object.defineProperties(destination[DataTemplatePropetyName], Object.getOwnPropertyDescriptors(template));
}
}
}
}

Expand All @@ -76,7 +156,7 @@ export type MixinTypes<
Structure<DataType, Omitted>[typeof kData] extends
// @ts-expect-error kData is protected
Mixins[number][typeof kData]
? Omit<MergePrototypes<Mixins>, keyof BaseClass>
? Omit<MergePrototypes<Mixins>, keyof BaseClass | typeof kMixinConstruct>
: never
: never
: never
Expand Down
52 changes: 40 additions & 12 deletions packages/structures/src/Structure.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
/* eslint-disable jsdoc/check-param-names */
import { kData } from './utils/symbols.js';
import { kData, kMixinConstruct } from './utils/symbols.js';
import type { ReplaceOmittedWithUnknown } from './utils/types.js';

/**
* Additional options needed to appropriately construct / patch data
*
* @internal
*/
export interface StructureExtraOptions {
/**
* The template used to remove unwanted properties from raw data storage
*/
template?: {};
}
// eslint-disable-next-line @typescript-eslint/no-empty-interface
export interface StructureExtraOptions {}

export const DataTemplatePropetyName = 'DataTemplate';
export const OptimizeDataPropertyName = '_optimizeData';

/**
* Represents a data model from the Discord API
Expand All @@ -30,6 +29,32 @@ export interface StructureExtraOptions {
* This is the most technically correct way of represnting the value, especially since there is no way to guarantee runtime matches the "type cast."
*/
export abstract class Structure<DataType, Omitted extends keyof DataType | '' = ''> {
/**
* A construct function used when mixing to allow mixins to set optimized property defaults
*
* @remarks This should only be used to set defaults, setting optimized values should be done
* in the mixins `_optimizeData` method, which is automatically called and should not be called here.
* @remarks Furthermore, this function will be called before the constructor in the mixed class, and can therefore
* never override defaults of the base class.
* @param data - The full API data received by the Structure
*/
protected [kMixinConstruct]?(data: Partial<DataType>): void;

/**
* The template used for removing data from the raw data stored for each Structure.
*
* @remarks This template should be overriden in all subclasses to provide more accurate type information.
* The template in the base {@link Structure} class will have no effect on most subclasses for this reason.
*/
protected static [DataTemplatePropetyName]: Record<string, unknown> = {};

/**
* @returns A cloned version of the data template, ready to create a new data object.
*/
private _getDataTemplate() {
return Object.create((this.constructor as typeof Structure).DataTemplate);
}

/**
* The raw data from the API for this struture
*
Expand All @@ -42,10 +67,12 @@ export abstract class Structure<DataType, Omitted extends keyof DataType | '' =
*
* @param data - the data from the API that this structure will represent
* @param extraOptions - any additional options needed to appropriately construct a structure from the data
* @remarks To be made public in subclasses
* @internal
*/
protected constructor(data: Readonly<Partial<DataType>>, { template }: StructureExtraOptions = {}) {
this[kData] = Object.assign(template ? Object.create(template) : {}, data);
protected constructor(data: Readonly<Partial<DataType>>) {
this[kData] = Object.assign(this._getDataTemplate(), data);
this[kMixinConstruct]?.(data);
this._optimizeData(data);
}

Expand All @@ -54,11 +81,12 @@ export abstract class Structure<DataType, Omitted extends keyof DataType | '' =
*
* @param data - the updated data from the API to patch with
* @param extraOptions - any additional options needed to appropriately patch the data
* @remarks To be made public in subclasses
* @returns this
* @internal
*/
protected _patch(data: Readonly<Partial<DataType>>, { template }: StructureExtraOptions = {}): this {
this[kData] = Object.assign(template ? Object.create(template) : {}, this[kData], data);
protected _patch(data: Readonly<Partial<DataType>>): this {
this[kData] = Object.assign(this._getDataTemplate(), this[kData], data);
this._optimizeData(data);
return this;
}
Expand All @@ -72,7 +100,7 @@ export abstract class Structure<DataType, Omitted extends keyof DataType | '' =
* @virtual
* @internal
*/
protected _optimizeData(_data: Partial<DataType>) {}
protected [OptimizeDataPropertyName](_data: Partial<DataType>) {}

/**
* Transforms this object to its JSON format with raw API data (or close to it),
Expand Down
Loading

0 comments on commit 1846ab6

Please sign in to comment.