Skip to content

Commit

Permalink
API Generator: Change default value for input field if property has n…
Browse files Browse the repository at this point in the history 
…o initializer (#2324)

Previously, the following property of an entity

```ts
@Property({ type: types.date, nullable: true })
@field({ nullable: true })
availableSince?: Date;
```

resulted in the following input being generated:

```ts
@isnullable()
@isdate()
@field({ nullable: true })
availableSince?: Date;
```

This was problematic for two reasons:

1. The error message would be misleading when trying to create an entity
without providing a value for the property. For example, a valid GraphQL
mutation

    ```graphql
    mutation CreateProduct {
createProduct(input: { title: "A", slug: "A", description: "FOO" }) {
            id
            availableSince
        }
    }
    ```

    would result in the following error:

    ```
    "isDate": "availableSince must be a Date instance"
    ```

2. Relying on the initializer as the default value is not obvious and
appears somewhat magical.

To address this, we now use `null` as the default value for nullable
properties if no initializer is provided. If an initializer is provided,
it is used as the default value.

---------

Co-authored-by: Johannes Obermair <johannes.obermair@vivid-planet.com>
  • Loading branch information
@dkarnutsch @johnnyomair
dkarnutsch and johnnyomair authored Jul 29, 2024
1 parent ff526f2 commit bb04fb8
Show file tree
Hide file tree
Showing 3 changed files with 170 additions and 8 deletions.
45 changes: 45 additions & 0 deletions .changeset/forty-ways-matter.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
---
"@comet/cms-api": minor
---

API Generator: Change default value for input field if property has no initializer

Previously, the following property of an entity

```ts
@Property({ type: types.date, nullable: true })
@Field({ nullable: true })
availableSince?: Date;
```

resulted in the following input being generated:

```ts
@IsNullable()
@IsDate()
@Field({ nullable: true })
availableSince?: Date;
```

This was problematic for two reasons:

1. The error message would be misleading when trying to create an entity without providing a value for the property. For example, a valid GraphQL mutation

```graphql
mutation CreateProduct {
createProduct(input: { title: "A", slug: "A", description: "FOO" }) {
id
availableSince
}
}
```

would result in the following error:

```
"isDate": "availableSince must be a Date instance"
```

2. Relying on the initializer as the default value is not obvious and appears somewhat magical.

To address this, we now use `null` as the default value for nullable properties if no initializer is provided. If an initializer is provided, it is used as the default value.
108 changes: 108 additions & 0 deletions packages/api/cms-api/src/generator/generate-crud-input.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,24 @@ export class TestEntityWithTextRuntimeType extends BaseEntity<TestEntityWithText
title: string;
}

@Entity()
export class TestEntityWithNullablePropWithInitializer extends BaseEntity<TestEntityWithNullablePropWithInitializer, "id"> {
@PrimaryKey({ type: "uuid" })
id: string = uuid();

@Property({ type: "text", nullable: true })
title?: string = undefined;
}

@Entity()
export class TestEntityWithNullablePropWithoutInitializer extends BaseEntity<TestEntityWithNullablePropWithoutInitializer, "id"> {
@PrimaryKey({ type: "uuid" })
id: string = uuid();

@Property({ type: "text", nullable: true })
title?: string;
}

describe("GenerateCrudInput", () => {
describe("string input class", () => {
it("should be a valid generated ts file", async () => {
Expand Down Expand Up @@ -276,4 +294,94 @@ describe("GenerateCrudInput", () => {
orm.close();
});
});

describe("nullable props input class with initializer", () => {
it("should be a valid generated ts file", async () => {
LazyMetadataStorage.load();
const orm = await MikroORM.init({
type: "postgresql",
dbName: "test-db",
entities: [TestEntityWithNullablePropWithInitializer],
});
const out = await generateCrudInput(
{ targetDirectory: __dirname },
orm.em.getMetadata().get("TestEntityWithNullablePropWithInitializer"),
);
const lintedOutput = await lintSource(out[0].content);
//console.log(lintedOutput);
const source = parseSource(lintedOutput);

const classes = source.getClasses();
expect(classes.length).toBe(2);

const cls = classes[0];
const structure = cls.getStructure();

expect(structure.properties?.length).toBe(1);
{
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const prop = structure.properties![0];
expect(prop.name).toBe("title");
expect(prop.type).toBe("string");

const decorators = prop.decorators?.map((i) => i.name);
expect(decorators).toContain("Field");
expect(decorators).toContain("IsString");
expect(decorators).toContain("IsNullable");

// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const fieldDecorator = prop.decorators!.find((i) => i.name === "Field")!;
const fieldDecoratorArguments = fieldDecorator.arguments as string[];
expect(fieldDecoratorArguments[0]).toContain("nullable: true");
expect(fieldDecoratorArguments[0]).toContain("defaultValue: null");
}

orm.close();
});
});

describe("nullable props input class without initializer", () => {
it("should be a valid generated ts file", async () => {
LazyMetadataStorage.load();
const orm = await MikroORM.init({
type: "postgresql",
dbName: "test-db",
entities: [TestEntityWithNullablePropWithoutInitializer],
});
const out = await generateCrudInput(
{ targetDirectory: __dirname },
orm.em.getMetadata().get("TestEntityWithNullablePropWithoutInitializer"),
);
const lintedOutput = await lintSource(out[0].content);
//console.log(lintedOutput);
const source = parseSource(lintedOutput);

const classes = source.getClasses();
expect(classes.length).toBe(2);

const cls = classes[0];
const structure = cls.getStructure();

expect(structure.properties?.length).toBe(1);
{
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const prop = structure.properties![0];
expect(prop.name).toBe("title");
expect(prop.type).toBe("string");

const decorators = prop.decorators?.map((i) => i.name);
expect(decorators).toContain("Field");
expect(decorators).toContain("IsString");
expect(decorators).toContain("IsNullable");

// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const fieldDecorator = prop.decorators!.find((i) => i.name === "Field")!;
const fieldDecoratorArguments = fieldDecorator.arguments as string[];
expect(fieldDecoratorArguments[0]).toContain("nullable: true");
expect(fieldDecoratorArguments[0]).toContain("defaultValue: null");
}

orm.close();
});
});
});
25 changes: 17 additions & 8 deletions packages/api/cms-api/src/generator/generate-crud-input.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ export async function generateCrudInput(
continue;
} else if (prop.enum) {
const initializer = morphTsProperty(prop.name, metadata).getInitializer()?.getText();
const defaultValue = prop.nullable && (initializer == "undefined" || initializer == "null") ? "null" : initializer;
const defaultValue =
prop.nullable && (initializer == "undefined" || initializer == "null" || initializer === undefined) ? "null" : initializer;
const fieldOptions = tsCodeRecordToString({ nullable: prop.nullable ? "true" : undefined, defaultValue });
const enumName = findEnumName(prop.name, metadata);
const importPath = findEnumImportPath(enumName, `${generatorOptions.targetDirectory}/dto`, metadata);
Expand All @@ -103,7 +104,8 @@ export async function generateCrudInput(
type = `${enumName}[]`;
} else if (prop.type === "string" || prop.type === "text") {
const initializer = morphTsProperty(prop.name, metadata).getInitializer()?.getText();
const defaultValue = prop.nullable && (initializer == "undefined" || initializer == "null") ? "null" : initializer;
const defaultValue =
prop.nullable && (initializer == "undefined" || initializer == "null" || initializer === undefined) ? "null" : initializer;
const fieldOptions = tsCodeRecordToString({ nullable: prop.nullable ? "true" : undefined, defaultValue });
decorators.push("@IsString()");
if (prop.name.startsWith("scope_")) {
Expand All @@ -116,7 +118,8 @@ export async function generateCrudInput(
type = "string";
} else if (prop.type === "DecimalType" || prop.type == "BigIntType" || prop.type === "number") {
const initializer = morphTsProperty(prop.name, metadata).getInitializer()?.getText();
const defaultValue = prop.nullable && (initializer == "undefined" || initializer == "null") ? "null" : initializer;
const defaultValue =
prop.nullable && (initializer == "undefined" || initializer == "null" || initializer === undefined) ? "null" : initializer;
const fieldOptions = tsCodeRecordToString({ nullable: prop.nullable ? "true" : undefined, defaultValue });
if (integerTypes.includes(prop.columnTypes[0])) {
decorators.push("@IsInt()");
Expand All @@ -128,14 +131,16 @@ export async function generateCrudInput(
type = "number";
} else if (prop.type === "DateType" || prop.type === "Date") {
const initializer = morphTsProperty(prop.name, metadata).getInitializer()?.getText();
const defaultValue = prop.nullable && (initializer == "undefined" || initializer == "null") ? "null" : initializer;
const defaultValue =
prop.nullable && (initializer == "undefined" || initializer == "null" || initializer === undefined) ? "null" : initializer;
const fieldOptions = tsCodeRecordToString({ nullable: prop.nullable ? "true" : undefined, defaultValue });
decorators.push("@IsDate()");
decorators.push(`@Field(${fieldOptions})`);
type = "Date";
} else if (prop.type === "BooleanType" || prop.type === "boolean") {
const initializer = morphTsProperty(prop.name, metadata).getInitializer()?.getText();
const defaultValue = prop.nullable && (initializer == "undefined" || initializer == "null") ? "null" : initializer;
const defaultValue =
prop.nullable && (initializer == "undefined" || initializer == "null" || initializer === undefined) ? "null" : initializer;
const fieldOptions = tsCodeRecordToString({ nullable: prop.nullable ? "true" : undefined, defaultValue });
decorators.push("@IsBoolean()");
decorators.push(`@Field(${fieldOptions})`);
Expand All @@ -153,7 +158,7 @@ export async function generateCrudInput(
type = "BlockInputInterface";
} else if (prop.reference == "m:1") {
const initializer = morphTsProperty(prop.name, metadata).getInitializer()?.getText();
const defaultValueNull = prop.nullable && (initializer == "undefined" || initializer == "null");
const defaultValueNull = prop.nullable && (initializer == "undefined" || initializer == "null" || initializer === undefined);
const fieldOptions = tsCodeRecordToString({
nullable: prop.nullable ? "true" : undefined,
defaultValue: defaultValueNull ? "null" : undefined,
Expand Down Expand Up @@ -290,7 +295,11 @@ export async function generateCrudInput(
if (tsType.isArray()) {
const initializer = tsProp.getInitializer()?.getText();
const defaultValue =
prop.nullable && (initializer == "undefined" || initializer == "null") ? "null" : initializer == "[]" ? "[]" : undefined;
prop.nullable && (initializer == "undefined" || initializer == "null" || initializer === undefined)
? "null"
: initializer == "[]"
? "[]"
: undefined;
const fieldOptions = tsCodeRecordToString({
nullable: prop.nullable ? "true" : undefined,
defaultValue: defaultValue,
Expand Down Expand Up @@ -330,7 +339,7 @@ export async function generateCrudInput(
}
} else if (prop.type == "uuid") {
const initializer = morphTsProperty(prop.name, metadata).getInitializer()?.getText();
const defaultValueNull = prop.nullable && (initializer == "undefined" || initializer == "null");
const defaultValueNull = prop.nullable && (initializer == "undefined" || initializer == "null" || initializer === undefined);
const fieldOptions = tsCodeRecordToString({
nullable: prop.nullable ? "true" : undefined,
defaultValue: defaultValueNull ? "null" : undefined,
Expand Down

0 comments on commit bb04fb8

Please sign in to comment.