`$ref` in `allOf` with depth of 2 is not correctly converted when multi-referenced #597

arnauddrain · 2024-05-29T17:21:31Z

I'm trying to reproduce an inheritance schema like this:

Thing
 |
Vehicle
 |     \ 
Car     Truck

Using the "allOf" keyword with a schema like this:

{
  "$schema": "http://json-schema.org/draft-07/schema#",
  "$id": "test",
  "definitions": {
    "Thing": {
      "type": "object",
      "properties": {
        "name": { "type": "string" }
      },
      "required": ["name"]
    },
    "Vehicle": {
      "type": "object",
      "allOf": [{ "$ref": "#/definitions/Thing" }],
      "properties": {
        "year": { "type": "integer" }
      },
      "required": ["year"]
    },
    "Car": {
      "type": "object",
      "allOf": [{ "$ref": "#/definitions/Vehicle" }],
      "properties": {
        "numDoors": { "type": "integer" }
      },
      "required": ["numDoors"]
    },
    "Truck": {
      "type": "object",
      "allOf": [{ "$ref": "#/definitions/Vehicle" }],
      "properties": {
        "numAxles": { "type": "integer" }
      },
      "required": ["numAxles"]
    }
  },
  "oneOf": [{ "$ref": "#/definitions/Car" }, { "$ref": "#/definitions/Truck" }]
}

The output looks like this:

export type Test = Car | Truck;
export type Car = Car1 & {
  numDoors: number;
};
export type Car1 = Vehicle; // <--- This is expected
export type Vehicle = Vehicle1 & {
  year: number;
};
export type Vehicle1 = Thing;
export type Truck = Truck1 & {
  numAxles: number;
};
export type Truck1 = Vehicle1; // <--- This is unexpected, should be Vehicle
  name: string;
}

Here, Truck is directly inheriting Thing instead of Vehicle, so its TypeScript definition does not includes the year property.
If I exchange the properties of the root oneOf, then the the issue appears in the type definition of Car.

The text was updated successfully, but these errors were encountered:

bcherny · 2024-06-02T12:13:47Z

This does seem like a bug.

In your case, I think you actually want extends, which would fix it for you:

arnauddrain · 2024-06-02T12:18:58Z

Thank you for your answer @bcherny !
extends will indeed generate the correct typescript classes, but then the validation will be incorrect, because the properties of the "parent" objets won't be required anymore and {"numDoors": 2} will become a valid input: https://jsonschema.dev/s/TdKME

bcherny · 2024-06-07T20:18:45Z

@arnauddrain extends is an old keyword that has been removed in modern JSON Schema versions. The website you linked doesn't seem to implement it correctly. If your validator supports it, extends will work fine the way I suggested.

See the spec: https://github.com/json-schema/json-schema/wiki/Extends/014e3cd8692250baad70c361dd81f6119ad0f696

Description - Given a schema that contains a named definition (`Level2B`), - And that named definition is referenced in multiple locations, - And that named schema is also an intersection type (`allOf` in this example), Then when parsed, the generated TypeScript will contain the correct reference only for the _first_ location in which the named schema is encountered, during a depth-first traversal. Subsequent references to the same schema will be generated as though they were only the intersection type, and not the named schema. Example Given the following schema: ```yaml description: Top Level type: object oneOf: - $ref: '#/definitions/Level2A' - $ref: '#/definitions/Level2B' definitions: Level2A: description: Level2A type: object allOf: [$ref: '#/definitions/Base'] properties: level_2A_ref: { $ref: '#/definitions/Level2B' } Level2B: description: Level2B type: object allOf: [$ref: '#/definitions/Base'] properties: level_2B_prop: { const: xyzzy } Base: description: Base type: object properties: base_prop: { type: string } ``` The current resulting TypeScript will be (comments adjusted for clarity): ```ts // Incorrect: should be `type Demo = Level2A | Level2B` // Note that the Level2B type at this location is the _second_ instance // to Level2B during a depth-first traversal. export type Demo = Level2A | Level2B1; export type Level2A = Level2A1 & { // Correct in this location because this property is reached first in // a depth-first traversal. level_2A_ref?: Level2B; [k: string]: unknown; }; export type Level2A1 = Base; export type Level2B = Level2B1 & { level_2B_prop?: "xyzzy"; [k: string]: unknown; }; export type Level2B1 = Base; export interface Base { base_prop?: string; [k: string]: unknown; } ``` Root Cause In `parser.ts`, [lines 57 - 75][1], when schema that matches multiple "types" is encountered, the parser generates a new `ALL_OF` intersection schema to contain each sub-type, then adds each sub-type to the new `ALL_OF` schema. Each sub-type is then parsed sequentially. During this process, `maybeStripNameHints` is called, which mutates the schema by removing the `$id`, `description`, and `name` properties. Notably, these properties are used by `typesOfSchema` to detect the `NAMED_SCHEMA` type. As a result, this schema object will never again be detected as a `NAMED_SCHEMA` type. Therefore, the _first_ instance of the schema object is correctly handled as an intersection schema **and** a named schema, but all subsequent instances are treated as though they are **only** an intersection schema. Proposed Solution - The call to `typesOfSchema` is moved from `parser.ts` to `normalizer.ts`, with the goal of avoiding confusion due to a mutated schema object. The resulting list of schema types is persisted as a `$types` property on the schema. - The generated intersection schema is _also_ moved from `parser.ts` to `normalizer.ts`. This is because it is advantageous to let the generated intersection schema participate in the caching mechanism (which it could not previously do, since it was generated dynamically during each encounter). Without this, multiple instances of the same schema are generated. Related Issues - bcherny#597 [1]: https://github.com/bcherny/json-schema-to-typescript/blob/31993def993b610ba238d3024260129e31ddc371/src/parser.ts#L57-L75 'parser.ts, lines 57 - 75'

- Given a schema that contains a named definition (`B`), - And that named definition is referenced in multiple locations, - And that named schema is also an intersection type (`allOf` in this example), Then when parsed, the generated TypeScript will contain the correct reference only for the _first_ location in which the named schema is encountered, during a depth-first traversal. Subsequent references to the same schema will be generated as though they were only the intersection type, and not the named schema. Example Given the following schema: ```yaml $id: Intersection type: object oneOf: - $ref: '#/definitions/A' - $ref: '#/definitions/B' definitions: A: type: object additionalProperties: false allOf: [$ref: '#/definitions/Base'] properties: b: {$ref: '#/definitions/B'} B: type: object additionalProperties: false, allOf: [$ref: '#/definitions/Base'] properties: x: {type: string} Base: type: object additionalProperties: false, properties: y: {type: string} ``` The current resulting TypeScript will be (comments adjusted for clarity): ```ts // Incorrect: should be `type Intersection = A | B` // Note that the B type at this location is the _second_ reference to // B during a depth-first traversal. export type Intersection = A | B1; export type A = A1 & { b?: B; }; export type A1 = Base; export type B = B1 & { x?: string; }; export type B1 = Base; export interface Base { y?: string; } ``` Root Cause In `parser.ts`, [lines 57 - 75][1], when schema that matches multiple "types" is encountered, the parser generates a new `ALL_OF` intersection schema to contain each sub-type, then adds each sub-type to the new `ALL_OF` schema. Each sub-type is then parsed sequentially. During this process, `maybeStripNameHints` is called, which mutates the schema by removing the `$id`, `description`, and `name` properties. Notably, these properties are used by `typesOfSchema` to detect the `NAMED_SCHEMA` type. As a result, this schema object will never again be detected as a `NAMED_SCHEMA` type. Therefore, the _first_ instance of the schema object is correctly handled as an intersection schema **and** a named schema, but all subsequent instances are treated as though they are **only** an intersection schema. Proposed Solution - The call to `typesOfSchema` is moved from `parser.ts` to `normalizer.ts`, with the goal of avoiding confusion due to a mutated schema object. The resulting list of schema types is persisted on the schema using a newly-introduced `Types` symbol. - The generated intersection schema is _also_ moved from `parser.ts` to `normalizer.ts`. This is because it is advantageous to let the generated intersection schema participate in the caching mechanism (which it could not previously do, since it was generated dynamically during each encounter). Without this, multiple instances of the same schema are generated. Related Issues - bcherny#597 [1]: https://github.com/bcherny/json-schema-to-typescript/blob/31993def993b610ba238d3024260129e31ddc371/src/parser.ts#L57-L75 'parser.ts, lines 57 - 75'

* Add new test cases. * Intersection schema generation is order-dependent - Given a schema that contains a named definition (`B`), - And that named definition is referenced in multiple locations, - And that named schema is also an intersection type (`allOf` in this example), Then when parsed, the generated TypeScript will contain the correct reference only for the _first_ location in which the named schema is encountered, during a depth-first traversal. Subsequent references to the same schema will be generated as though they were only the intersection type, and not the named schema. Example Given the following schema: ```yaml $id: Intersection type: object oneOf: - $ref: '#/definitions/A' - $ref: '#/definitions/B' definitions: A: type: object additionalProperties: false allOf: [$ref: '#/definitions/Base'] properties: b: {$ref: '#/definitions/B'} B: type: object additionalProperties: false, allOf: [$ref: '#/definitions/Base'] properties: x: {type: string} Base: type: object additionalProperties: false, properties: y: {type: string} ``` The current resulting TypeScript will be (comments adjusted for clarity): ```ts // Incorrect: should be `type Intersection = A | B` // Note that the B type at this location is the _second_ reference to // B during a depth-first traversal. export type Intersection = A | B1; export type A = A1 & { b?: B; }; export type A1 = Base; export type B = B1 & { x?: string; }; export type B1 = Base; export interface Base { y?: string; } ``` Root Cause In `parser.ts`, [lines 57 - 75][1], when schema that matches multiple "types" is encountered, the parser generates a new `ALL_OF` intersection schema to contain each sub-type, then adds each sub-type to the new `ALL_OF` schema. Each sub-type is then parsed sequentially. During this process, `maybeStripNameHints` is called, which mutates the schema by removing the `$id`, `description`, and `name` properties. Notably, these properties are used by `typesOfSchema` to detect the `NAMED_SCHEMA` type. As a result, this schema object will never again be detected as a `NAMED_SCHEMA` type. Therefore, the _first_ instance of the schema object is correctly handled as an intersection schema **and** a named schema, but all subsequent instances are treated as though they are **only** an intersection schema. Proposed Solution - The call to `typesOfSchema` is moved from `parser.ts` to `normalizer.ts`, with the goal of avoiding confusion due to a mutated schema object. The resulting list of schema types is persisted on the schema using a newly-introduced `Types` symbol. - The generated intersection schema is _also_ moved from `parser.ts` to `normalizer.ts`. This is because it is advantageous to let the generated intersection schema participate in the caching mechanism (which it could not previously do, since it was generated dynamically during each encounter). Without this, multiple instances of the same schema are generated. Related Issues - #597 [1]: https://github.com/bcherny/json-schema-to-typescript/blob/31993def993b610ba238d3024260129e31ddc371/src/parser.ts#L57-L75 'parser.ts, lines 57 - 75' * Additionally hoist `allOf` behavior. * Traverse the generated intersection schema.

bcherny · 2024-07-22T10:32:53Z

Merged #603

bcherny · 2024-07-22T10:36:17Z

Fix included in v15.0.0.

arnauddrain changed the title ~~AllOf with depth of 2 is not corrently converted when multi-referenced~~ AllOf with depth of 2 is not correctly converted when multi-referenced May 29, 2024

arnauddrain changed the title ~~AllOf with depth of 2 is not correctly converted when multi-referenced~~ $ref in allOf with depth of 2 is not correctly converted when multi-referenced May 30, 2024

bcherny added bug help wanted labels Jun 2, 2024

This was referenced Jun 23, 2024

Generation for intersection/named schemas is order-dependent #602

Closed

Intersection schema generation is order-dependent #603

Merged

bcherny closed this as completed Jul 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`$ref` in `allOf` with depth of 2 is not correctly converted when multi-referenced #597

`$ref` in `allOf` with depth of 2 is not correctly converted when multi-referenced #597

arnauddrain commented May 29, 2024

bcherny commented Jun 2, 2024

arnauddrain commented Jun 2, 2024

bcherny commented Jun 7, 2024

bcherny commented Jul 22, 2024

bcherny commented Jul 22, 2024

$ref in allOf with depth of 2 is not correctly converted when multi-referenced #597

$ref in allOf with depth of 2 is not correctly converted when multi-referenced #597

Comments

arnauddrain commented May 29, 2024

bcherny commented Jun 2, 2024

arnauddrain commented Jun 2, 2024

bcherny commented Jun 7, 2024

bcherny commented Jul 22, 2024

bcherny commented Jul 22, 2024

`$ref` in `allOf` with depth of 2 is not correctly converted when multi-referenced #597

`$ref` in `allOf` with depth of 2 is not correctly converted when multi-referenced #597