From a5812ac92ba3846eae36783b817df14832bfcf9d Mon Sep 17 00:00:00 2001 From: Gregor Becker Date: Sun, 19 Jul 2026 23:27:12 +0200 Subject: [PATCH] fix(pinia-orm): accept eager load callback for optional relations Relations declared with TypeScript optional properties (posts?: Post[]) were not covered by WithKeys and the eager load callback conditionals, because the property type includes undefined. The callback of with(), whereHas() etc. then resolved to '() => void'/'never' and passing a query constraint produced a compile error. fixes #1999 --- packages/pinia-orm/src/model/Model.ts | 3 +- packages/pinia-orm/src/query/Options.ts | 2 +- packages/pinia-orm/src/query/Query.ts | 12 +++--- .../pinia-orm/src/repository/Repository.ts | 10 ++--- .../relations/constraints/constraints.spec.ts | 37 ++++++++++++++++++- 5 files changed, 49 insertions(+), 15 deletions(-) diff --git a/packages/pinia-orm/src/model/Model.ts b/packages/pinia-orm/src/model/Model.ts index d7a45b67b..8b1c246a7 100644 --- a/packages/pinia-orm/src/model/Model.ts +++ b/packages/pinia-orm/src/model/Model.ts @@ -59,8 +59,7 @@ export interface InheritanceTypes { [key: string]: typeof Model } -export type WithKeys = { [P in keyof T]: T[P] extends (Model | null) | Model[] ? P & string : never }[keyof T] -// export type WithKeys = { [P in keyof T]: T[P] extends Model[] ? P : never }[keyof T]; +export type WithKeys = { [P in keyof T]-?: NonNullable extends Model | Model[] ? P & string : never }[keyof T] export class Model { declare _meta: undefined | MetaValues diff --git a/packages/pinia-orm/src/query/Options.ts b/packages/pinia-orm/src/query/Options.ts index 0b9932c19..5bd12ac90 100644 --- a/packages/pinia-orm/src/query/Options.ts +++ b/packages/pinia-orm/src/query/Options.ts @@ -10,7 +10,7 @@ export interface Where { // eslint-disable-next-line @typescript-eslint/no-unsafe-function-type export type NonMethodKeys = { [P in keyof T]: T[P] extends Function ? never : P }[keyof T] export type GetElementType = T extends (infer U)[] ? U : T -export type UltimateKeys = { [T in keyof M]: M[T] extends Model | Model[] | null ? GetElementType> : never } +export type UltimateKeys = { [T in keyof M]-?: NonNullable extends Model | Model[] ? GetElementType> : never } export type WherePrimaryClosure = (model: T) => boolean export type WhereSecondaryClosure = (value: T) => boolean diff --git a/packages/pinia-orm/src/query/Query.ts b/packages/pinia-orm/src/query/Query.ts index 1784529ea..da983d75b 100644 --- a/packages/pinia-orm/src/query/Query.ts +++ b/packages/pinia-orm/src/query/Query.ts @@ -292,14 +292,14 @@ export class Query { /** * Add a "where has" clause to the query. */ - whereHas>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null ? EagerLoadConstraint>> : () => void = () => { }, operator?: string | number, count?: number): this { + whereHas>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null | undefined ? EagerLoadConstraint>> : () => void = () => { }, operator?: string | number, count?: number): this { return this.where(this.getFieldWhereForRelations(relation, callback, operator, count)) } /** * Add an "or where has" clause to the query. */ - orWhereHas>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null ? EagerLoadConstraint>> : () => void = () => { }, operator?: string | number, count?: number): this { + orWhereHas>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null | undefined ? EagerLoadConstraint>> : () => void = () => { }, operator?: string | number, count?: number): this { return this.orWhere(this.getFieldWhereForRelations(relation, callback, operator, count)) } @@ -334,14 +334,14 @@ export class Query { /** * Add a "where doesn't have" clause to the query. */ - whereDoesntHave>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null ? EagerLoadConstraint>> : () => void = () => { }): this { + whereDoesntHave>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null | undefined ? EagerLoadConstraint>> : () => void = () => { }): this { return this.where(this.getFieldWhereForRelations(relation, callback, '=', 0)) } /** * Add an "or where doesn't have" clause to the query. */ - orWhereDoesntHave>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null ? EagerLoadConstraint>> : () => void = () => { }): this { + orWhereDoesntHave>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null | undefined ? EagerLoadConstraint>> : () => void = () => { }): this { return this.orWhere(this.getFieldWhereForRelations(relation, callback, '=', 0)) } @@ -386,7 +386,7 @@ export class Query { /** * Set the relationships that should be eager loaded. */ - with>(name: T | string & {}, callback: M[T] extends Model | Model[] | null ? EagerLoadConstraint>> : () => void = () => { }): this { + with>(name: T | string & {}, callback: M[T] extends Model | Model[] | null | undefined ? EagerLoadConstraint>> : () => void = () => { }): this { this.getNewHydrated = true // @ts-expect-error name type can be used this.eagerLoad[name] = callback @@ -434,7 +434,7 @@ export class Query { /** * Get where closure for relations */ - protected getFieldWhereForRelations>(relation: T | (string & {}), callback: M[T] extends Model | Model[] | null ? EagerLoadConstraint>> : () => void = () => { }, operator?: string | number, count?: number): WherePrimaryClosure { + protected getFieldWhereForRelations>(relation: T | (string & {}), callback: M[T] extends Model | Model[] | null | undefined ? EagerLoadConstraint>> : () => void = () => { }, operator?: string | number, count?: number): WherePrimaryClosure { const modelIdsByRelation = this.newQuery(this.model.$entity()).with(relation, callback).get(false) .filter((model) => { const modelRelation = model[relation as T] diff --git a/packages/pinia-orm/src/repository/Repository.ts b/packages/pinia-orm/src/repository/Repository.ts index 8d4bd7617..e7b7684d2 100644 --- a/packages/pinia-orm/src/repository/Repository.ts +++ b/packages/pinia-orm/src/repository/Repository.ts @@ -248,14 +248,14 @@ export class Repository { /** * Add a "where has" clause to the query. */ - whereHas>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null ? EagerLoadConstraint>> : () => void = () => { }, operator?: string | number, count?: number): Query { + whereHas>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null | undefined ? EagerLoadConstraint>> : () => void = () => { }, operator?: string | number, count?: number): Query { return this.query().whereHas(relation, callback, operator, count) } /** * Add an "or where has" clause to the query. */ - orWhereHas>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null ? EagerLoadConstraint>> : () => void = () => { }, operator?: string | number, count?: number): Query { + orWhereHas>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null | undefined ? EagerLoadConstraint>> : () => void = () => { }, operator?: string | number, count?: number): Query { return this.query().orWhereHas(relation, callback, operator, count) } @@ -290,14 +290,14 @@ export class Repository { /** * Add a "where doesn't have" clause to the query. */ - whereDoesntHave>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null ? EagerLoadConstraint>> : () => void = () => { }): Query { + whereDoesntHave>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null | undefined ? EagerLoadConstraint>> : () => void = () => { }): Query { return this.query().whereDoesntHave(relation, callback) } /** * Add an "or where doesn't have" clause to the query. */ - orWhereDoesntHave>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null ? EagerLoadConstraint>> : () => void = () => { }): Query { + orWhereDoesntHave>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null | undefined ? EagerLoadConstraint>> : () => void = () => { }): Query { return this.query().orWhereDoesntHave(relation, callback) } @@ -353,7 +353,7 @@ export class Repository { /** * Set the relationships that should be eager loaded. */ - with>(name: string & {} | T, callback?: M[T] extends Model | Model[] | null ? EagerLoadConstraint>> : never): Query { + with>(name: string & {} | T, callback?: M[T] extends Model | Model[] | null | undefined ? EagerLoadConstraint>> : never): Query { return this.query().with(name, callback) } diff --git a/packages/pinia-orm/tests/feature/relations/constraints/constraints.spec.ts b/packages/pinia-orm/tests/feature/relations/constraints/constraints.spec.ts index 023cb5cee..6fe8a9a22 100644 --- a/packages/pinia-orm/tests/feature/relations/constraints/constraints.spec.ts +++ b/packages/pinia-orm/tests/feature/relations/constraints/constraints.spec.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from 'vitest' import { Model, useRepo } from '../../../../src' -import { Attr, BelongsToMany, HasOne, Num, Str } from '../../../../src/decorators' +import { Attr, BelongsToMany, HasMany, HasOne, Num, Str } from '../../../../src/decorators' describe('feature/relations/constraints/constraints', () => { class Type extends Model { @@ -153,4 +153,39 @@ describe('feature/relations/constraints/constraints', () => { expect(users[0].roles.length).toBe(3) expect(users2[0].roles).toBe(undefined) }) + + it('can add constraints to a relationship declared as optional', () => { + class Comment extends Model { + static entity = 'comments' + + @Attr() declare id: number + @Attr() declare postId: number + @Str('') declare body: string + } + + class Post extends Model { + static entity = 'posts' + + @Attr() declare id: number + @Str('') declare title: string + + @HasMany(() => Comment, 'postId') + declare comments?: Comment[] + } + + const postsRepo = useRepo(Post) + + postsRepo.save([ + { id: 1, title: 'A', comments: [{ id: 1, body: 'first' }, { id: 2, body: 'second' }] }, + ]) + + const post = postsRepo + .with('comments', (query) => { + query.where('body', 'second') + }) + .first() + + expect(post?.comments?.length).toBe(1) + expect(post?.comments?.[0].body).toBe('second') + }) })