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.

Sign up for GitHub

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(zod): treat additionalProperties keyword #1443

Merged
merged 4 commits into from
Jun 11, 2024
Merged

fix(zod): treat additionalProperties keyword #1443

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eaec970
chore(zod): typos
ixth Jun 8, 2024
695565f
refactor(zod): pull up definition type
ixth Jun 8, 2024
6448cef
test(zod): add tests for additionalProperties keyword
ixth Jun 8, 2024
b4692cc
fix(zod): fix handling of additionalProperties in zod generator
ixth Jun 8, 2024
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
58 changes: 29 additions & 29 deletions packages/zod/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,20 @@ const resolveZodType = (schemaTypeValue: SchemaObject['type']) => {
let constsUniqueCounter: Record<string, number> = {};

// https://github.com/colinhacks/zod#coercion-for-primitives
const COERCEABLE_TYPES = ['string', 'number', 'boolean', 'bigint', 'date'];
const COERCIBLE_TYPES = ['string', 'number', 'boolean', 'bigint', 'date'];

export type ZodValidationSchemaDefinition = {
functions: [string, any][];
consts: string[];
};

export const generateZodValidationSchemaDefinition = (
schema: SchemaObject | undefined,
context: ContextSpecs,
_required: boolean | undefined,
name: string,
strict: boolean,
): { functions: [string, any][]; consts: string[] } => {
): ZodValidationSchemaDefinition => {
if (!schema) return { functions: [], consts: [] };

const consts: string[] = [];
Expand Down Expand Up @@ -224,15 +229,15 @@ export const generateZodValidationSchemaDefinition = (
if (schema.additionalProperties) {
functions.push([
'additionalProperties',
isBoolean(schema.additionalProperties)
? schema.additionalProperties
: generateZodValidationSchemaDefinition(
schema.additionalProperties as SchemaObject,
context,
true,
name,
strict,
),
generateZodValidationSchemaDefinition(
isBoolean(schema.additionalProperties)
Copy link
Contributor Author

@ixth ixth Jun 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Actual fix which I'm not sure about: since additionalProperties: true is just an alias for additionalProperties: {}, I decided to treat it as latter. Should definition be faithful to original scheme?

Copy link
Member

Choose a reason for hiding this comment

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

Is it correct to understand that if it is simply true, it is determined that the additional property does not exist?

Copy link

@ixth-ind ixth-ind Jun 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

As far as I understood from the OpenAPI docs, it's the opposite:

If the dictionary values can be of any type (aka free-form object), use additionalProperties: true:

https://swagger.io/docs/specification/data-models/dictionaries/#free-form

So, additionalProperties: true is equivalent to additionalProperties: {}, which means that any type of property values allowed. Basically, it means it's Record<string, any>. additionalProperties: {} works fine, btw, it's just additionalProperties: true that's broken.

UPD. Oops, used work account. Comment is still relevant, though.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, I understood.

? {}
: (schema.additionalProperties as SchemaObject),
context,
true,
name,
strict,
),
]);

break;
Expand Down Expand Up @@ -292,14 +297,9 @@ export const generateZodValidationSchemaDefinition = (
return { functions, consts: uniq(consts) };
};

export type ZodValidationSchemaDefinitionInput = {
functions: [string, any][];
consts: string[];
};

export const parseZodValidationSchemaDefinition = (
input: ZodValidationSchemaDefinitionInput,
contex: ContextSpecs,
input: ZodValidationSchemaDefinition,
context: ContextSpecs,
coerceTypes: boolean | ZodCoerceType[] = false,
preprocessResponse?: GeneratorMutator,
): { zod: string; consts: string } => {
Expand Down Expand Up @@ -360,10 +360,10 @@ export const parseZodValidationSchemaDefinition = (
return `zod.object({
${Object.entries(args)
.map(([key, schema]) => {
const value = (schema as ZodValidationSchemaDefinitionInput).functions
const value = (schema as ZodValidationSchemaDefinition).functions
.map(parseProperty)
.join('');
consts += (schema as ZodValidationSchemaDefinitionInput).consts.join('\n');
consts += (schema as ZodValidationSchemaDefinition).consts.join('\n');
return ` "${key}": ${value.startsWith('.') ? 'zod' : ''}${value}`;
})
.join(',\n')}
Expand All @@ -387,11 +387,11 @@ ${Object.entries(args)
coerceTypes &&
(Array.isArray(coerceTypes)
? coerceTypes.includes(fn as ZodCoerceType)
: COERCEABLE_TYPES.includes(fn));
: COERCIBLE_TYPES.includes(fn));

if (
(fn !== 'date' && shouldCoerceType) ||
(fn === 'date' && shouldCoerceType && contex.output.override.useDates)
(fn === 'date' && shouldCoerceType && context.output.override.useDates)
) {
return `.coerce.${fn}(${args})`;
}
Expand Down Expand Up @@ -461,7 +461,7 @@ const parseBodyAndResponse = ({
name: string;
strict: boolean;
}): {
input: ZodValidationSchemaDefinitionInput;
input: ZodValidationSchemaDefinition;
isArray: boolean;
} => {
if (!data) {
Expand Down Expand Up @@ -531,9 +531,9 @@ const parseParameters = ({
response: boolean;
};
}): {
headers: ZodValidationSchemaDefinitionInput;
queryParams: ZodValidationSchemaDefinitionInput;
params: ZodValidationSchemaDefinitionInput;
headers: ZodValidationSchemaDefinition;
queryParams: ZodValidationSchemaDefinition;
params: ZodValidationSchemaDefinition;
} => {
if (!data) {
return {
Expand Down Expand Up @@ -609,7 +609,7 @@ const parseParameters = ({
>,
);

const headers: ZodValidationSchemaDefinitionInput = {
const headers: ZodValidationSchemaDefinition = {
functions: [],
consts: [],
};
Expand All @@ -622,7 +622,7 @@ const parseParameters = ({
}
}

const queryParams: ZodValidationSchemaDefinitionInput = {
const queryParams: ZodValidationSchemaDefinition = {
functions: [],
consts: [],
};
Expand All @@ -635,7 +635,7 @@ const parseParameters = ({
}
}

const params: ZodValidationSchemaDefinitionInput = {
const params: ZodValidationSchemaDefinition = {
functions: [],
consts: [],
};
Expand Down
113 changes: 111 additions & 2 deletions packages/zod/src/zod.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { describe, expect, it } from 'vitest';
import {
type ZodValidationSchemaDefinitionInput,
type ZodValidationSchemaDefinition,
parseZodValidationSchemaDefinition,
generateZodValidationSchemaDefinition,
} from '.';
import { SchemaObject } from 'openapi3-ts/oas30';
import { ContextSpecs } from '@orval/core';

const queryParams: ZodValidationSchemaDefinitionInput = {
const queryParams: ZodValidationSchemaDefinition = {
functions: [
[
'object',
Expand Down Expand Up @@ -42,6 +42,29 @@ const queryParams: ZodValidationSchemaDefinitionInput = {
consts: [],
};

const record: ZodValidationSchemaDefinition = {
functions: [
[
'object',
{
queryParams: {
functions: [
[
'additionalProperties',
{
functions: [['any', undefined]],
consts: [],
},
],
],
consts: [],
},
},
],
],
consts: [],
};

describe('parseZodValidationSchemaDefinition', () => {
describe('with `override.coerceTypes = false` (default)', () => {
it('does not emit coerced zod property schemas', () => {
Expand Down Expand Up @@ -82,6 +105,24 @@ describe('parseZodValidationSchemaDefinition', () => {
);
});
});

it('treats additionalProperties properly', () => {
const parseResult = parseZodValidationSchemaDefinition(
record,
{
output: {
override: {
useDates: false,
},
},
} as ContextSpecs,
false,
);

expect(parseResult.zod).toBe(
'zod.object({\n "queryParams": zod.record(zod.string(), zod.any())\n})',
);
});
});

const objectIntoObjectSchema: SchemaObject = {
Expand Down Expand Up @@ -119,6 +160,20 @@ const deepRequiredSchema: SchemaObject = {
},
};

const additionalPropertiesSchema: SchemaObject = {
type: 'object',
properties: {
any: {
type: 'object',
additionalProperties: {},
},
true: {
type: 'object',
additionalProperties: true,
},
},
};

describe('generateZodValidationSchemaDefinition`', () => {
it('required', () => {
const result = generateZodValidationSchemaDefinition(
Expand Down Expand Up @@ -224,4 +279,58 @@ describe('generateZodValidationSchemaDefinition`', () => {
consts: [],
});
});

it('additionalProperties', () => {
const result = generateZodValidationSchemaDefinition(
additionalPropertiesSchema,
{
output: {
override: {
useDates: false,
},
},
} as ContextSpecs,
true,
'strict',
true,
);

expect(result).toEqual({
functions: [
[
'object',
{
any: {
functions: [
[
'additionalProperties',
{
functions: [['any', undefined]],
consts: [],
},
],
['optional', undefined],
],
consts: [],
},
true: {
functions: [
[
'additionalProperties',
{
functions: [['any', undefined]],
consts: [],
},
],
['optional', undefined],
],
consts: [],
},
},
],
['strict', undefined],
],
consts: [],
});
});
});
9 changes: 9 additions & 0 deletions tests/configs/zod.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,4 +128,13 @@ export default defineConfig({
target: '../specifications/circular.yaml',
},
},
additionalProperties: {
output: {
target: '../generated/zod',
client: 'zod',
},
input: {
target: '../specifications/additional-properties.yaml',
},
},
});
Loading