Skip to content

Commit

Permalink
fix(core): ensure json type is recognized with type: 'jsonb'
Browse files Browse the repository at this point in the history
Also supports inference from `columnType` and improves the overall logic
for internal custom type mapping that is needed for JSON columns.

Closes #3998
  • Loading branch information
B4nan committed Feb 4, 2023
1 parent f06eeb0 commit e1f82bc
Show file tree
Hide file tree
Showing 7 changed files with 18 additions and 11 deletions.
9 changes: 8 additions & 1 deletion packages/core/src/metadata/ReflectMetadataProvider.ts
Expand Up @@ -10,12 +10,19 @@ export class ReflectMetadataProvider extends MetadataProvider {
}

protected initPropertyType(meta: EntityMetadata, prop: EntityProperty) {
const type = Reflect.getMetadata('design:type', meta.prototype, prop.name);
let type = Reflect.getMetadata('design:type', meta.prototype, prop.name);

if (!type || (type === Object && prop.reference !== ReferenceType.SCALAR)) {
throw new Error(`Please provide either 'type' or 'entity' attribute in ${meta.className}.${prop.name}`);
}

// Instead of requiring the type everywhere, we default to string, which maintains the behaviour,
// as we were mapping it to UnknownType which is a string. This is to prevent defaulting to JSON
// column type, which can be often hard to revert and cause hard to understand issues with PKs.
if (prop.reference === ReferenceType.SCALAR && type === Object) {
type = String;
}

prop.type = type.name;

if (prop.type && ['string', 'number', 'boolean', 'array', 'object'].includes(prop.type.toLowerCase())) {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/utils/EntityComparator.ts
Expand Up @@ -285,7 +285,7 @@ export class EntityComparator {
}

if (prop.type === 'boolean') {
lines.push(` if (typeof ${propName(prop.fieldNames[0])} !== 'undefined') { ret${this.wrap(prop.name)} = ${propName(prop.fieldNames[0])} == null ? ${propName(prop.fieldNames[0])} : !!${propName(prop.fieldNames[0])}; mapped.${prop.fieldNames[0]} = true; }`);
lines.push(` if (typeof ${propName(prop.fieldNames[0])} !== 'undefined') { ret${this.wrap(prop.name)} = ${propName(prop.fieldNames[0])} == null ? ${propName(prop.fieldNames[0])} : !!${propName(prop.fieldNames[0])}; ${propName(prop.fieldNames[0], 'mapped')} = true; }`);
} else {
lines.push(` if (typeof ${propName(prop.fieldNames[0])} !== 'undefined') { ret${this.wrap(prop.name)} = ${propName(prop.fieldNames[0])}; ${propName(prop.fieldNames[0], 'mapped')} = true; }`);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/entities-sql/BaseEntity2.ts
Expand Up @@ -6,7 +6,7 @@ export abstract class BaseEntity2 {
id!: number;

@Property({ persist: false })
hookTest = false;
hookTest: boolean = false;

protected constructor() {
const props = wrap(this, true).__meta.properties;
Expand Down
2 changes: 1 addition & 1 deletion tests/entities/BaseEntity.ts
Expand Up @@ -23,7 +23,7 @@ export abstract class BaseEntity<T extends { id: unknown; _id: unknown }, Option
foo?: string;

@Property({ persist: false })
hookTest = false;
hookTest: boolean = false;

@BeforeCreate()
baseBeforeCreate() {
Expand Down
6 changes: 3 additions & 3 deletions tests/features/composite-keys/GH1157.test.ts
Expand Up @@ -7,7 +7,7 @@ import { SqliteDriver } from '@mikro-orm/sqlite';
export class D {

@PrimaryKey()
id = v4();
id: string = v4();

@ManyToOne({ entity: 'A' })
a!: any;
Expand All @@ -18,15 +18,15 @@ export class D {
export class C {

@PrimaryKey()
id = v4();
id: string = v4();

}

@Entity()
export class B {

@PrimaryKey()
id = v4();
id: string = v4();

}

Expand Down
4 changes: 2 additions & 2 deletions tests/features/custom-types/json-hydration.postgres.test.ts
@@ -1,13 +1,13 @@
import { MikroORM } from '@mikro-orm/postgresql';
import { Embedded, Entity, PrimaryKey, Embeddable, OneToOne, Property, t } from '@mikro-orm/core';
import { Embedded, Entity, PrimaryKey, Embeddable, OneToOne, Property } from '@mikro-orm/core';

@Embeddable()
export class Page {

private _attestations!: string[];
static log: unknown[] = [];

@Property({ type: t.json })
@Property({ type: 'jsonb' })
get attestations(): string[] {
return this._attestations;
}
Expand Down
4 changes: 2 additions & 2 deletions tests/issues/GH1958.test.ts
Expand Up @@ -5,10 +5,10 @@ import { SqliteDriver } from '@mikro-orm/sqlite';
class LoopOptions {

@Property()
'enabled-prop' = false;
'enabled-prop': boolean = false;

@Property()
'type-prop' = 'a';
'type-prop': string = 'a';

}

Expand Down

0 comments on commit e1f82bc

Please sign in to comment.