From e15a8fc035549f362a86da9006400a7d4e813762 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 14 Jul 2021 23:30:27 -0400 Subject: [PATCH 1/4] Added defaultOptions and hardcoded two-level defaults nesting --- index.d.ts | 33 +++++++++++----- index.js | 5 +++ index.test-d.ts | 101 ++++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 121 insertions(+), 18 deletions(-) diff --git a/index.d.ts b/index.d.ts index 31ff266..f4c0e97 100644 --- a/index.d.ts +++ b/index.d.ts @@ -31,6 +31,13 @@ declare type ReturnTypeOf = ? UnionToIntersection, void>> : never; +type ConstructorRequiringVersion = + { defaultOptions: PredefinedOptions } & ( + PredefinedOptions extends { version: string } + ? { new (options?: NowProvided): Class & { options: NowProvided & PredefinedOptions }; } + : { new (options: Base.Options & NowProvided): Class & { options: NowProvided & PredefinedOptions }; } + ); + export declare class Base { static plugins: Plugin[]; @@ -75,18 +82,24 @@ export declare class Base { * const base = new MyBase({ option: 'value' }); // `version` option is not required * base.options // typed as `{ version: string, otherDefault: string, option: string }` * ``` + * @remarks + * Ideally, we would want to make this infinitely recursive: allowing any number of + * .defaults({ ... }).defaults({ ... }).defaults({ ... }).defaults({ ... })... + * However, we don't see a clean way in today's TypeSript syntax to do so. + * We instead artificially limit accurate type inference to just three levels, + * since real users are not likely to go past that. + * @see https://github.com/gr2m/javascript-plugin-architecture-with-typescript-definitions/pull/57 */ static defaults< - TDefaults extends Base.Options, - S extends Constructor> - >( - this: S, - defaults: Partial - ): { - new (...args: any[]): { - options: TDefaults; - }; - } & S; + PredefinedOptionsOne, + Class extends Constructor> + >(this: Class, defaults: PredefinedOptionsOne): ConstructorRequiringVersion & { + defaults(this: Class, defaults: PredefinedOptionsTwo): ConstructorRequiringVersion & { + defaults(this: Class, defaults: PredefinedOptionsThree): ConstructorRequiringVersion & Class; + } & Class; + } & Class; + + static defaultOptions: {}; /** * options passed to the constructor as constructor defaults diff --git a/index.js b/index.js index f715b4d..6096996 100644 --- a/index.js +++ b/index.js @@ -14,13 +14,18 @@ export class Base { ); }; } + static defaults(defaults) { return class extends this { constructor(...args) { super(Object.assign({}, defaults, args[0] || {})); } + + static defaultOptions = defaults; }; } + static defaultOptions = {}; + static plugins = []; } diff --git a/index.test-d.ts b/index.test-d.ts index bca6858..e9b7f55 100644 --- a/index.test-d.ts +++ b/index.test-d.ts @@ -13,21 +13,105 @@ const base = new Base({ // @ts-expect-error unknown properties cannot be used, see #31 base.unknown; -const BaseWithDefaults = Base.defaults({ +const BaseWithEmptyDefaults = Base.defaults({ // there should be no required options }); -const FooBase = Base.plugin(fooPlugin).defaults({ - default: "value", +// 'version' is missing and should still be required +// @ts-expect-error +new BaseWithEmptyDefaults() + +// 'version' is missing and should still be required +// @ts-expect-error +new BaseWithEmptyDefaults({}) + +const BaseLevelOne = Base.plugin(fooPlugin).defaults({ + defaultOne: "value", version: "1.2.3", }); -const fooBase = new FooBase({ - option: "value", + +// Because 'version' is already provided, this needs no argument +new BaseLevelOne(); +new BaseLevelOne({}); + +expectType<{ + defaultOne: string, + version: string, +}>(BaseLevelOne.defaultOptions); + +const baseLevelOne = new BaseLevelOne({ + optionOne: "value", }); -expectType(fooBase.options.default); -expectType(fooBase.options.option); -expectType(fooBase.foo); +expectType(baseLevelOne.options.defaultOne); +expectType(baseLevelOne.options.optionOne); +expectType(baseLevelOne.options.version); +// @ts-expect-error unknown properties cannot be used, see #31 +baseLevelOne.unknown; + +const BaseLevelTwo = BaseLevelOne.defaults({ + defaultTwo: 0, +}); + +expectType<{ + defaultOne: string, + defaultTwo: number, + version: string, +}>(BaseLevelTwo.defaultOptions); + +// Because 'version' is already provided, this needs no argument +new BaseLevelTwo(); +new BaseLevelTwo({}); + +// 'version' may be overriden, though it's not necessary +new BaseLevelTwo({ + version: 'new version', +}); + +const baseLevelTwo = new BaseLevelTwo({ + optionTwo: true +}); + +expectType(baseLevelTwo.options.defaultTwo); +expectType(baseLevelTwo.options.defaultOne); +expectType(baseLevelTwo.options.optionTwo); +expectType(baseLevelTwo.options.version); +// @ts-expect-error unknown properties cannot be used, see #31 +baseLevelTwo.unknown; + +const BaseLevelThree = BaseLevelTwo.defaults({ + defaultThree: ['a', 'b', 'c'], +}); + +expectType<{ + defaultOne: string, + defaultTwo: number, + defaultThree: string[], + version: string, +}>(BaseLevelThree.defaultOptions); + +// Because 'version' is already provided, this needs no argument +new BaseLevelThree(); +new BaseLevelThree({}); + +// Previous settings may be overriden, though it's not necessary +new BaseLevelThree({ + optionOne: '', + optionTwo: false, + version: 'new version', +}); + +const baseLevelThree = new BaseLevelThree({ + optionThree: [0, 1, 2] +}); + +expectType(baseLevelThree.options.defaultOne); +expectType(baseLevelThree.options.defaultTwo); +expectType(baseLevelThree.options.defaultThree); +expectType(baseLevelThree.options.optionThree); +expectType(baseLevelThree.options.version); +// @ts-expect-error unknown properties cannot be used, see #31 +baseLevelThree.unknown; const BaseWithVoidPlugin = Base.plugin(voidPlugin); const baseWithVoidPlugin = new BaseWithVoidPlugin({ @@ -69,3 +153,4 @@ const baseWithOptionsPlugin = new BaseWithOptionsPlugin({ }); expectType(baseWithOptionsPlugin.getFooOption()); + From 61c8fd78c6bfc37b33d6c3cb041f3b1ccfaeafd9 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Thu, 15 Jul 2021 00:23:05 -0400 Subject: [PATCH 2/4] Update index.js --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index 6096996..4027b80 100644 --- a/index.js +++ b/index.js @@ -21,7 +21,7 @@ export class Base { super(Object.assign({}, defaults, args[0] || {})); } - static defaultOptions = defaults; + static defaultOptions = { ...defaults, ...this.defaultOptions }; }; } From 8cd146fa80a4609daa81086fdcd411677dcbcbf4 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Thu, 15 Jul 2021 00:26:35 -0400 Subject: [PATCH 3/4] Added and corrected test coverage --- index.test-d.ts | 4 ++-- test/base.test.js | 10 ++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/index.test-d.ts b/index.test-d.ts index e9b7f55..b3b5cf8 100644 --- a/index.test-d.ts +++ b/index.test-d.ts @@ -57,7 +57,7 @@ expectType<{ defaultOne: string, defaultTwo: number, version: string, -}>(BaseLevelTwo.defaultOptions); +}>({ ...BaseLevelTwo.defaultOptions }); // Because 'version' is already provided, this needs no argument new BaseLevelTwo(); @@ -88,7 +88,7 @@ expectType<{ defaultTwo: number, defaultThree: string[], version: string, -}>(BaseLevelThree.defaultOptions); +}>({ ...BaseLevelThree.defaultOptions }); // Because 'version' is already provided, this needs no argument new BaseLevelThree(); diff --git a/test/base.test.js b/test/base.test.js index 619521f..df98d33 100644 --- a/test/base.test.js +++ b/test/base.test.js @@ -33,10 +33,20 @@ test(".defaults({foo: 'bar'})", () => { const BaseWithDefaults = Base.defaults({ foo: "bar" }); const defaultsTest = new BaseWithDefaults(); const mergedOptionsTest = new BaseWithDefaults({ baz: "daz" }); + assert.equal(BaseWithDefaults.defaultOptions, { foo: "bar" }); assert.equal(defaultsTest.options, { foo: "bar" }); assert.equal(mergedOptionsTest.options, { foo: "bar", baz: "daz" }); }); +test(".defaults({foo: 'bar', baz: 'daz' })", () => { + const BaseWithDefaults = Base.defaults({ foo: "bar" }).defaults({ baz: "daz" }); + const defaultsTest = new BaseWithDefaults(); + const mergedOptionsTest = new BaseWithDefaults({ faz: "boo" }); + assert.equal(BaseWithDefaults.defaultOptions, { foo: "bar", baz: "daz" }); + assert.equal(defaultsTest.options, { foo: "bar", baz: "daz" }); + assert.equal(mergedOptionsTest.options, { foo: "bar", baz: "daz", faz: "boo" }); +}); + test(".plugin().defaults()", () => { const BaseWithPluginAndDefaults = Base.plugin(fooPlugin).defaults({ baz: "daz", From 8a27ce530e7fd140b98069d20d9f35ba0bffb841 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Thu, 15 Jul 2021 00:27:18 -0400 Subject: [PATCH 4/4] One small typo --- index.d.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/index.d.ts b/index.d.ts index f4c0e97..dafd1b2 100644 --- a/index.d.ts +++ b/index.d.ts @@ -26,16 +26,16 @@ declare type UnionToIntersection = ( declare type AnyFunction = (...args: any) => any; declare type ReturnTypeOf = T extends AnyFunction - ? ReturnType - : T extends AnyFunction[] - ? UnionToIntersection, void>> - : never; + ? ReturnType + : T extends AnyFunction[] + ? UnionToIntersection, void>> + : never; type ConstructorRequiringVersion = { defaultOptions: PredefinedOptions } & ( PredefinedOptions extends { version: string } - ? { new (options?: NowProvided): Class & { options: NowProvided & PredefinedOptions }; } - : { new (options: Base.Options & NowProvided): Class & { options: NowProvided & PredefinedOptions }; } + ? { new (options?: NowProvided): Class & { options: NowProvided & PredefinedOptions }; } + : { new (options: Base.Options & NowProvided): Class & { options: NowProvided & PredefinedOptions }; } ); export declare class Base { @@ -85,7 +85,7 @@ export declare class Base { * @remarks * Ideally, we would want to make this infinitely recursive: allowing any number of * .defaults({ ... }).defaults({ ... }).defaults({ ... }).defaults({ ... })... - * However, we don't see a clean way in today's TypeSript syntax to do so. + * However, we don't see a clean way in today's TypeScript syntax to do so. * We instead artificially limit accurate type inference to just three levels, * since real users are not likely to go past that. * @see https://github.com/gr2m/javascript-plugin-architecture-with-typescript-definitions/pull/57 @@ -108,4 +108,4 @@ export declare class Base { constructor(options: TOptions); } -export {}; +export { };