definitions for schemas w/ typed additionalProperties are not compatible w/ strictNullChecks

[G-Rath]

This is all based around an attempt to resolve issues like #201, #210, etc

Currently, j2t generates definitions that are not strict mode compatible:

{
  "$schema": "http://json-schema.org/draft-04/schema#",
  "properties": {
    "myProp": {
      "type": "string"
    }
  },
  "additionalProperties": {
    "type": "string"
  }
}

results in

/* tslint:disable */
/**
 * This file was automatically generated by json-schema-to-typescript.
 * DO NOT MODIFY IT BY HAND. Instead, modify the source JSONSchema file,
 * and run json-schema-to-typescript to regenerate this file.
 */

export interface Foo {
  myProp?: string;
  [k: string]: string;
}

This isn't valid under strict, specifically due to strictNullChecks.

To be valid, | undefined needs to be explicitly added.
The "downside" to this is that you have to guard against undefined:

interface Mx {
  [k: string]: string | undefined;
}

const o: Mx = {};

o.myProp.toUpperCase();
// ^^ Error:(53, 1) TS2532: Object is possibly 'undefined'.

for (const key in o) {
  o[key].toUpperCase();
  // ^^ Error:(57, 3) TS2532: Object is possibly 'undefined'.
}

Personally, I actually think this is for the better anyway, given that since you're not requiring myProp, it could very well be undefined.

This isn't actually a breaking change, since this will only affect code that has strictNullChecks, but if strictNullChecks is on, generated definitions would stop compiling anyway, making it a catch-22 😄

---

The implementation on how to support this is the big question - For package.json, the fix is easy: use tsType, but the problem w/ this is it's a complete override, which means its brittle:

{
  "$schema": "http://json-schema.org/draft-04/schema#",
  "properties": {
    "myProp": {
      "type": "string"
    }
  },
  "definitions": {
    "myDef": {
      "type": "string"
    }
  },
  "additionalProperties": {
    "$ref": "#/definitions/myDef",
    "tsType": "undefined"
  }
}

Desired:

export type MyDef = string;

export interface Foo2 {
  myProp?: string;
  [k: string]: MyDef | undefined;
}

Actual:

export interface Foo {
  myProp?: string;
  [k: string]: undefined;
}

Hence, ideally, what we need is a way to append types; I don't really see much of a use-case outside of appending undefined, so I'd be happy w/ something like "tsCanBeUndefined": true.

Alternatively, a solution that is a bit nicer but more breaking (IMO) would be whenever an interface is generated w/ an index type (other than any), to add | undefined if the interface has any optional properties.

[bcherny]

Alternatively, a solution that is a bit nicer but more breaking (IMO) would be whenever an interface is generated w/ an index type (other than any), to add | undefined if the interface has any optional properties.

This seems like the correct solution to me. Want to PR this behind a --strictIndexSignatures option?

[G-Rath]

Awesome - I'll have a crack at a PR, but will ping you if I run into any problems :)

[G-Rath]

I've started work on this.

In refreshing my memory on this, I think at least for now I'm going to have --strictIndexSigntures stick | undefined on all index signatures (instead of only ones that have optional properties).

The reasoning for this is that you can't assume a property exists unless it's defined, and an index signature is saying that any extra properties which may or may not exist should be of type "x".

This is actually a bit of a sticky situation; the whole reason TS doesn't treat index signatures by default as | undefined is actually for convenience - when you have an index signature, you don't have any reason to do a direct access (since you don't know the names of the properties); instead you'd typically loop - however if it's | undefined, it'd mean you'd have to use a guard:

interface Mx {
  [k: string]: number | undefined;
}

const o: Mx = {};

for (const key in o) {
    const v = o[key];

    if(!v) {
        continue;
    }
    
    v.toUpperCase();
}

This isn't a problem for defined properties:

interface Mx {
  name: string;
  [k: string]: string | undefined;
}

const o: Mx = {
    name: ''
};

o.name.toLocaleLowerCase();

o.myProp.toUpperCase();
// ^^ Error:(53, 1) TS2532: Object is possibly 'undefined'.

Worse-case, someone can complain & we can look to refine the behaviour :)

Just wanted to have this written down so that I don't forgot it again 😄
This is the issue that tracks the situation in TypeScript: microsoft/TypeScript#13778