feat: derive constructor options from chainable Base.defaults() calls

[gr2m]

Given that a version constructor parameter is required, the following code should work without type errors.

const Base123 = Base.defaults({ version: "1.2.3" });
const base = new Base123();

[gr2m]

I asked a question about this on StackOverflow:
https://stackoverflow.com/questions/68201386/how-to-make-required-constructor-options-optional-when-they-were-set-using-mycla

[gr2m]

progress (I think): I managed to create a function which accepts both defaults and options, and all the type behavior behaves as expected:

type Optional<T extends object, K extends string | number | symbol> = Omit<T, K> & Partial<Pick<T, keyof T & K>>;

type Options = {
  requiredOption: string;
  optionalOption?: string;
};

function testWithDefaults<TDefaults extends Options>(defaults: TDefaults);
function testWithDefaults<
  TDefaults extends Partial<Options>,
  TOptions extends Optional<Options, keyof TDefaults> & Record<string, unknown>
>(defaults: TDefaults, options: TOptions);
function testWithDefaults(
  defaults: Record<string, unknown>,
  options?: Record<string, unknown>
) {
  return {
    ...defaults,
    ...options,
  };
}

const test = testWithDefaults({ requiredOption: ""}, {})

playground link

[gr2m]

I've spent way too much time on this already.

The answers to my the StackOverflow question do not work, because they both only implement a single Base.defaults() call and do not support chaining (Base.defaults().defaults()).

I've reached out to @JoshuaKGoldberg for more help and their assessment is that it's not possible with today's TypeScript features. Josh opened two issues on the TS repository

• Allow value pointing to a class or whose type has a new signature to be used as a type microsoft/TypeScript#45013
• Allow class expressions in type declarations microsoft/TypeScript#45014

two other related issues are

• typeof class microsoft/TypeScript#41581
• Polymorphic "this" for static members microsoft/TypeScript#5863

I'd like to try one more approach: limit the amount of chaining to 3x and implement the carrying of state when chaining static class methods by duplicating & nesting code.

[gr2m]

why the destructoring? Isn't this the same as

Suggested change

	}>({ ...BaseLevelTwo.defaultOptions });
	}>(BaseLevelTwo.defaultOptions);

[JoshuaKGoldberg]

Ha, this one was nifty. tsd sees an intersection type as not being the same as its equivalent solo type.

  index.test-d.ts:86:0
  ✖  56:0  Parameter type { defaultOne: string; defaultTwo: number; version: string; } is not identical to argument type { defaultOne: string; version: string; } & { defaultTwo: number; }.  

[gr2m]

I accidentally closed #59 by pushing it to the defaults-options-inheritance branch in this repository instead of @JoshuaKGoldberg's fork.

Josh wrote

Builds onto #57 with the long-lived temporary solution @gr2m referenced in that PR thread of having a known finite level of nesting.

I did away with fancy conditional types to make sure the type system doesn't get bogged down computing them all for plugins with a few options. I can add them back if you have test cases that require more than just version being required?

The big return type of the static defaults in theory could be automated a little bit with a recursive type. However, we wouldn't be able to go infinitely deep, and again the code to get that to work would be a good bit longer and less readable than how it is now. 🤷

I think what broke is the combination of .plugin().defaults() call now

Here is a complex test I'd like to see pass

const BaseWithManyChainedDefaultsAndPlugins = Base.defaults({
  defaultOne: "value",
})
  .plugin(fooPlugin, barPlugin, voidPlugin)
  .defaults({
    defaultTwo: 0,
  })
  .plugin(withOptionsPlugin)
  .defaults({
    defaultThree: ["a", "b", "c"],
  });

expectType<{
  defaultOne: string;
  defaultTwo: number;
  defaultThree: string[];
}>({ ...BaseWithManyChainedDefaultsAndPlugins.defaultOptions });

const baseWithManyChainedDefaultsAndPlugins =
  new BaseWithManyChainedDefaultsAndPlugins({
    version: "1.2.3",
    foo: "bar",
  });

expectType<string>(baseWithManyChainedDefaultsAndPlugins.foo);
expectType<string>(baseWithManyChainedDefaultsAndPlugins.bar);
expectType<string>(baseWithManyChainedDefaultsAndPlugins.getFooOption());

Right now the last three expectType calls checking the mixins from the plugins fail

A minimal test case to reproduce this problem is

const BaseWithManyChainedDefaultsAndPlugins = Base.defaults({
  defaultOne: "value",
})
  .plugin(fooPlugin)
  .defaults({
    defaultTwo: 0,
  });

const baseWithManyChainedDefaultsAndPlugins =
  new BaseWithManyChainedDefaultsAndPlugins({
    version: "1.2.3",
  });

expectType<string>(baseWithManyChainedDefaultsAndPlugins.foo);

Calling Base.defaults().plugin().defaults() breaks the mixins from the plugins. Both Base.defaults().plugin() and Base.plugin().defaults() works

[JoshuaKGoldberg]

New little nifty improvement recently added to tuples in TypeScript ✨ no more multiple parameters!

[JoshuaKGoldberg]

This is a tough one! It doesn't repro if either of the .defaults({ ... }) are removed above... Tricky!

[JoshuaKGoldberg]

Got it! It was the this: Class generic.

My leading hunch was that whatever information added the foo: string from the .plugin(fooPlugin) call was being abandoned by the .defaults calls going on top of each other. That meant the piece of the type that contained foo wasn't getting preserved.

That piece of type was the this: Class -> & Class. Using just one <Class> generic from the start meant new type information on new this scopes that had extra stuff added to them was being lost.

[gr2m]

just a question, looks great otherwise! Super cool that you got it all working!

[gr2m]

oops sorry I forgot to submit my review yesterday

[gr2m]

It's okay to hard-code the required version right now, but we should make this flexible later.

The Base.Options interface can be extended with other required options, and these should be taken into account. Also the version is random and I just put it in here for exploring the blockers I ran into with Octokit, I don't think it makes sense to have any default options in the Base class, they should all be added when using Base by extending the Base.Options interface

I'll create a follow up issue, I think this PR is big enough as-is 😁

[github-actions]

🎉 This PR is included in version 3.3.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀