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

fix: generate same title one types #516

Closed
wants to merge 1 commit into from

Conversation

ianfuin
Copy link

@ianfuin ianfuin commented Feb 24, 2023

If two schemas are (a) identical and (b) have the same explicit title, then emit just one type to represent the schema

fix #510

If two schemas are (a) identical and (b) have the same explicit title, then emit just one type to
represent the schema

fix bcherny#510
@ianfuin ianfuin marked this pull request as draft February 24, 2023 02:33
@ianfuin ianfuin marked this pull request as ready for review February 24, 2023 03:21
@jemand771
Copy link

hi @ianfuin and thanks for giving this a shot :D

this mostly works for me, but I think I've found a small regression:

{
    "anyOf": [
        {
            "title": "host",
            "enum": [
                "example.com",
                "localhost"
            ]
        },
        {
            "title": "ip",
            "enum": [
                "123.123.123.123"
            ]
        }
    ],
    "type": "string",
    "title": "server"
}

would previously turn into

export type Server = Server1 & Server2;
export type Server1 = Host | Ip;
export type Host = "example.com" | "localhost";
export type Ip = "123.123.123.123";
export type Server2 = string;

but is now

export type Server = Server;

all while calling compile with the following options:

compile(
        schema,
        undefined,
        {
            bannerComment: "",
            additionalProperties: false,
            strictIndexSignatures: true,
            sameExplicitTitle: sameExplicitTitle,
        }
    );

The example above is inspired by an anyOf schema with {"format": "ipv4"} and {"format": "hostname"}, which this lib doesn't seem to support - the enum keys are just there to make it at least a tiny bit realistic, but leaving them out gives a similar result.

I can "solve" this by removing either type or title from the outmost object. I can understand removing the title as it's redundant, but the type should just work like that1

Footnotes

  1. https://json-schema.org/understanding-json-schema/reference/combining.html#id11

@jemand771
Copy link

hm, I can also trigger the type Foo = Foo; case using just

import { compile } from 'json-schema-to-typescript/dist/index.js';

console.log(await compile(
    {
        title: "host",
        tsType: "Host"
    },
    undefined,
    {
        bannerComment: ""
    }
));

which yields

export type Host = Host;

BUT that also happens without sameExplicitTitle set. maybe I'm just using tsType wrong, but it still feels related.
possibly also related to #466 (export type StatusBlock = StatusBlock; there)

Copy link
Owner

@bcherny bcherny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be misreading your code, but where do you check that two schemas are identical? With your code, it seems like if two schemas share the same name they will share the same emitted type, even if their properties are completely different.

This also needs more tests, and it should be the default behavior (not behind a flag).

@bcherny
Copy link
Owner

bcherny commented May 4, 2023

Closing out stale PRs. Feel free to rebase and re-open if you'd like to revisit this PR.

@bcherny bcherny closed this May 4, 2023
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.

Identical properties with same title generate multiple types
3 participants