Export additionalProperties as intersection for complex types

[fabrykowski]

A quick fix for #299, #356 and #402.

Given the schema

{
  "title": "AdditionalProperties",
  "type": "object",
  "properties": {
    "foo": {
      "type": "string"
    }
  },
  "additionalProperties": {
    "type": "number"
  }
}

it is rendered by the library as

export interface AdditionalProperties {
  foo?: string;
  [k: string]: number;
}

which results in the compiler error

TS2411: Property 'foo' of type 'string | undefined' is not assignable to string index type 'number'.

This PR renders the schema above as

export type AdditionalProperties = {
  foo?: string;
} & {
  [k: string]: number;
};

which is valid TypeScript.

[bohjak]

But it doesn't seem to actually fix the problem? Or am I doing something wrong?

The generated type is indeed valid TypeScript in the sense that it does compile. But if you try to create an object with it, it will still fail:

export type AdditionalProperties = {
  foo?: string;
} & {
  [k: string]: number;
};

const bar: AdditionalProperties = {
    foo: 'baz'
}

/*
Type '{ foo: string; }' is not assignable to type 'AdditionalProperties'.
  Type '{ foo: string; }' is not assignable to type '{ [k: string]: number; }'.
    Property 'foo' is incompatible with index signature.
      Type 'string' is not assignable to type 'number'.
*/

TypeScript Playground

[fabrykowski]

It is impossible to create such an object in TypeScript, but you can use it after a cast or assertion.

export type AdditionalProperties = {
  foo?: string;
} & {
  [k: string]: number | undefined;
};

const bar = {
    foo: 'bar',
    baz: 42
} as unknown as AdditionalProperties;

const foo = bar.foo; // type: string | undefined
const baz = bar.baz; // type: number | undefined
const tmp = bar.tmp; // type: number | undefined

TypeScript Playground

[fabrykowski]

This is especially of interest, when a received JSON object – such as an HTTP request body – is parsed.

[fabrykowski]

We need this in our code base – any chance of a merge?

[Michael-Hering]

+1 - TS serverless files are a huge win for teams building serverless apps in TS and this PR would allow that to happen

[maximseshuk]

+1

[luc-heyday]

+1

[electblake]

+1

[fabrykowski]

Any news? We could really use this in our production code. @bcherny This PR could close three issues at once.

[macsj200]

I would greatly appreciate this fix as I'm currently blocked by https://github.com/serverless/typescript/issues/27.

[fabrykowski]

@bcherny – this PR would close multiple tickets. Would it be possible to initiate a review?

[G-Rath]

@fabrykowski (and potential reviewers) please keep in mind my comment here: https://github.com/bcherny/json-schema-to-typescript/issues/356#issuecomment-751144641

This change will be breaking as it prevents you from using declaration merging (which can only be done with interfaces) which can be a very powerful tool with additionalProperties.

A better solution would be to generate a KnownProperties interface that is used in-place of the anonymous interface in the union that this change has generated.

e.g.

interface AdditionalPropertiesKnownProperties {
  foo?: string;
}

export type AdditionalProperties = AdditionalPropertiesKnownProperties & { 
  [k: string]: number;
};

[fabrykowski]

@G-Rath – in the example you gave only the finished type AdditionalProperties is exported – which means that declaration merging would not be available. On the other hand exporting the [...]KnownProperties-interface for each type with additionalProperties seems a tad unwieldy and pollutes – in my opinion – the namespace.

I would argue that declaration merging is a very useful tool for DefinitelyTyped, allowing for example to define specific headers; should not the aim of this package be the opposite? If I have a JSON-schema and I validate a JSON-object (e.g. via ajv) then I can safely cast the object to (or assert it to be) the generated TypeScript-type! (This is, at least, the way I use this package.)

If there is strong demand for it, I could add another optional compile option (additionalPropertiesInterfaceSuffix?) which, when not falsey, turnes on the export of interface {TypeName}{additionalPropertiesInterfaceSuffix}. I believe, however, that this should be a second PR, because this PR is "breaking" only in the most formal sense, since the currently generated interface does not compile anyways. (This PR still returns an interface, when no additionalProperties are present.)

[G-Rath]

in the example you gave only the finished type AdditionalProperties is exported ... exporting the [...]KnownProperties-interface for each type with additionalProperties seems a tad unwieldy and pollutes – in my opinion – the namespace.

Sure in a perfect world it'd be great to less stuff while still being able to have the flexibility but this is what the language requires us to do in order for this change to not be breaking + retain that flexibility.

should not the aim of this package be the opposite I can safely cast the object to (or assert it to be) the generated TypeScript-type!

While neither are terrible alternatives, they are not without costs: casting can introduce bugs if not understood and maintained properly because the whole point of them is to have TypeScript ignore type violations like missing properties and type mismatches, and assertions can have a significant runtime cost depending on the size of the objects (which people may shortcut with approximation assertions, which is fine but then you're in the first tradeoff again).

because this PR is "breaking" only in the most formal sense This PR still returns an interface, when no additionalProperties are present.

Why is this being re-written as a type then?

I'm definitely happier if it doesn't change interfaces with no additionalProperties are present, but I still think because there's a solution that allows you to have your cake and eat it too is better than this current one - splitting that out into another config option IMO is bloating the library needlessly.

---

Please keep in mind that not everyone is validating their objects against the actual schemas at runtime - for example, over at https://github.com/octokit/webhooks we maintain schemas for all the webhook payloads which are used to generate the types that consumed by related packages; while the schemas are valid and can be used, they are not by the related octokit packages that are using the types. This is also the case for the rest of the GitHub API.

Declaration merging is very powerful here for the same reason its powerful over at DT: if we've made a mistake, or there's some new feature, and no one's gotten around to updating the schemas & types (we're pretty responsive but stuff happens), then people can easily amend the types using DM.