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

Give format precendence over type property #831

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: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
# editors
#.vscode
.idea/
*.iml

# dependencies
node_modules
Expand Down Expand Up @@ -47,3 +48,4 @@ test.json
docs/.vitepress/cache/**
docs/.vitepress/dist/**
!__snapshots__
.pnpm-store/
1 change: 0 additions & 1 deletion packages/core/mocks/hellowWorld.js

This file was deleted.

51 changes: 51 additions & 0 deletions packages/swagger-ts/mocks/type_assertions.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
components:
schemas:
Body_upload_file_api_assets_post:
properties:
file:
format: binary
title: File
type: string
required:
- file
title: Body_upload_file_api_assets_post
type: object
$comment: |
export type BodyUploadFileApiAssetsPost = {
/**
* @type string binary
*/
file: Blob;
};
Plain_file:
format: binary
title: File
type: string
$comment: export type PlainFile = Blob;
Plain_date:
format: date
title: Date
type: string
$comment: export type PlainDate = Date;
Plain_time:
format: time
title: Time
type: number
$comment: export type PlainTime = number;
Plain_date_time:
format: date-time
title: Datetime
type: string
$comment:
Plain_email:
format: email
title: Email
type: string
Plain_uuid:
format: uuid
title: UUID
type: string
info:
title: Type Assertions
openapi: 3.1.0
paths: {}
159 changes: 159 additions & 0 deletions packages/swagger-ts/src/TypeGenerator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -457,3 +457,162 @@ describe('TypeGenerator enums', async () => {
expect(await format(output)).toMatchSnapshot()
})
})

describe('TypeGenerator type assertions', async () => {
const schemaPath = path.resolve(__dirname, '../mocks/type_assertions.yaml')
const oas = await new OasManager().parse(schemaPath)

const schemas = oas.getDefinition().components?.schemas

test('generates file property with `File` type', async () => {
const generator = new TypeGenerator({
usedEnumNames: {},
enumType: 'asConst',
dateType: 'string',
optionalType: 'questionToken',
transformers: {},
oasType: false,
unknownType: 'unknown',
}, {
oas,
pluginManager: mockedPluginManager,
})
const node = generator.build({ schema: schemas?.Body_upload_file_api_assets_post as OasTypes.SchemaObject, baseName: 'Body_upload_file_api_assets_post' })

const output = print(node, undefined)

expect(output).toBeDefined()
expect(output).toMatchSnapshot()
})

test('generates Plain_File types correctly', async () => {
const generator = new TypeGenerator({
usedEnumNames: {},
enumType: 'asConst',
dateType: 'string',
optionalType: 'questionToken',
transformers: {},
oasType: false,
unknownType: 'unknown',
}, {
oas,
pluginManager: mockedPluginManager,
})
const node = generator.build({ schema: schemas?.Plain_file as OasTypes.SchemaObject, baseName: 'Plain_file' })

const output = print(node, undefined)

expect(output).toBeDefined()
expect(output).toMatchSnapshot()
})

test('generates Date type correctly when dateType = date', async () => {
const generator = new TypeGenerator({
usedEnumNames: {},
enumType: 'asConst',
dateType: 'date',
optionalType: 'questionToken',
transformers: {},
oasType: false,
unknownType: 'unknown',
}, {
oas,
pluginManager: mockedPluginManager,
})

const node = generator.build({ schema: schemas?.Plain_date as OasTypes.SchemaObject, baseName: 'Plain_date' })

const output = print(node, undefined)

expect(output).toBeDefined()
expect(output).toMatchSnapshot()
})

test('generates Date type correctly when dateType = string', async () => {
const generator = new TypeGenerator({
usedEnumNames: {},
enumType: 'asConst',
dateType: 'string',
optionalType: 'questionToken',
transformers: {},
oasType: false,
unknownType: 'unknown',
}, {
oas,
pluginManager: mockedPluginManager,
})

const node = generator.build({ schema: schemas?.Plain_date as OasTypes.SchemaObject, baseName: 'Plain_date' })

const output = print(node, undefined)

expect(output).toBeDefined()
expect(output).toMatchSnapshot()
})

test('generates Time type correctly', async () => {
const generator = new TypeGenerator({
usedEnumNames: {},
enumType: 'asConst',
dateType: 'string',
optionalType: 'questionToken',
transformers: {},
oasType: false,
unknownType: 'unknown',
}, {
oas,
pluginManager: mockedPluginManager,
})

const node = generator.build({ schema: schemas?.Plain_time as OasTypes.SchemaObject, baseName: 'Plain_time' })

const output = print(node, undefined)

expect(output).toBeDefined()
expect(output).toMatchSnapshot()
})

test('generates Email type correctly', async () => {
const generator = new TypeGenerator({
usedEnumNames: {},
enumType: 'asConst',
dateType: 'string',
optionalType: 'questionToken',
transformers: {},
oasType: false,
unknownType: 'unknown',
}, {
oas,
pluginManager: mockedPluginManager,
})

const node = generator.build({ schema: schemas?.Plain_email as OasTypes.SchemaObject, baseName: 'Plain_email' })

const output = print(node, undefined)

expect(output).toBeDefined()
expect(output).toMatchSnapshot()
})

test('generates UUID type correctly', async () => {
const generator = new TypeGenerator({
usedEnumNames: {},
enumType: 'asConst',
dateType: 'string',
optionalType: 'questionToken',
transformers: {},
oasType: false,
unknownType: 'unknown',
}, {
oas,
pluginManager: mockedPluginManager,
})

const node = generator.build({ schema: schemas?.Plain_uuid as OasTypes.SchemaObject, baseName: 'Plain_uuid' })

const output = print(node, undefined)

expect(output).toBeDefined()
expect(output).toMatchSnapshot()
})
})
65 changes: 57 additions & 8 deletions packages/swagger-ts/src/TypeGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,14 @@
const schema = properties[name] as OasTypes.SchemaObject

const isRequired = Array.isArray(required) ? required.includes(name) : !!required
let type = this.getTypeFromSchema(schema, this.context.pluginManager.resolveName({ name: `${baseName || ''} ${name}`, pluginKey, type: 'type' }))
let type = this.getTypeFromSchema(
schema,
this.context.pluginManager.resolveName({
name: `${baseName || ''} ${name}`,
pluginKey,
type: 'type',
}),
)

if (!type) {
return null
Expand Down Expand Up @@ -303,7 +310,14 @@
type: this.options.enumType,
}),
)
return factory.createTypeReferenceNode(this.context.pluginManager.resolveName({ name: enumName, pluginKey, type: 'type' }), undefined)
return factory.createTypeReferenceNode(
this.context.pluginManager.resolveName({
name: enumName,
pluginKey,
type: 'type',
}),
undefined,
)
}

if (schema.enum) {
Expand Down Expand Up @@ -386,8 +400,47 @@
})
}

if (this.options.dateType === 'date' && ['date', 'date-time'].some((item) => item === schema.format)) {
return factory.createTypeReferenceNode(factory.createIdentifier('Date'))
/**
* > Structural validation alone may be insufficient to allow an application to correctly utilize certain values. The "format"
* > annotation keyword is defined to allow schema authors to convey semantic information for a fixed subset of values which are
* > accurately described by authoritative resources, be they RFCs or other external specifications.
*
* In other words: format is more specific than type alone, hence it should override the type value, if possible.
*
* see also https://json-schema.org/draft/2020-12/draft-bhutton-json-schema-validation-00#rfc.section.7
*/
if (schema.format) {
switch (schema.format) {
case 'binary':
if (schema.type === 'string') {
return factory.createTypeReferenceNode('Blob', [])
}
break

Check warning on line 418 in packages/swagger-ts/src/TypeGenerator.ts

View check run for this annotation

Codecov / codecov/patch

packages/swagger-ts/src/TypeGenerator.ts#L418

Added line #L418 was not covered by tests
// 7.3.1. Dates, Times, and Duration
case 'date-time':
case 'date':
case 'time':
Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually, im not sure if time should really be cast a Date. Maybe number instead?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We have dateType which can now be a date or a string so maybe we can also add number if that would be needed for other users.

if (this.options.dateType === 'date') {
return factory.createTypeReferenceNode(factory.createIdentifier('Date'))
}
case 'uuid':
// TODO how to work with this? use https://www.npmjs.com/package/uuid for it?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, uuid would be ignored like all the others, although it would made sense to make it an uuid, but this would require a custom task or a dependency uuid

Copy link
Collaborator

Choose a reason for hiding this comment

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

In the end, it will be a string, Typescript does not have a type uuid so I don't think we need to change something here for the ts plugin.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The thing is, the cases added here are directly from the openapi spec. That's why I added them here, too.
But I'll remove unsupported cases if you like.

Imo, being able to map to a specific type is very convinient. But kubb itself should add as little runtime dependencies as possible. Hence, uuid support via @types/uuid and uuid would be a great use case for a kubb plugin? Is that already possible with plugins, then I might give them a try.

break
case 'duration':
case 'email':
case 'idn-email':
case 'hostname':
case 'idn-hostname':
case 'ipv4':
case 'ipv6':
case 'uri':
case 'uri-reference':
case 'json-pointer':
case 'relative-json-pointer':
default:
// formats not yet implemented: ignore.
break
}
}

// string, boolean, null, number
Expand All @@ -396,10 +449,6 @@
}
}

if (schema.format === 'binary') {
return factory.createTypeReferenceNode('Blob', [])
}

return this.#unknownReturn
}

Expand Down
40 changes: 40 additions & 0 deletions packages/swagger-ts/src/__snapshots__/TypeGenerator.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -221,3 +221,43 @@ exports[`TypeGenerator petStoreRef > generate type for Pets 1`] = `
"export type Pets = Pet[]
"
`;

exports[`TypeGenerator type assertions > generates Date type correctly when dateType = date 1`] = `
"export type PlainDate = Date;
"
`;

exports[`TypeGenerator type assertions > generates Date type correctly when dateType = string 1`] = `
"export type PlainDate = string;
"
`;

exports[`TypeGenerator type assertions > generates Email type correctly 1`] = `
"export type PlainEmail = string;
"
`;

exports[`TypeGenerator type assertions > generates Plain_File types correctly 1`] = `
"export type PlainFile = Blob;
"
`;

exports[`TypeGenerator type assertions > generates Time type correctly 1`] = `
"export type PlainTime = number;
"
`;

exports[`TypeGenerator type assertions > generates UUID type correctly 1`] = `
"export type PlainUuid = string;
"
`;

exports[`TypeGenerator type assertions > generates file property with \`File\` type 1`] = `
"export type BodyUploadFileApiAssetsPost = {
/**
* @type string binary
*/
file: Blob;
};
"
`;
Loading