Skip to content

Conversation

@bcherny
Copy link
Owner

@bcherny bcherny commented May 22, 2022

WARNING: This is a breaking change, that changes names for some emitted types. It will require a major version bump.

Context

The problem: when emitting TS, we can run into a cycle when the emitted AST:

  1. References a schema
  2. That schema does not have a name
  3. That schema references something (itself, or another schema) that eventually references that same schema

The approach in this PR

This PR has 2 parts:

  1. An upstream PR to json-schema-ref-parser (forked for now, will upstream later) to add an onDereference hook that we can use to grab the original $ref path for dereferenced schemas.
  2. This PR, which takes advantage of original $refs to more correctly name emitted types.

(2) is something I've wanted to do for a while. It gives us more correct behavior (which we only approximated before), and more fully deals with cyclical references.

Alternative solutions considered

  • Always generate standalone names for interfaces & arrays. This fully avoids cycles, but loses the elegance of emitting inline objects for simple unnamed schemas.
  • When we detect a cycle, look behind to add a standalone name to the closest thing that needs it
  • When we generate a type, look ahead to see if there's a cycle, and if so, generate a standalone name for the given type

@bcherny bcherny changed the title [WIP] Fix cycles during emission (fix #376, fix #323) [Work in progress] Fix cycles during emission (fix #376, fix #323) Jun 11, 2022
@bcherny bcherny force-pushed the 376 branch 3 times, most recently from ec7ff44 to 1f7347b Compare June 27, 2022 08:43
@bcherny bcherny changed the title [Work in progress] Fix cycles during emission (fix #376, fix #323) Fix cycles during emission (fix #376, fix #323) Jun 29, 2022
@bcherny bcherny merged commit c6676b6 into master Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants