bcherny / json-schema-to-typescript

Compile JSON Schema to TypeScript type declarations
https://bcherny.github.io/json-schema-to-typescript-browser/
MIT License
2.95k stars 392 forks source link

`additionalProperties` union clashes with defined properties #402

Open joscha opened 3 years ago

joscha commented 3 years ago

A schema of:

{
  "type": "object",
  "properties": {
    "$schema": {
      "description": "The JSON schema reference.",
      "type": "string"
    },
  },
  "additionalProperties": {
   "type": "object"
  }
}

resolves to:

export interface MySchema {
  /**
   * The JSON schema reference.
   */
  $schema?: string;
  [k: string]: {
    [k: string]: unknown;
  };
}

which is an incompatible type, as $schema clearly doesn't follow { [k: string]: unknown }. You can see the error in the typescript playground.

I encountered this here: https://dprint.dev/schemas/v0.json which is the schema for the dprint configuration. (cc @dsherret)

A potential fix is to generate additionalProperties as:

  [k: string]: {
    [k: string]: unknown;
  } | unknown;

Test code:

const { compile } = require('json-schema-to-typescript');

// or, compile a JS object
let mySchema = {
  "type": "object",
  "properties": {
    "$schema": {
      "description": "The JSON schema reference.",
      "type": "string"
    },
  },
  additionalProperties: {
   type: "object"
  }
}
compile(mySchema, 'MySchema')
  .then(console.log)
bcherny commented 3 years ago

This is tricky. We want to say "additional properties should be X, but not Y". I worry that unioning with unknown is too permissive.

This is a TypeScript feature request, tracked here: https://github.com/microsoft/TypeScript/issues/17867.

joscha commented 3 years ago

ACK. For the time being the generated types are unusable however, the way they are generated. I think unioning with unknown is not ideal but in the meantime, until there is a better way to to express this, it might be better than what is currently generated?

On Mon, 13 Sep 2021, 07:08 Boris Cherny, @.***> wrote:

This is tricky. We want to say "additional properties should be X, but not Y". I worry that unioning with unknown is too permissive.

This is a TypeScript feature request, tracked here: microsoft/TypeScript#17867 https://github.com/microsoft/TypeScript/issues/17867.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/bcherny/json-schema-to-typescript/issues/402#issuecomment-917710735, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABN5BXXPS6M46C3DCFQDZLUBUJFNANCNFSM5CUPDYYA .

fabrykowski commented 2 years ago

Hi! Any chance of reviewing PR #383 which might close this issue?