bcherny / json-schema-to-typescript

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

discrepancy between an internal and external references resolution (`$ref`) #143

Open ducin opened 6 years ago

ducin commented 6 years ago

Hi @bcherny,

Once again, thanks for your great work!

I'm having a strange situation, might be considered as a bug, or a feature... depends on your interpretation. For me it's kinda inconsistent.

I have a scenario.schema.json file where I define an entity, it has internal sub-definitions (used for money, enums, etc.). It looks like the following:

{
  "title": "Scenario",
  "type": "object",
  "properties": {
    "id": {
      "type": "string",
      "faker": "random.uuid"
    },
    "name": {
      "type": "string",
      "faker": {
        "fake": "{{company.bsNoun}} improvement"
      }
    },
    "description": {
      "type": "string",
      "faker": "lorem.sentences"
    },
    "sharing": {
      "$ref": "#/definitions/ScenarioSharing"
    },
    "currentVersion": {
      "type": "string",
      "pattern": "v0\\.\\d"
    },
    "lastRun": {
      "type": "string",
      "faker": "date.past"
    },
    "versions": {
      "type": "array",
      "items": {
        "$ref": "#/definitions/VersionDefinition"
      }
    },
    "summary": {
      "$ref": "#/definitions/ScenarioSummary"
    }
  },
  "additionalProperties": false,
  "required": [
    "id",
    "name",
    "description",
    "sharing",
    "currentVersion",
    "lastRun",
    "versions",
    "summary"
  ],
  "definitions": {
    "ScenarioSharing": {
      "enum": [
        "PUBLIC",
        "PRIVATE"
      ]
    },
    "Currency": {
      "enum": [
        "USD",
        "EUR",
        "CHF",
        "GBP"
      ]
    },
    "VersionDefinition": {
      "type": "object",
      "properties": {
        "version": {
          "type": "string",
          "pattern": "v0\\.\\d"
        },
        "date": {
          "type": "string",
          "faker": {
            "date.between": ["2018-01-01", "2018-12-31"]
          }
        },
        "editor": {
          "type": "string",
          "faker": "name.findName"
        },
        "notes": {
          "type": "string",
          "faker": "lorem.sentence"
        }
      },
      "additionalProperties": false,
      "required": [
        "version",
        "date",
        "editor",
        "notes"
      ]
    },
    "ScenarioSummary": {
      "type": "object",
      "properties": {
        "startDate": {
          "type": "string",
          "faker": {
            "date.between": ["2019-01-01", "2019-12-31"]
          }
        },
        "endDate": {
          "type": "string",
          "faker": {
            "date.between": ["2020-01-01", "2020-12-31"]
          }
        },
        "projectCost": {
          "$ref": "#/definitions/Money"
        },
        "BAUCost": {
          "$ref": "#/definitions/Money"
        }
      },
      "additionalProperties": false,
      "required": [
        "startDate",
        "endDate",
        "projectCost",
        "BAUCost"
      ]
    },
    "Money": {
      "type": "object",
      "properties": {
        "amount": {
          "type": "number",
          "faker": {
            "random.number": {
              "min": 100000,
              "max": 10000000,
              "precision": 10000
            }
          }
        },
        "currency": {
          "$ref": "#/definitions/Currency"
        }
      },
      "additionalProperties": false,
      "required": [
        "amount",
        "currency"
      ]
    }
  }
}

I have another file which is just a collection of these entities:

{
  "title": "Scenarios",
  "type": "array",
  "items": {
    "$ref": "api/modelling/scenario.schema.json"
  }
}

. I use both files in a RAML contract (generating html docs with raml2html and .d.ts files with your lib here). They need separate schema files, since GET /scenarios is different than GET /scenarios/{id}.

For a single scenario schema I get following:

// Auto-generated with `raml-to-typescript`/`json-schema-to-typescript`

export type ScenarioSharing = "PUBLIC" | "PRIVATE";
export type Currency = "USD" | "EUR" | "CHF" | "GBP";

export interface Scenario {
  id: string;
  name: string;
  description: string;
  sharing: ScenarioSharing;
  currentVersion: string;
  lastRun: string;
  versions: VersionDefinition[];
  summary: ScenarioSummary;
}
export interface VersionDefinition {
  version: string;
  date: string;
  editor: string;
  notes: string;
}
export interface ScenarioSummary {
  startDate: string;
  endDate: string;
  projectCost: Money;
  BAUCost: Money;
}
export interface Money {
  amount: number;
  currency: Currency;
}

whereas for array with external reference, I get:

// Auto-generated with `raml-to-typescript`/`json-schema-to-typescript`

export type Scenarios = Scenario[];

export interface Scenario {
  id: string;
  name: string;
  description: string;
  sharing: "PUBLIC" | "PRIVATE";
  currentVersion: string;
  lastRun: string;
  versions: {
    version: string;
    date: string;
    editor: string;
    notes: string;
  }[];
  summary: {
    startDate: string;
    endDate: string;
    projectCost: {
      amount: number;
      currency: "USD" | "EUR" | "CHF" | "GBP";
    };
    BAUCost: {
      amount: number;
      currency: "USD" | "EUR" | "CHF" | "GBP";
    };
  };
}

Semantically (from TS point of view), the structures are the same. However, internal references get normalized and "compiled into" the external file. I think it would be better if you normalized schemas after all $ref get resolved (no intermediate resolutions along the way).

What is your opinion on this?

bcherny commented 6 years ago

Hi @ducin! This does look like a bug to me. I won't have time to look at it in the near future, but am happy to look over any PRs you send over.

dhermes commented 5 years ago

I also ran into this. @bcherny do you think you might point to specific parts of the code where $ref get resolved?

nkappler commented 4 years ago

I have the same problem untfortunately. I would be willing to dig into this, however a starting point would be really nice to have

nkappler commented 4 years ago

It looks like the issue is that the dereference function from json-schema-ref-parser doesn't supply any information about the original definitions. As a result, the standaloneName property on the AST is undefined for each interface (or arg or ast-param or however you want to call it). Finally, they don't get declared as interface in the resulting declaration file because the name is lost. If I supply a standaloneName manually during debugging, it works fine...

nkappler commented 4 years ago

I previously worked around this by re-declaring the definitions in the extended schema and adding a reference for each interface.

I now found a better workaround which is to set the id property on each definition you want to generate an interface for. Whatever you have assigned to id will be the name of the interface.

⚠️ Note don't confuse this with the $id property which can be used for references

In the above example this would look like this:

  ...
 "Money": {
      "id": "Money",
      "type": "object",
      "properties": {
        "amount": {
        ....
    },
  ...
nkappler commented 4 years ago

This also fixes an issue where some interfaces would disappear when adding a comment to them 😅

nkappler commented 4 years ago

I prepared a fix (see links above). However, it depends on the pull request from json-schema-ref-parser (also linked above). Until the PR is submitted and a new version is released, I won't create a pull request for this fix, since it would have no effect.