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.

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

Schema definitions are generated multiple times when setting a property description #466

Open
queengooborg opened this issue Jul 7, 2022 · 6 comments

Comments

@queengooborg
Copy link
Contributor

queengooborg commented Jul 7, 2022

This change was found somewhere between 10.1.5 and 11.0.1. When generating types, I am noticing that typedefs are generated twice in certain scenarios, particularly when referencing a schema definition.

Schema:

// This schema has been simplified to include only the relevant portions of the schema
{
  "$schema": "http://json-schema.org/schema#",

  "definitions": {
    /* ... */
    "status_block": {
      "type": "object",
      "properties": {
        "experimental": {
          "type": "boolean",
          "description": "A boolean value that indicates whether this functionality is intended to be an addition to the Web platform. Set to false, it means the functionality is mature, and no significant incompatible changes are expected in the future."
        },
        "standard_track": {
          "type": "boolean",
          "description": "A boolean value indicating whether the feature is part of an active specification or specification process."
        },
        "deprecated": {
          "type": "boolean",
          "description": "A boolean value that indicates whether the feature is no longer recommended. It might be removed in the future or might only be kept for compatibility purposes. Avoid using this functionality."
        }
      },
      "required": ["experimental", "standard_track", "deprecated"],
      "additionalProperties": false
    },

    "compat_statement": {
      "type": "object",
      "properties": {
        /* ... */
        "status": {
          "$ref": "#/definitions/status_block",
          "description": "An object containing information about the stability of the feature.",
          "tsType": "StatusBlock"
        }
      },
      /* ... */
    },
}

Before:

export interface CompatStatement {
  /* ... */
  /**
   * An object containing information about the stability of the feature.
   */
  status?: StatusBlock;
}

/**
 * This interface was referenced by `CompatDataFile`'s JSON-Schema
 * via the `definition` "status_block".
 */
export interface StatusBlock {
  /**
   * A boolean value that indicates whether this functionality is intended to be an addition to the Web platform. Set to false, it means the functionality is mature, and no significant incompatible changes are expected in the future.
   */
  experimental: boolean;
  /**
   * A boolean value indicating whether the feature is part of an active specification or specification process.
   */
  standard_track: boolean;
  /**
   * A boolean value that indicates whether the feature is no longer recommended. It might be removed in the future or might only be kept for compatibility purposes. Avoid using this functionality.
   */
  deprecated: boolean;
}

After:

/**
 * An object containing information about the stability of the feature.
 */
export type StatusBlock = StatusBlock;
export interface CompatStatement {
  /* ... */
  status?: StatusBlock;
}
/**
 * This interface was referenced by `CompatDataFile`'s JSON-Schema
 * via the `definition` "status_block".
 */
export interface StatusBlock1 {
  /**
   * A boolean value that indicates whether this functionality is intended to be an addition to the Web platform. Set to false, it means the functionality is mature, and no significant incompatible changes are expected in the future.
   */
  experimental: boolean;
  /**
   * A boolean value indicating whether the feature is part of an active specification or specification process.
   */
  standard_track: boolean;
  /**
   * A boolean value that indicates whether the feature is no longer recommended. It might be removed in the future or might only be kept for compatibility purposes. Avoid using this functionality.
   */
  deprecated: boolean;
}
@bcherny
Copy link
Owner

bcherny commented Jul 11, 2022

Thanks for the report! It's hard to help without a reproducible case. Can you please comment with a self-contained, reproducible example of the bug?

@queengooborg
Copy link
Contributor Author

queengooborg commented Jul 11, 2022

Certainly -- here's the above example put into a runnable script to show the issue:

import { compile } from 'json-schema-to-typescript';

const opts = {
  bannerComment: '',
  unreachableDefinitions: true,
};

const schema = {
  $schema: 'http://json-schema.org/schema#',

  definitions: {
    status_block: {
      type: 'object',
      properties: {
        experimental: {
          type: 'boolean',
          description:
            'A boolean value that indicates whether this functionality is intended to be an addition to the Web platform. Set to false, it means the functionality is mature, and no significant incompatible changes are expected in the future.',
        },
        standard_track: {
          type: 'boolean',
          description:
            'A boolean value indicating whether the feature is part of an active specification or specification process.',
        },
        deprecated: {
          type: 'boolean',
          description:
            'A boolean value that indicates whether the feature is no longer recommended. It might be removed in the future or might only be kept for compatibility purposes. Avoid using this functionality.',
        },
      },
      required: ['experimental', 'standard_track', 'deprecated'],
      additionalProperties: false,
    },

    compat_statement: {
      type: 'object',
      properties: {
        status: {
          $ref: '#/definitions/status_block',
          description:
            'An object containing information about the stability of the feature.',
          tsType: 'StatusBlock',
        },
      },
    },
  },

  title: 'CompatDataFile',
  type: 'object',
  patternProperties: {
    '^__compat$': {
      $ref: '#/definitions/compat_statement',
    },
  },
};

console.log(await compile(schema, 'Schema', opts));

Note: if you comment out the line that says tsType: 'StatusBlock', then export type StatusBlock = StatusBlock; becomes export interface StatusBlock {...}, and the interface is still duplicated.

@queengooborg
Copy link
Contributor Author

Update: it seems that unreachableDefinitions: true causes this issue, as switching it to false will generate the expected typedefs. However, because we define custom tsTypes, disabling the option is not possible for us and removing the tsType is not an option either.

@dhvector
Copy link

dhvector commented Aug 1, 2022

Getting rid of tsType and moving description to the status_block definition fixes this. Would that work for you?

I'm not too familiar with the code, but it seems that when additional fields are present alongside $ref, then the property (e.g. your status property) is promoted as a unique interface as of 2ca6e50, e.g. StatusBlock1. Prior to that commit, it would get inlined instead of generating a new interface, and at that time tsType helped mask the issue.

edit: ^ btw, it appears this is due to @apidevtools/json-schema-ref-parser treating your $ref with additional fields as an "extended ref" and thus creates a new merged object as a shallow copy of the ref'd object plus the extra properties that are siblings to the $ref. Since they aren't referentially equal after running the resolver, json-schema-to-typescript sets different parents during its linking phase, and during code generation as of 2ca6e50 it becomes a new interface.

Outside of the quick fix I mentioned, just thinking out loud, I could envision a scenario where certain keywords, like description, are extracted into a separate tree prior to $ref resolution, and referenced during code generation to, for example, tack comments onto properties.

@queengooborg queengooborg changed the title Schema definitions are generated multiple times Schema definitions are generated multiple times when setting a property description Aug 1, 2022
@queengooborg
Copy link
Contributor Author

Thanks for digging into this! Moving the description to the status_block definition did fix the issue. Unfortunately as a result, we lose the description on the status property itself (and thus lose IDE hints, which contributors do care about), but at least the TypeScript generates without duplicates. I even removed the custom tsType successfully!

I think that the potential solution of extracting properties prior to $ref resolution is a great one for a long-term fix. I can imagine other projects may not want to make this compromise, and it should greatly help reduce confusion during debugging!

@mribichich
Copy link

referencing my comment about this in another PR:

#193 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants