Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
refactor(core): PrimaryKeyType symbol should be defined as optional
BREAKING CHANGE:
Previously when we had nonstandard PK types, we could use `PrimaryKeyType` symbol
to let the type system know it. It was required to define this property as required,
now it can be defined as optional too.
  • Loading branch information
B4nan committed Jan 30, 2022
1 parent 2385f1d commit db0b399
Show file tree
Hide file tree
Showing 26 changed files with 59 additions and 49 deletions.
8 changes: 4 additions & 4 deletions docs/docs/composite-keys.md
Expand Up @@ -33,7 +33,7 @@ export class Car {
@PrimaryKey()
year: number;

[PrimaryKeyType]: [string, number]; // this is needed for proper type checks in `FilterQuery`
[PrimaryKeyType]?: [string, number]; // this is needed for proper type checks in `FilterQuery`

constructor(name: string, year: number) {
this.name = name;
Expand Down Expand Up @@ -120,7 +120,7 @@ export class ArticleAttribute {
@Property()
value!: string;

[PrimaryKeyType]: [number, string]; // this is needed for proper type checks in `FilterQuery`
[PrimaryKeyType]?: [number, string]; // this is needed for proper type checks in `FilterQuery`

constructor(name: string, value: string, article: Article) {
this.attribute = name;
Expand Down Expand Up @@ -155,7 +155,7 @@ export class Address {
@OneToOne({ primary: true })
user!: User;

[PrimaryKeyType]: number; // this is needed for proper type checks in `FilterQuery`
[PrimaryKeyType]?: number; // this is needed for proper type checks in `FilterQuery`

}
```
Expand Down Expand Up @@ -223,7 +223,7 @@ export class OrderItem {
@Property()
offeredPrice: number;

[PrimaryKeyType]: [number, number]; // this is needed for proper type checks in `FilterQuery`
[PrimaryKeyType]?: [number, number]; // this is needed for proper type checks in `FilterQuery`

constructor(order: Order, product: Product, amount = 1) {
this.order = order;
Expand Down
13 changes: 13 additions & 0 deletions docs/docs/upgrading-v4-to-v5.md
Expand Up @@ -224,3 +224,16 @@ const a = await em.find(Author, { books: [1, 2, 3] }, {
populateWhere: PopulateHint.INFER,
});
```

## `em.create()` respects required properties

`em.create()` will now require you to pass all non-optional properties. Some
properties might be defined as required for TS, but we have a default value for
them (either runtime, or database one) - for such we can use `OptionalProps`
symbol to specify which properties should be considered as optional.

## `PrimaryKeyType` symbol should be defined as optional

Previously when we had nonstandard PK types, we could use `PrimaryKeyType` symbol
to let the type system know it. It was required to define this property as
required, now it can be defined as optional too.
10 changes: 5 additions & 5 deletions packages/core/src/EntityManager.ts
Expand Up @@ -32,7 +32,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
private readonly unitOfWork: UnitOfWork = new UnitOfWork(this);
private readonly entityFactory: EntityFactory = new EntityFactory(this.unitOfWork, this);
private readonly resultCache = this.config.getResultCacheAdapter();
private filters: Dictionary<FilterDef<any>> = {};
private filters: Dictionary<FilterDef> = {};
private filterParams: Dictionary<Dictionary> = {};
private transactionContext?: Transaction;
private flushMode?: FlushMode;
Expand Down Expand Up @@ -187,8 +187,8 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
/**
* Registers global filter to this entity manager. Global filters are enabled by default (unless disabled via last parameter).
*/
addFilter(name: string, cond: FilterQuery<AnyEntity> | ((args: Dictionary) => FilterQuery<AnyEntity>), entityName?: EntityName<AnyEntity> | EntityName<AnyEntity>[], enabled = true): void {
const options: FilterDef<AnyEntity> = { name, cond, default: enabled };
addFilter(name: string, cond: Dictionary | ((args: Dictionary) => FilterQuery<AnyEntity>), entityName?: EntityName<AnyEntity> | EntityName<AnyEntity>[], enabled = true): void {
const options: FilterDef = { name, cond, default: enabled };

if (entityName) {
options.entity = Utils.asArray(entityName).map(n => Utils.className(n));
Expand Down Expand Up @@ -251,15 +251,15 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
*/
async applyFilters<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, options: Dictionary<boolean | Dictionary> | string[] | boolean, type: 'read' | 'update' | 'delete'): Promise<FilterQuery<T>> {
const meta = this.metadata.find<T>(entityName);
const filters: FilterDef<any>[] = [];
const filters: FilterDef[] = [];
const ret: Dictionary[] = [];

if (!meta) {
return where;
}

const active = new Set<string>();
const push = (source: Dictionary<FilterDef<any>>) => {
const push = (source: Dictionary<FilterDef>) => {
const activeFilters = QueryHelper
.getActiveFilters(entityName, options, source)
.filter(f => !active.has(f.name));
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/decorators/Filter.ts
@@ -1,10 +1,10 @@
import { MetadataStorage } from '../metadata';
import type { Dictionary, FilterDef } from '../typings';

export function Filter<T>(options: FilterDef<T>) {
export function Filter<T>(options: FilterDef) {
return function <U>(target: U & Dictionary) {
const meta = MetadataStorage.getMetadataFromDecorator(target);
meta.filters[options.name] = options as unknown as FilterDef<U>;
meta.filters[options.name] = options;

return target;
};
Expand Down
11 changes: 4 additions & 7 deletions packages/core/src/typings.ts
Expand Up @@ -30,7 +30,7 @@ export const PrimaryKeyProp = Symbol('PrimaryKeyProp');
export const OptionalProps = Symbol('OptionalProps');

type ReadonlyPrimary<T> = T extends any[] ? Readonly<T> : T;
export type Primary<T> = T extends { [PrimaryKeyType]: infer PK } // TODO `PrimaryKeyType` should be optional
export type Primary<T> = T extends { [PrimaryKeyType]?: infer PK }
? ReadonlyPrimary<PK> : T extends { _id: infer PK }
? ReadonlyPrimary<PK> | string : T extends { uuid: infer PK }
? ReadonlyPrimary<PK> : T extends { id: infer PK }
Expand Down Expand Up @@ -73,14 +73,12 @@ export type OperatorMap<T> = {

export type FilterValue2<T> = T | ExpandScalar<T> | Primary<T>;
export type FilterValue<T> = OperatorMap<FilterValue2<T>> | FilterValue2<T> | FilterValue2<T>[] | null;
// eslint-disable-next-line @typescript-eslint/ban-types
type ExpandObject<T> = T extends object
? T extends Scalar
? never
: { [K in keyof T]?: Query<ExpandProperty<T[K]>> | FilterValue<ExpandProperty<T[K]>> | null }
: never;

// eslint-disable-next-line @typescript-eslint/ban-types
export type Query<T> = T extends object
? T extends Scalar
? never
Expand Down Expand Up @@ -433,7 +431,7 @@ export interface EntityMetadata<T extends AnyEntity<T> = any> {
class: Constructor<T>;
abstract: boolean;
useCache: boolean;
filters: Dictionary<FilterDef<T>>;
filters: Dictionary<FilterDef>;
comment?: string;
selfReferencing?: boolean;
readonly?: boolean;
Expand Down Expand Up @@ -534,9 +532,9 @@ export interface MigrationObject {
class: Constructor<Migration>;
}

export type FilterDef<T extends AnyEntity<T>> = {
export type FilterDef = {
name: string;
cond: FilterQuery<T> | ((args: Dictionary, type: 'read' | 'update' | 'delete', em: any) => FilterQuery<T> | Promise<FilterQuery<T>>);
cond: Dictionary | ((args: Dictionary, type: 'read' | 'update' | 'delete', em: any) => Dictionary | Promise<Dictionary>);
default?: boolean;
entity?: string[];
args?: boolean;
Expand All @@ -558,7 +556,6 @@ type StringKeys<T> = T extends Collection<any>
? `${Exclude<keyof ExtractType<T>, symbol>}`
: T extends Reference<any>
? `${Exclude<keyof ExtractType<T>, symbol>}`
// eslint-disable-next-line @typescript-eslint/ban-types
: T extends object
? `${Exclude<keyof ExtractType<T>, symbol>}`
: never;
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/utils/Configuration.ts
Expand Up @@ -399,7 +399,7 @@ export interface MikroORMOptions<D extends IDatabaseDriver = IDatabaseDriver> ex
entities: (string | EntityClass<AnyEntity> | EntityClassGroup<AnyEntity> | EntitySchema<any>)[]; // `any` required here for some TS weirdness
entitiesTs: (string | EntityClass<AnyEntity> | EntityClassGroup<AnyEntity> | EntitySchema<any>)[]; // `any` required here for some TS weirdness
subscribers: EventSubscriber[];
filters: Dictionary<{ name?: string } & Omit<FilterDef<AnyEntity>, 'name'>>;
filters: Dictionary<{ name?: string } & Omit<FilterDef, 'name'>>;
discovery: {
warnWhenNoEntities?: boolean;
requireEntitiesArray?: boolean;
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/utils/QueryHelper.ts
Expand Up @@ -175,7 +175,7 @@ export class QueryHelper {
}, {} as ObjectQuery<T>);
}

static getActiveFilters(entityName: string, options: Dictionary<boolean | Dictionary> | string[] | boolean, filters: Dictionary<FilterDef<any>>): FilterDef<any>[] {
static getActiveFilters(entityName: string, options: Dictionary<boolean | Dictionary> | string[] | boolean, filters: Dictionary<FilterDef>): FilterDef[] {
if (options === false) {
return [];
}
Expand All @@ -196,7 +196,7 @@ export class QueryHelper {
});
}

static isFilterActive(entityName: string, filterName: string, filter: FilterDef<any>, options: Dictionary<boolean | Dictionary>): boolean {
static isFilterActive(entityName: string, filterName: string, filter: FilterDef, options: Dictionary<boolean | Dictionary>): boolean {
if (filter.entity && !filter.entity.includes(entityName)) {
return false;
}
Expand Down
4 changes: 2 additions & 2 deletions packages/knex/src/query/QueryBuilder.ts
@@ -1,7 +1,7 @@
import type { Knex } from 'knex';
import type {
AnyEntity, Dictionary, EntityData, EntityMetadata, EntityProperty, FlatQueryOrderMap,
GroupOperator, MetadataStorage, PopulateOptions, QBFilterQuery, QueryOrderMap, QueryResult, FlushMode,
GroupOperator, MetadataStorage, PopulateOptions, QBFilterQuery, QueryOrderMap, QueryResult, FlushMode, FilterQuery,
} from '@mikro-orm/core';
import { LoadStrategy, LockMode, QueryFlag, QueryHelper, ReferenceType, Utils, ValidationError } from '@mikro-orm/core';
import { QueryType } from './enums';
Expand Down Expand Up @@ -185,7 +185,7 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
cond = { [`(${cond})`]: Utils.asArray(params) };
operator = operator || '$and';
} else {
cond = QueryHelper.processWhere(cond, this.entityName, this.metadata, this.platform, this.flags.has(QueryFlag.CONVERT_CUSTOM_TYPES))!;
cond = QueryHelper.processWhere(cond, this.entityName, this.metadata, this.platform, this.flags.has(QueryFlag.CONVERT_CUSTOM_TYPES)) as FilterQuery<T>;
}

const op = operator || params as keyof typeof GroupOperator;
Expand Down
2 changes: 1 addition & 1 deletion tests/EntityManager.mongo.test.ts
Expand Up @@ -530,7 +530,7 @@ describe('EntityManagerMongo', () => {

const conn = driver.getConnection();
const ctx = await conn.begin();
const first = await driver.nativeInsert(Publisher.name, { name: 'test 123', type: 'GLOBAL' }, { ctx });
const first = await driver.nativeInsert<Publisher>(Publisher.name, { name: 'test 123', type: PublisherType.GLOBAL }, { ctx });
await conn.commit(ctx);
await driver.nativeUpdate<Publisher>(Publisher.name, first.insertId, { name: 'test 456' });
await driver.nativeUpdateMany<Publisher>(Publisher.name, [first.insertId], [{ name: 'test 789' }]);
Expand Down
2 changes: 1 addition & 1 deletion tests/entities-sql/Car2.ts
Expand Up @@ -19,7 +19,7 @@ export class Car2 {
@ManyToMany(() => User2, u => u.cars)
users = new Collection<User2>(this);

[PrimaryKeyType]: [string, number];
[PrimaryKeyType]?: [string, number];

constructor(name: string, year: number, price: number) {
this.name = name;
Expand Down
2 changes: 1 addition & 1 deletion tests/entities-sql/FooParam2.ts
Expand Up @@ -17,7 +17,7 @@ export class FooParam2 {
@Property({ version: true })
version!: Date;

[PrimaryKeyType]: [number, number];
[PrimaryKeyType]?: [number, number];
[PrimaryKeyProp]?: 'bar' | 'baz';

constructor(bar: FooBar2, baz: FooBaz2, value: string) {
Expand Down
2 changes: 1 addition & 1 deletion tests/entities-sql/User2.ts
Expand Up @@ -23,7 +23,7 @@ export class User2 {
@OneToOne({ entity: () => Car2, nullable: true })
favouriteCar?: Car2;

[PrimaryKeyType]: [string, string];
[PrimaryKeyType]?: [string, string];

constructor(firstName: string, lastName: string) {
this.firstName = firstName;
Expand Down
4 changes: 2 additions & 2 deletions tests/features/composite-keys/GH1079.test.ts
Expand Up @@ -14,7 +14,7 @@ class User {
@Entity()
class Wallet {

[PrimaryKeyType]: [string, string];
[PrimaryKeyType]?: [string, string];

@PrimaryKey()
currencyRef!: string;
Expand Down Expand Up @@ -56,7 +56,7 @@ enum DepositStatus {
@Entity()
export class Deposit extends AbstractDeposit<'status'> {

[PrimaryKeyType]: [string, string, string];
[PrimaryKeyType]?: [string, string, string];

@PrimaryKey()
txRef!: string;
Expand Down
6 changes: 3 additions & 3 deletions tests/features/composite-keys/GH1624.test.ts
Expand Up @@ -52,7 +52,7 @@ export class User {
@OneToMany({ entity: 'UserRole', mappedBy: 'user' })
userRoles = new Collection<UserRole>(this);

[PrimaryKeyType]: [string, string];
[PrimaryKeyType]?: [string, string];

constructor(value: Partial<User> = {}) {
Object.assign(this, value);
Expand Down Expand Up @@ -101,7 +101,7 @@ export class UserRole {
})
role!: IdentifiedReference<Role>;

[PrimaryKeyType]: [string, string, string];
[PrimaryKeyType]?: [string, string, string];

constructor(value: Partial<UserRole> = {}) {
Object.assign(this, value);
Expand Down Expand Up @@ -155,7 +155,7 @@ export class Site {
@Property({ columnType: 'varchar' })
name!: string;

[PrimaryKeyType]: [string, string, string];
[PrimaryKeyType]?: [string, string, string];

constructor(value: Partial<Site> = {}) {
Object.assign(this, value);
Expand Down
2 changes: 1 addition & 1 deletion tests/features/composite-keys/GH1914.test.ts
Expand Up @@ -43,7 +43,7 @@ export class SiteCategory {
@ManyToOne({ entity: () => Category, primary: true })
category!: Category;

[PrimaryKeyType]: [number, number];
[PrimaryKeyType]?: [number, number];

}

Expand Down
6 changes: 3 additions & 3 deletions tests/features/composite-keys/composite-keys.sqlite.test.ts
Expand Up @@ -51,7 +51,7 @@ export class FooParam2 {
@Property({ version: true })
version!: Date;

[PrimaryKeyType]: [number, number];
[PrimaryKeyType]?: [number, number];

constructor(bar: FooBar2, baz: FooBaz2, value: string) {
this.bar = bar;
Expand Down Expand Up @@ -155,7 +155,7 @@ export class Car2 {
@ManyToMany('User2', 'cars')
users = new Collection<User2>(this);

[PrimaryKeyType]: [string, number];
[PrimaryKeyType]?: [string, number];

constructor(name: string, year: number, price: number) {
this.name = name;
Expand Down Expand Up @@ -186,7 +186,7 @@ export class User2 {
@OneToOne({ entity: () => Car2, nullable: true })
favouriteCar?: Car2;

[PrimaryKeyType]: [string, string];
[PrimaryKeyType]?: [string, string];

constructor(firstName: string, lastName: string) {
this.firstName = firstName;
Expand Down
2 changes: 1 addition & 1 deletion tests/features/custom-types/GH446.test.ts
Expand Up @@ -44,7 +44,7 @@ class C {
@OneToOne({ primary: true })
b!: B;

[PrimaryKeyType]: B | A | string;
[PrimaryKeyType]?: B | A | string;

}

Expand Down
2 changes: 1 addition & 1 deletion tests/issues/GH1111.test.ts
Expand Up @@ -14,7 +14,7 @@ class Node {
@Entity()
class A {

[PrimaryKeyType]: number;
[PrimaryKeyType]?: number;
[PrimaryKeyProp]: 'node';
@OneToOne({ entity: () => Node, wrappedReference: true, primary: true, onDelete: 'cascade', onUpdateIntegrity: 'cascade' })
node!: IdentifiedReference<Node>;
Expand Down
2 changes: 1 addition & 1 deletion tests/issues/GH1224.test.ts
Expand Up @@ -26,7 +26,7 @@ class B {
@Entity()
class A {

[PrimaryKeyType]: number;
[PrimaryKeyType]?: number;
[PrimaryKeyProp]: 'node';
@OneToOne({ entity: 'Node', wrappedReference: true, primary: true, onDelete: 'cascade', onUpdateIntegrity: 'cascade' })
node!: IdentifiedReference<Node>;
Expand Down
2 changes: 1 addition & 1 deletion tests/issues/GH1902.test.ts
Expand Up @@ -55,7 +55,7 @@ class UserTenantEntity {
@ManyToOne({ primary: true, entity: () => TenantEntity, fieldName: 'tenantId', cascade: [] })
tenant!: TenantEntity;

[PrimaryKeyType]: [number, number];
[PrimaryKeyType]?: [number, number];
[OptionalProps]?: 'isActive';

@Property({ type: 'boolean', fieldName: 'isActive' })
Expand Down
8 changes: 4 additions & 4 deletions tests/issues/GH2648.test.ts
Expand Up @@ -15,7 +15,7 @@ export class A {
@Entity()
export class B1 {

[PrimaryKeyType]: number;
[PrimaryKeyType]?: number;
@ManyToOne({ entity: () => A, primary: true, wrappedReference: true })
a!: IdentifiedReference<A>;

Expand All @@ -27,7 +27,7 @@ export class B2 {
@PrimaryKey()
id!: number;

[PrimaryKeyType]: number;
[PrimaryKeyType]?: number;
@OneToOne({ entity: () => A, primary: true, wrappedReference: true })
a!: IdentifiedReference<A>;

Expand All @@ -36,7 +36,7 @@ export class B2 {
@Entity()
export class B3 {

[PrimaryKeyType]: number;
[PrimaryKeyType]?: number;
@OneToOne({ entity: () => A, primary: true, wrappedReference: true })
a!: IdentifiedReference<A>;

Expand All @@ -48,7 +48,7 @@ export class B4 {
@PrimaryKey()
id!: number;

[PrimaryKeyType]: number;
[PrimaryKeyType]?: number;
@OneToOne({ entity: () => A, primary: true, wrappedReference: true })
a!: IdentifiedReference<A>;

Expand Down
2 changes: 1 addition & 1 deletion tests/issues/GH529.test.ts
Expand Up @@ -70,7 +70,7 @@ export class OrderItem {
@Property()
offeredPrice: number;

[PrimaryKeyType]: [number, number]; // this is needed for proper type checks in `FilterQuery`
[PrimaryKeyType]?: [number, number]; // this is needed for proper type checks in `FilterQuery`

constructor(order: Order, product: Product, amount = 1) {
this.order = order;
Expand Down

0 comments on commit db0b399

Please sign in to comment.