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

Support nullable #535

Closed
wants to merge 9 commits into from
31 changes: 30 additions & 1 deletion src/normalizer.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {JSONSchemaTypeName, LinkedJSONSchema, NormalizedJSONSchema, Parent} from './types/JSONSchema'
import {appendToDescription, escapeBlockComment, isSchemaLike, justName, toSafeString, traverse} from './utils'
import {appendToDescription, escapeBlockComment, isSchemaLike, justName, toSafeString, traverse, warning} from './utils'
import {Options} from './'
import {DereferencedPaths} from './resolver'
import {isDeepStrictEqual} from 'util'
Expand Down Expand Up @@ -215,6 +215,35 @@ rules.set('Transform definitions to $defs', (schema, fileName) => {
}
})

rules.set('Transform nullable to null type', schema => {
if (schema.nullable !== true) {
return
}

delete schema.nullable
mdmower marked this conversation as resolved.
Show resolved Hide resolved

if (schema.const !== undefined) {
if (schema.const !== null) {
warning('normalizer', 'const should be set to null when schema is nullable', schema)

Choose a reason for hiding this comment

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

I understand why you added warning as its a good addition, but unless I'm reading the utils incorrectly it will always log if this conditional is hit. I don't see any other logs in this file, just errors being thrown.

The only location error is imported is in the cli.ts file and that means the programmatic version will always log if this is hit, when you are actually under the hood fixing the issue vs. logging a properly formated error during a cli command.

(see my comment on the recommended change in utils.tsa).

schema.enum = [schema.const, null]
delete schema.const
}
} else if (schema.enum) {
if (!schema.enum.includes(null)) {
warning('normalizer', 'enum should include null when schema is nullable', schema)
schema.enum.push(null)
}
} else if (schema.type) {
if (Array.isArray(schema.type)) {
if (!schema.type.includes('null')) {
schema.type.push('null')
}
} else if (schema.type !== 'null') {
schema.type = [schema.type, 'null']
}
}
})

rules.set('Transform const to singleton enum', schema => {
if (schema.const !== undefined) {
schema.enum = [schema.const]
Expand Down
7 changes: 7 additions & 0 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,13 @@ export function error(...messages: any[]): void {
console.error(getStyledTextForLogging('red')?.('error'), ...messages)
}

export function warning(...messages: any[]): void {
if (!process.env.VERBOSE) {
return console.warn(messages)
}
console.warn(getStyledTextForLogging('yellow')?.('warning'), ...messages)
Comment on lines +213 to +216

Choose a reason for hiding this comment

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

This will always log. If you look at the log function below it actually returns void when its NOT verbose.

Since this is just a warning I recommend the following change:

Suggested change
if (!process.env.VERBOSE) {
return console.warn(messages)
}
console.warn(getStyledTextForLogging('yellow')?.('warning'), ...messages)
if (!process.env.VERBOSE) {
return
}
console.warn(getStyledTextForLogging('yellow')?.('warning'), ...messages)

Copy link
Author

Choose a reason for hiding this comment

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

This was written to mimic

export function error(...messages: any[]): void {
if (!process.env.VERBOSE) {
return console.error(messages)
}
console.error(getStyledTextForLogging('red')?.('error'), ...messages)
}

I don't pretend to understand that choice and it's not my aim to question it. I'm just following precedence here.

Choose a reason for hiding this comment

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

I figured, but I believe that is only done in the cli not meant for run time usage as I don't really want console.error or console.warn being called when I use the library programatically.

}

type LogStyle = 'blue' | 'cyan' | 'green' | 'magenta' | 'red' | 'white' | 'yellow'

export function log(style: LogStyle, title: string, ...messages: unknown[]): void {
Expand Down
27 changes: 27 additions & 0 deletions test/__snapshots__/test/test.ts.md
Original file line number Diff line number Diff line change
Expand Up @@ -1883,6 +1883,33 @@ Generated by [AVA](https://avajs.dev).
}␊
`

## nullable.js

> Expected output to match snapshot for e2e test: nullable.js

`/* eslint-disable */␊
/**␊
* This file was automatically generated by json-schema-to-typescript.␊
* DO NOT MODIFY IT BY HAND. Instead, modify the source JSONSchema file,␊
* and run json-schema-to-typescript to regenerate this file.␊
*/␊
export interface Nullable {␊
a?: "" | null;␊
b?: null;␊
c?: "a" | "b" | null;␊
d?: "" | null;␊
e?: string | null;␊
f?: null;␊
g?: "" | null;␊
h?: null;␊
i?: "a" | "b" | null;␊
j?: "" | null;␊
k?: string | number | null;␊
l?: string | null;␊
}␊
`

## oneOf.js

> Expected output to match snapshot for e2e test: oneOf.js
Expand Down
Binary file modified test/__snapshots__/test/test.ts.snap
Binary file not shown.
58 changes: 58 additions & 0 deletions test/e2e/nullable.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
export const input = {
type: 'object',
properties: {
a: {
const: '',
nullable: true,
},
b: {
const: null,
nullable: true,
},
c: {
enum: ['a', 'b'],
nullable: true,
},
d: {
enum: ['', null],
nullable: true,
},
e: {
type: 'string',
nullable: true,
},
f: {
type: 'null',
nullable: true,
},
g: {
type: 'string',
const: '',
nullable: true,
},
h: {
type: 'string',
const: null,
nullable: true,
},
i: {
type: 'string',
enum: ['a', 'b'],
nullable: true,
},
j: {
type: 'string',
enum: ['', null],
nullable: true,
},
k: {
type: ['string', 'integer'],
nullable: true,
},
l: {
type: ['string', 'null'],
nullable: true,
},
},
additionalProperties: false,
}
111 changes: 111 additions & 0 deletions test/normalizer/nullableAddsNullToType.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
{
"name": "Nullable adds null to type",
"in": {
"$id": "a",
"type": "object",
"properties": {
"a": {
"const": "",
"nullable": true
},
"b": {
"const": null,
"nullable": true
},
"c": {
"enum": ["a", "b"],
"nullable": true
},
"d": {
"enum": ["", null],
"nullable": true
},
"e": {
"type": "string",
"nullable": true
},
"f": {
"type": "null",
"nullable": true
},
"g": {
"type": "string",
"const": "",
"nullable": true
},
"h": {
"type": "string",
"const": null,
"nullable": true
},
"i": {
"type": "string",
"enum": ["a", "b"],
"nullable": true
},
"j": {
"type": "string",
"enum": ["", null],
"nullable": true
},
"k": {
"type": ["string", "integer"],
"nullable": true
},
"l": {
"type": ["string", "null"],
"nullable": true
}
},
"required": [],
"additionalProperties": false
},
"out": {
"$id": "a",
"type": "object",
"properties": {
"a": {
"enum": ["", null]
},
"b": {
"enum": [null]
},
"c": {
"enum": ["a", "b", null]
},
"d": {
"enum": ["", null]
},
"e": {
"type": ["string", "null"]
},
"f": {
"type": "null"
},
"g": {
"type": "string",
"enum": ["", null]
},
"h": {
"type": "string",
"enum": [null]
},
"i": {
"type": "string",
"enum": ["a", "b", null]
},
"j": {
"type": "string",
"enum": ["", null]
},
"k": {
"type": ["string", "integer", "null"]
},
"l": {
"type": ["string", "null"]
}
},
"required": [],
"additionalProperties": false
}
}