Enforce strict typing for TypeScript interfaces

[zharinov]

This issue is meant to be a starting point of discussion on how we can gradually enforce more strict types.

Some questions are:

• Is it the good idea force nullable types when possible? To the one hand, it would be nice to have single semantics instead of potentially different !!foo, foo === null, foo === undefined and typeof foo.bar === 'undefined'. Also, it enhance debugging UX a little bit. To the another hand, there can be endpoints where we may need something like if (foo.bar === null) { delete foo.bar; }. For the latter case, how can we avoid ad-hoc way of doing so?

• Which boundary checkpoints should we introduce? For example, here is the one for BranchConfig type. The boundaries can be divided to external -> internal, internal -> internal or internal -> external. How should we deal with each of these types?

• Which tools do we need? Some of requirements can be:

  • Validation
  • Powerful enough schema DSL (nested composition, and maybe pick/omit functions too)
  • Simple coercions (example API):
    • default values such as null for missing fields
    • string functions like trim(), etc
  • Type inference, once we have strictNullChecks
  • Error messages for logger
  • JSON schema support?

  From projects I've seen so far, looks like it's hard to have all of these in just one library. Thus, we may need to pick the good enough one, and then compose with it.

• On which 20% of code can we work to reduce 80% of strictNullChecks errors? My guess is that we should search the places where we make "defensive" assumptions on input data and propagate them further.

[zharinov]

One more type enforcement approach which isn't requiring structNullChecks: typecheck.macro.

UPD. The downside is that validated types should be defined in single file

[zharinov]

Here is another concern.
In the example below, for input we can have semanticCommits: 'enabled' | 'disabled' | 'auto'.
But in the downstream code, it will be only 'enabled' | 'disabled'.
The question is how such a type refining can be done?

export async function getRepoConfig(
  config_: RenovateConfig
): Promise<RenovateConfig> {
  let config = { ...config_ };
  // ...
  config.semanticCommits =
    config.semanticCommits ?? (await detectSemanticCommits());
  return config;
}

[zharinov]

Here is blog-post listing different approaches: https://2ality.com/2020/06/validating-data-typescript.html

[viceice]

Maybe a better approach is to fully define typescript types / interfaces for our allowed config options, and then generate the json schema from them.

We can additionally use zod to validate config. If not valid, try to migrate, if again not valid throw an error. 🤔
Traversing our config multiple times shouldn't be an issue for us, as other parts of the app are more time intensive.

[zharinov]

What about space between validated point A and validated point B? We want to have some degree of typesafe freedom while being able to recompose functions working with incomplete data. I don't think it's a best idea to pre-fill config A with null "holes" and then "initialize" them in order to reach the point B. It would be pretty an equivalent to optional types we're using for now, and also it couples null as no data at the moment and null as the part of model.

What do you think about pattern like this?

// point A

interface StartConfig {
  foo: string;
}

// bar.ts

interface Bar {
  bar: number;
}

function bar<T extends StartConfig>(config: T): T & Bar {
  return { ...config, bar: config.foo.length };
}

// baz.ts

interface Baz {
  baz: boolean;
}

function baz<T extends StartConfig & Bar>(config: T): T & Baz {
  return { ...config, baz: config.bar % 2 === 0 };
}

// qux.ts

interface Qux {
  qux?: string;
}

function qux<T extends StartConfig & Qux>(config: T): T & { qux: string } {
  return { ...config, qux: 'qux' + (config.qux || '') };
}

// point B

interface FinishConfig extends StartConfig {
  bar: number;
  baz: boolean;
  qux: string;
}

function transformConfig(config: StartConfig): FinishConfig {
  return baz(qux(bar(config)));
//   return bar(qux(baz(config)));
}

console.log(transformConfig({ foo: 'hello' }));

[JamieMagee]

semi-related: we're also missing a few strict-related options from this configuration: https://github.com/tsconfig/bases/blob/main/bases/strictest.json

[viceice]

Yeah, let's do one step at time and get fully strict.

[viceice]

@zharinov We should raise priority for strict null checks, as they currently make review harder, brcause most user see the issues on failing ci only. it also wastes a lot of ci resources. So lets get at least the null-checks done.