Skip to content

Commit

Permalink
fix: various failures when type is defined as array (#1209)
Browse files Browse the repository at this point in the history
Fixes #1196
  • Loading branch information
iliapolo committed Oct 5, 2023
1 parent f81fa55 commit 7d5abc5
Show file tree
Hide file tree
Showing 3 changed files with 287 additions and 0 deletions.
64 changes: 64 additions & 0 deletions src/type-generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,17 @@ export interface TypeGeneratorOptions {
*/
readonly toJson?: boolean;

/**
* When set to true, enums are sanitized from the 'null' literal value,
* allowing typing the property as an enum, instead of the underlying type.
*
* Note that switching this from 'false' to 'true' is a breaking change in
* the generated code as it might change a property type.
*
* @default false
*/
readonly sanitizeEnums?: boolean;

/**
* Given a definition name, render the type name to be emitted by that definition.
*
Expand Down Expand Up @@ -102,6 +113,7 @@ export class TypeGenerator {
private readonly exclude: string[];
private readonly definitions: { [def: string]: JSONSchema4 };
private readonly toJson: boolean;
private readonly sanitizeEnums: boolean;
private readonly renderTypeName: (def: string) => string;

/**
Expand All @@ -113,6 +125,7 @@ export class TypeGenerator {
this.exclude = options.exclude ?? [];
this.definitions = {};
this.toJson = options.toJson ?? true;
this.sanitizeEnums = options.sanitizeEnums ?? false;
this.renderTypeName = options.renderTypeName ?? DEFAULT_RENDER_TYPE_NAME;

for (const [typeName, def] of Object.entries(options.definitions ?? {})) {
Expand Down Expand Up @@ -156,6 +169,48 @@ export class TypeGenerator {
return this.emitTypeInternal(typeName, def, structFqn).type;
}

/**
* Many schemas define a type as an array of types to indicate union types.
* To avoid having the type generator be aware of that, we transform those types
* into their corresponding typescript definitions.
* --------------------------------------------------
*
* Strictly speaking, these definitions are meant to allow the liternal 'null' value
* to be used in addition to the actual types. However, since union types are not supported
* in jsii, allowing this would mean falling back to 'any' and loosing all type safety for such
* properties. Transforming it into a single concrete optional type provides both type safety and
* the option to omit the property. What it doesn't allow is explicitly passing 'null', which might
* be desired in some cases. For now we prefer type safety over that.
*
* 1. ['null', '<type>'] -> optional '<type>'
* 2. ['null', '<type1>', '<type2>'] -> optional 'any'
*
* This is the normal jsii conversion, nothing much we can do here.
*
* 3. ['<type1>', '<type2>'] -> 'any'
*/
private maybeTransformTypeArray(def: JSONSchema4) {

if (!Array.isArray(def.type)) {
return;
}

const nullType = def.type.some(t => t === 'null');
const nonNullTypes = new Set(def.type.filter(t => t !== 'null'));

if (nullType) {
def.required = false;
}

if (nonNullTypes.size === 0) {
def.type = 'null';
} else {
// if its a union of non null types we use 'any' to be jsii compliant
def.type = nonNullTypes.size > 1 ? 'any' : nonNullTypes.values().next().value;
}

}

/**
* Emit a type based on a JSON schema. If `def` is not specified, the
* definition of the type will be looked up in the `definitions` provided
Expand All @@ -175,6 +230,15 @@ export class TypeGenerator {
}
}

this.maybeTransformTypeArray(def);

if (def.enum && this.sanitizeEnums) {
// santizie enums from liternal 'null' because they prevent emitting the enum
// and instead cause a fallback to 'string'. we assume the optionality of the enum
// covers the 'null' value.
def.enum = def.enum.filter(d => d !== null);
}

// callers expect that emit a type named `typeName` so we can't change it here
// but at least we can verify it's correct.
if (TypeGenerator.normalizeTypeName(typeName) !== typeName) {
Expand Down
110 changes: 110 additions & 0 deletions test/__snapshots__/type-generator.test.ts.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

113 changes: 113 additions & 0 deletions test/type-generator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,119 @@ test('if "toJson" is disabled, toJson functions are not generated', async () =>
expect(await generate(gen)).toMatchSnapshot();
});

test('type can be an array with null and a single non null type', async () => {

const schema: JSONSchema4 = {
properties: {
bar: { type: ['null', 'boolean'] },
},
};

const gen = TypeGenerator.forStruct('Foo', schema, {
toJson: false,
});

expect(await generate(gen)).toMatchSnapshot();

});

test('additionalProperties when type is defined as array', async () => {

const schema: JSONSchema4 = {
properties: {
foo: {
type: ['null', 'object'],
additionalProperties: {
type: 'string',
},
},
},
};

const gen = TypeGenerator.forStruct('Foo', schema, {
toJson: false,
});

expect(await generate(gen)).toMatchSnapshot();

});

test('properties when type is defined as array', async () => {

const schema: JSONSchema4 = {
type: ['null', 'object'],
properties: {
bar: { type: 'string' },
},
};

const gen = TypeGenerator.forStruct('Foo', schema, {
toJson: false,
});

expect(await generate(gen)).toMatchSnapshot();

});

test('enum when type is defined as array', async () => {

const schema: JSONSchema4 = {
properties: {
foo: {
type: ['null', 'string'],
enum: ['val1', 'val2'],
},
},
};

const gen = TypeGenerator.forStruct('Foo', schema, {
toJson: false,
});

expect(await generate(gen)).toMatchSnapshot();

});

test('sanitize string enum when one of the values is null', async () => {

const schema: JSONSchema4 = {
properties: {
foo: {
type: ['null', 'string'],
enum: ['val1', null],
},
},
};

const gen = TypeGenerator.forStruct('Foo', schema, {
toJson: false,
sanitizeEnums: true,
});

expect(await generate(gen)).toMatchSnapshot();

});

test('sanitize number enum when one of the values is null', async () => {

const schema: JSONSchema4 = {
properties: {
foo: {
type: ['null', 'number'],
enum: [3, null],
},
},
};

const gen = TypeGenerator.forStruct('Foo', schema, {
toJson: false,
sanitizeEnums: true,
});

expect(await generate(gen)).toMatchSnapshot();

});

test('custom ref normalization', async () => {

const foo = 'io.k8s.v1beta1.Foo';
Expand Down

0 comments on commit 7d5abc5

Please sign in to comment.