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

Add x-enum-varnames support #405

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ json2ts -i foo.json -o foo.d.ts --style.singleQuote --no-style.semi
## Custom schema properties:

- `tsType`: Overrides the type that's generated from the schema. Useful for forcing a type to `any` or when using non-standard JSON schema extensions ([eg](https://github.com/sokra/json-schema-to-typescript/blob/f1f40307cf5efa328522bb1c9ae0b0d9e5f367aa/test/e2e/customType.ts)).
- `tsEnumNames`: Overrides the names used for the elements in an enum. Can also be used to create string enums ([eg](https://github.com/johnbillion/wp-json-schemas/blob/647440573e4a675f15880c95fcca513fdf7a2077/schemas/properties/post-status-name.json)).
- `'x-enum-varnames'`: Overrides the names used for the elements in an enum. Can also be used to create string enums ([eg](https://github.com/johnbillion/wp-json-schemas/blob/647440573e4a675f15880c95fcca513fdf7a2077/schemas/properties/post-status-name.json)).

## Not expressible in TypeScript:

Expand Down
16 changes: 16 additions & 0 deletions src/normalizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,22 @@ rules.set('Transform const to singleton enum', schema => {
}
})

/**
* `tsEnumNames` is being replaced by `x-enum-varnames`. Since we don't want to
* break existing usage of `tsEnumNames`, we'll convert `tsEnumNames` properties
* to `x-enum-varnames`.
*/
rules.set('Convert tsEnumNames -> x-enum-varnames', schema => {
if (schema.tsEnumNames) {
if (Object.keys(schema).includes('x-enum-varnames')) {
throw new Error('Cannot set both x-enum-varnames and tsEnumNames in the same schema. Only use x-enum-varnames.')
Copy link
Owner

Choose a reason for hiding this comment

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

Mind being a little more lenient, and only throwing if the two don't match? These should also have tests, if possible.

ie.

// ok
{
  tsEnumNames: ['a']
}

// ok
{
  x-enum-varnames: ['a']
}

// ok
{
  tsEnumNames: ['a'],
  x-enum-varnames: ['a']
}

// error
{
  tsEnumNames: ['a'],
  x-enum-varnames: []
}

// error
{
  tsEnumNames: [],
  x-enum-varnames: ['a']
}

// error
{
  tsEnumNames: ['a'],
  x-enum-varnames: ['b']
}

// error
{
  tsEnumNames: ['a'],
  x-enum-varnames: ['a', 'b']
}

}

schema['x-enum-varnames'] = schema.tsEnumNames
delete schema.tsEnumNames
}
})

export function normalize(rootSchema: LinkedJSONSchema, filename: string, options: Options): NormalizedJSONSchema {
rules.forEach(rule => traverse(rootSchema, schema => rule(schema, filename, options)))
return rootSchema as NormalizedJSONSchema
Expand Down
2 changes: 1 addition & 1 deletion src/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ function parseNonLiteral(
standaloneName: standaloneName(schema, keyNameFromDefinition ?? keyName, usedNames)!,
params: schema.enum!.map((_, n) => ({
ast: parse(_, options, undefined, processed, usedNames),
keyName: schema.tsEnumNames![n]
keyName: schema['x-enum-varnames']![n]
})),
type: 'ENUM'
}
Expand Down
14 changes: 13 additions & 1 deletion src/types/JSONSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ export interface JSONSchema extends JSONSchema4 {
* schema extension to support numeric enums
*/
tsEnumNames?: string[]
/**
* schema extension to support numeric enums
*/
'x-enum-varnames'?: string[]
/**
* schema extension to support custom types
*/
Expand Down Expand Up @@ -93,10 +97,18 @@ export interface EnumJSONSchema extends NormalizedJSONSchema {
enum: any[]
}

export interface NamedEnumJSONSchema extends NormalizedJSONSchema {
interface DeprecatedNamedEnumJSONSchema extends NormalizedJSONSchema {
'x-enum-varnames'?: never
tsEnumNames: string[]
Copy link
Owner

Choose a reason for hiding this comment

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

Normalized schemas shouldn't contain tsEnumNames, since you're normalizing it away.

Copy link
Author

@goodoldneon goodoldneon Sep 25, 2021

Choose a reason for hiding this comment

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

So I should delete DeprecatedNamedEnumJSONSchema and rename _NamedEnumJSONSchema to NamedEnumJSONSchema?

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah just add x-enum-varnames: string[] and tsEnumNames: never to NormalizedJSONSchema. No need for new types if you can avoid it.

}

interface _NamedEnumJSONSchema extends NormalizedJSONSchema {
'x-enum-varnames': string[]
tsEnumNames?: never
}

export type NamedEnumJSONSchema = _NamedEnumJSONSchema | DeprecatedNamedEnumJSONSchema

export interface SchemaSchema extends NormalizedJSONSchema {
properties: {
[k: string]: NormalizedJSONSchema
Expand Down
6 changes: 3 additions & 3 deletions src/typesOfSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ const matchers: Record<SchemaType, (schema: JSONSchema) => boolean> = {
return false // Explicitly handled before we try to match
},
NAMED_ENUM(schema) {
return 'enum' in schema && 'tsEnumNames' in schema
return 'enum' in schema && 'x-enum-varnames' in schema
},
NAMED_SCHEMA(schema) {
return 'id' in schema && ('patternProperties' in schema || 'properties' in schema)
return 'id' in schema && 'patternProperties' in schema
},
NULL(schema) {
return schema.type === 'null'
Expand Down Expand Up @@ -122,7 +122,7 @@ const matchers: Record<SchemaType, (schema: JSONSchema) => boolean> = {
return Array.isArray(schema.type)
},
UNNAMED_ENUM(schema) {
if ('tsEnumNames' in schema) {
if ('x-enum-varnames' in schema) {
return false
}
if (
Expand Down
20 changes: 20 additions & 0 deletions src/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,32 @@ import {mapDeep} from './utils'
type Rule = (schema: JSONSchema) => boolean | void
const rules = new Map<string, Rule>()

rules.set('Enum members and x-enum-varnames must be of the same length', schema => {
if (schema.enum && schema['x-enum-varnames'] && schema.enum.length !== schema['x-enum-varnames'].length) {
return false
}
})

/**
* `tsEnumNames` is being replaced by `x-enum-varnames`, but we'll leave this
* validator until `tsEnumNames` is removed.
*/
Copy link
Owner

Choose a reason for hiding this comment

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

Please make sure this is still covered by tests.

rules.set('Enum members and tsEnumNames must be of the same length', schema => {
if (schema.enum && schema.tsEnumNames && schema.enum.length !== schema.tsEnumNames.length) {
return false
}
})

rules.set('x-enum-varnames must be an array of strings', schema => {
if (schema['x-enum-varnames'] && schema['x-enum-varnames'].some(_ => typeof _ !== 'string')) {
return false
}
})

/**
* `tsEnumNames` is being replaced by `x-enum-varnames`, but we'll leave this
* validator until `tsEnumNames` is removed.
*/
Copy link
Owner

Choose a reason for hiding this comment

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

Please make sure this is still covered by tests.

Copy link
Author

Choose a reason for hiding this comment

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

Should I copy enumValidation.1.ts and enumValidation.2.ts, but sed "s/tsEnumNames/x-enum-varnames/" in the new test files?

Copy link
Owner

Choose a reason for hiding this comment

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

Sounds ok to me, or merge into a single test file that covers all the cases (x-, ts-, x- and ts-, etc.).

rules.set('tsEnumNames must be an array of strings', schema => {
if (schema.tsEnumNames && schema.tsEnumNames.some(_ => typeof _ !== 'string')) {
return false
Expand Down
Loading