-
Notifications
You must be signed in to change notification settings - Fork 394
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Intersection schema generation is order-dependent #603
Merged
Commits on Jun 24, 2024
-
Configuration menu - View commit details
-
Copy full SHA for fb9fc26 - Browse repository at this point
Copy the full SHA fb9fc26View commit details -
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 - bcherny#597 [1]: https://github.com/bcherny/json-schema-to-typescript/blob/31993def993b610ba238d3024260129e31ddc371/src/parser.ts#L57-L75 'parser.ts, lines 57 - 75'
Configuration menu - View commit details
-
Copy full SHA for f604d66 - Browse repository at this point
Copy the full SHA f604d66View commit details -
Configuration menu - View commit details
-
Copy full SHA for 7d3f495 - Browse repository at this point
Copy the full SHA 7d3f495View commit details
Commits on Jul 2, 2024
-
Configuration menu - View commit details
-
Copy full SHA for 6abd384 - Browse repository at this point
Copy the full SHA 6abd384View commit details -
Configuration menu - View commit details
-
Copy full SHA for ae98923 - Browse repository at this point
Copy the full SHA ae98923View commit details
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.