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

Allow specification of a custom interface name #456

Closed
wants to merge 3 commits into from

Conversation

queengooborg
Copy link
Contributor

This PR adds a new tsName schema property that allows a developer to overwrite the default generated name. This is super helpful in cases where two interfaces/types are generated with the same name (and one has a numerical increment), or where the developer may wish to ensure backwards compatibility of their TypeScript definitions.

Before:

/**
 * This interface was referenced by `Browsers`'s JSON-Schema
 * via the `definition` "browsers".
 */
export type Browsers1 = Record<string, BrowserStatement>;

export interface Browsers {
  browsers?: Browsers1;
}

After:

/**
 * This interface was referenced by `Browsers`'s JSON-Schema
 * via the `definition` "browsers".
 */
export type Browsers = Record<string, BrowserStatement>;

export interface BrowsersData {
  browsers?: Browsers;
}

This is related to #181 (and potentially resolves it?). Unlike #215, however, this PR aims to grant the developer more finite control over the names.

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.

Seems reasonable! A few minor suggestions before we merge this.

test/e2e/customName.ts Outdated Show resolved Hide resolved
test/e2e/customName.ts Outdated Show resolved Hide resolved
grocery: {$ref: '#/definitions/grocery'}
},
additionalProperties: false,
tsName: 'GroceryList'
Copy link
Owner

Choose a reason for hiding this comment

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

Please also add a test case for when title is defined, and another for when description is defined. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't realize that title was an actual schema property! In that case, the title property solves the problem for our use case, but I'll still update the tests and see this PR through to the end!

Copy link
Owner

Choose a reason for hiding this comment

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

If title works for your use case, we can also pause the work on this PR. No need to add more options unless there’s strong demand for them.


type: 'object',
properties: {
grocery: {$ref: '#/definitions/grocery'}
Copy link
Owner

Choose a reason for hiding this comment

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

Can you just add a single inline boolean property? This test case is about naming, so we can simplify it to remove the definitions.

README.md Outdated Show resolved Hide resolved
@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
@queengooborg queengooborg deleted the custom-name branch May 4, 2023 20:01
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.

None yet

2 participants