Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Adding a description to a $ref object referencing an enum inlines the enum #472

Open
JoshuaKGoldberg opened this issue Jul 29, 2022 · 3 comments · May be fixed by #521
Open

Adding a description to a $ref object referencing an enum inlines the enum #472

JoshuaKGoldberg opened this issue Jul 29, 2022 · 3 comments · May be fixed by #521

Comments

@JoshuaKGoldberg
Copy link
Contributor

Perhaps related to #470, but not the same issue. Here, I'm seeing that when a $ref pointing to a defined enum has a description, the enum gets inlined.

{
  "$definitions": {
    "shared": {
      "enum": ["a", "b"]
    }
  },
  "properties": {
    "first": {
      "$ref": "#/$definitions/shared",
      "description": "A first property."
    }
  },
  "additionalProperties": false,
  "title": "Example Schema",
  "type": "object"
}

Note that if the description is removed from properties.first, the export type Shared = "a" | "b"; gets printed as expected.

Actual

export interface ExampleSchema {
  /**
   * A first property.
   */
  first?: "a" | "b";
}

Expected

export type Shared = "a" | "b";

export interface ExampleSchema {
  /**
   * A first property.
   */
  first?: Shared;
}
@JoshuaKGoldberg JoshuaKGoldberg mentioned this issue Jul 29, 2022
Closed
@mribichich
Copy link

Referencing other PRs that could be related to this, at least from the description perspective:

#193
#466

@bradzacher
Copy link
Contributor

Doing some deeper investigation into this - the reason that this happens is due to how the json-schema-ref-parser lib works.

json-schema-to-typescript uses json-schema-ref-parser (specifically boris's fork) and the library essentially scans the schema looking for $ref's and it replaces the $ref with the referenced schema.

It does this by merging the referenced schema into the current schema. I.e. newSchema.properties.first === { ...oldSchema.$defs.shared, ...oldSchema.properties.first }
However - if the library sees { $ref: '#/path/to/ref' } it has a short-cirtuit path which lets it return the referenced schema directly.
Above logic is here: https://github.com/bcherny/json-schema-ref-parser/blob/3241ca426d7364455ca8d85f231d51211d7ab7fe/lib/ref.js#L271-L292

When the parser attempts to look to see if the referenced schema has a definition (so that it can get the key and thus the name), it just does an === check to see if the current schema is a definition schema:

json-schema-to-typescript/src/parser.ts

Line 123 in 6e3fbca

const keyNameFromDefinition = findKey(definitions, _ => _ === schema)

So obviously if you have a simple just $ref object, this check will find the key, but as soon as you add any other prop, the check fails to find the key because it's now a new object.

@JoshuaKGoldberg's fix (#473) works around this specifically for enum types by doing a lookup to see if the .enum properties are equal. It's a hack that works because (as above) it's a shallow spread copy.

What's the correct, general solution for this? Well we probably need to keep a property which tracks the ref path for a given schema so that we can resolve it instead of looking up the key.

@bradzacher bradzacher linked a pull request Apr 12, 2023 that will close this issue
Draft
@bradzacher
Copy link
Contributor

I tried solving this (#521) but I ran into a problem in that the description comment gets put in the wrong spot (on the referenced schema, not on the referencing thing).

The codebase is pretty complex so I'm not sure how to work through this more right now, but at least I've prototyped a generic solution for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@JoshuaKGoldberg @mribichich @bradzacher