Skip to content

Commit a5812ac

Browse files
committed
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
1 parent 7a32aec commit a5812ac

5 files changed

Lines changed: 49 additions & 15 deletions

File tree

packages/pinia-orm/src/model/Model.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,7 @@ export interface InheritanceTypes {
5959
[key: string]: typeof Model
6060
}
6161

62-
export type WithKeys<T> = { [P in keyof T]: T[P] extends (Model | null) | Model[] ? P & string : never }[keyof T]
63-
// export type WithKeys<T> = { [P in keyof T]: T[P] extends Model[] ? P : never }[keyof T];
62+
export type WithKeys<T> = { [P in keyof T]-?: NonNullable<T[P]> extends Model | Model[] ? P & string : never }[keyof T]
6463

6564
export class Model {
6665
declare _meta: undefined | MetaValues

packages/pinia-orm/src/query/Options.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ export interface Where<T = Model> {
1010
// eslint-disable-next-line @typescript-eslint/no-unsafe-function-type
1111
export type NonMethodKeys<T> = { [P in keyof T]: T[P] extends Function ? never : P }[keyof T]
1212
export type GetElementType<T extends unknown[] | unknown> = T extends (infer U)[] ? U : T
13-
export type UltimateKeys<M> = { [T in keyof M]: M[T] extends Model | Model[] | null ? GetElementType<NonNullable<M[T]>> : never }
13+
export type UltimateKeys<M> = { [T in keyof M]-?: NonNullable<M[T]> extends Model | Model[] ? GetElementType<NonNullable<M[T]>> : never }
1414
export type WherePrimaryClosure<T> = (model: T) => boolean
1515

1616
export type WhereSecondaryClosure<T> = (value: T) => boolean

packages/pinia-orm/src/query/Query.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -292,14 +292,14 @@ export class Query<M extends Model = Model> {
292292
/**
293293
* Add a "where has" clause to the query.
294294
*/
295-
whereHas<T extends WithKeys<M>>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null ? EagerLoadConstraint<GetElementType<NonNullable<M[T]>>> : () => void = () => { }, operator?: string | number, count?: number): this {
295+
whereHas<T extends WithKeys<M>>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null | undefined ? EagerLoadConstraint<GetElementType<NonNullable<M[T]>>> : () => void = () => { }, operator?: string | number, count?: number): this {
296296
return this.where(this.getFieldWhereForRelations(relation, callback, operator, count))
297297
}
298298

299299
/**
300300
* Add an "or where has" clause to the query.
301301
*/
302-
orWhereHas<T extends WithKeys<M>>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null ? EagerLoadConstraint<GetElementType<NonNullable<M[T]>>> : () => void = () => { }, operator?: string | number, count?: number): this {
302+
orWhereHas<T extends WithKeys<M>>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null | undefined ? EagerLoadConstraint<GetElementType<NonNullable<M[T]>>> : () => void = () => { }, operator?: string | number, count?: number): this {
303303
return this.orWhere(this.getFieldWhereForRelations(relation, callback, operator, count))
304304
}
305305

@@ -334,14 +334,14 @@ export class Query<M extends Model = Model> {
334334
/**
335335
* Add a "where doesn't have" clause to the query.
336336
*/
337-
whereDoesntHave<T extends WithKeys<M>>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null ? EagerLoadConstraint<GetElementType<NonNullable<M[T]>>> : () => void = () => { }): this {
337+
whereDoesntHave<T extends WithKeys<M>>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null | undefined ? EagerLoadConstraint<GetElementType<NonNullable<M[T]>>> : () => void = () => { }): this {
338338
return this.where(this.getFieldWhereForRelations(relation, callback, '=', 0))
339339
}
340340

341341
/**
342342
* Add an "or where doesn't have" clause to the query.
343343
*/
344-
orWhereDoesntHave<T extends WithKeys<M>>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null ? EagerLoadConstraint<GetElementType<NonNullable<M[T]>>> : () => void = () => { }): this {
344+
orWhereDoesntHave<T extends WithKeys<M>>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null | undefined ? EagerLoadConstraint<GetElementType<NonNullable<M[T]>>> : () => void = () => { }): this {
345345
return this.orWhere(this.getFieldWhereForRelations(relation, callback, '=', 0))
346346
}
347347

@@ -386,7 +386,7 @@ export class Query<M extends Model = Model> {
386386
/**
387387
* Set the relationships that should be eager loaded.
388388
*/
389-
with<T extends WithKeys<M>>(name: T | string & {}, callback: M[T] extends Model | Model[] | null ? EagerLoadConstraint<GetElementType<NonNullable<M[T]>>> : () => void = () => { }): this {
389+
with<T extends WithKeys<M>>(name: T | string & {}, callback: M[T] extends Model | Model[] | null | undefined ? EagerLoadConstraint<GetElementType<NonNullable<M[T]>>> : () => void = () => { }): this {
390390
this.getNewHydrated = true
391391
// @ts-expect-error name type can be used
392392
this.eagerLoad[name] = callback
@@ -434,7 +434,7 @@ export class Query<M extends Model = Model> {
434434
/**
435435
* Get where closure for relations
436436
*/
437-
protected getFieldWhereForRelations<T extends WithKeys<M>>(relation: T | (string & {}), callback: M[T] extends Model | Model[] | null ? EagerLoadConstraint<GetElementType<NonNullable<M[T]>>> : () => void = () => { }, operator?: string | number, count?: number): WherePrimaryClosure<M> {
437+
protected getFieldWhereForRelations<T extends WithKeys<M>>(relation: T | (string & {}), callback: M[T] extends Model | Model[] | null | undefined ? EagerLoadConstraint<GetElementType<NonNullable<M[T]>>> : () => void = () => { }, operator?: string | number, count?: number): WherePrimaryClosure<M> {
438438
const modelIdsByRelation = this.newQuery(this.model.$entity()).with(relation, callback).get(false)
439439
.filter((model) => {
440440
const modelRelation = model[relation as T]

packages/pinia-orm/src/repository/Repository.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -248,14 +248,14 @@ export class Repository<M extends Model = Model> {
248248
/**
249249
* Add a "where has" clause to the query.
250250
*/
251-
whereHas<T extends WithKeys<M>>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null ? EagerLoadConstraint<GetElementType<NonNullable<M[T]>>> : () => void = () => { }, operator?: string | number, count?: number): Query<M> {
251+
whereHas<T extends WithKeys<M>>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null | undefined ? EagerLoadConstraint<GetElementType<NonNullable<M[T]>>> : () => void = () => { }, operator?: string | number, count?: number): Query<M> {
252252
return this.query().whereHas<T>(relation, callback, operator, count)
253253
}
254254

255255
/**
256256
* Add an "or where has" clause to the query.
257257
*/
258-
orWhereHas<T extends WithKeys<M>>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null ? EagerLoadConstraint<GetElementType<NonNullable<M[T]>>> : () => void = () => { }, operator?: string | number, count?: number): Query<M> {
258+
orWhereHas<T extends WithKeys<M>>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null | undefined ? EagerLoadConstraint<GetElementType<NonNullable<M[T]>>> : () => void = () => { }, operator?: string | number, count?: number): Query<M> {
259259
return this.query().orWhereHas(relation, callback, operator, count)
260260
}
261261

@@ -290,14 +290,14 @@ export class Repository<M extends Model = Model> {
290290
/**
291291
* Add a "where doesn't have" clause to the query.
292292
*/
293-
whereDoesntHave<T extends WithKeys<M>>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null ? EagerLoadConstraint<GetElementType<NonNullable<M[T]>>> : () => void = () => { }): Query<M> {
293+
whereDoesntHave<T extends WithKeys<M>>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null | undefined ? EagerLoadConstraint<GetElementType<NonNullable<M[T]>>> : () => void = () => { }): Query<M> {
294294
return this.query().whereDoesntHave(relation, callback)
295295
}
296296

297297
/**
298298
* Add an "or where doesn't have" clause to the query.
299299
*/
300-
orWhereDoesntHave<T extends WithKeys<M>>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null ? EagerLoadConstraint<GetElementType<NonNullable<M[T]>>> : () => void = () => { }): Query<M> {
300+
orWhereDoesntHave<T extends WithKeys<M>>(relation: T | string & {}, callback: M[T] extends Model | Model[] | null | undefined ? EagerLoadConstraint<GetElementType<NonNullable<M[T]>>> : () => void = () => { }): Query<M> {
301301
return this.query().orWhereDoesntHave(relation, callback)
302302
}
303303

@@ -353,7 +353,7 @@ export class Repository<M extends Model = Model> {
353353
/**
354354
* Set the relationships that should be eager loaded.
355355
*/
356-
with<T extends WithKeys<M>>(name: string & {} | T, callback?: M[T] extends Model | Model[] | null ? EagerLoadConstraint<GetElementType<NonNullable<M[T]>>> : never): Query<M> {
356+
with<T extends WithKeys<M>>(name: string & {} | T, callback?: M[T] extends Model | Model[] | null | undefined ? EagerLoadConstraint<GetElementType<NonNullable<M[T]>>> : never): Query<M> {
357357
return this.query().with(name, callback)
358358
}
359359

packages/pinia-orm/tests/feature/relations/constraints/constraints.spec.ts

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { describe, expect, it } from 'vitest'
22

33
import { Model, useRepo } from '../../../../src'
4-
import { Attr, BelongsToMany, HasOne, Num, Str } from '../../../../src/decorators'
4+
import { Attr, BelongsToMany, HasMany, HasOne, Num, Str } from '../../../../src/decorators'
55

66
describe('feature/relations/constraints/constraints', () => {
77
class Type extends Model {
@@ -153,4 +153,39 @@ describe('feature/relations/constraints/constraints', () => {
153153
expect(users[0].roles.length).toBe(3)
154154
expect(users2[0].roles).toBe(undefined)
155155
})
156+
157+
it('can add constraints to a relationship declared as optional', () => {
158+
class Comment extends Model {
159+
static entity = 'comments'
160+
161+
@Attr() declare id: number
162+
@Attr() declare postId: number
163+
@Str('') declare body: string
164+
}
165+
166+
class Post extends Model {
167+
static entity = 'posts'
168+
169+
@Attr() declare id: number
170+
@Str('') declare title: string
171+
172+
@HasMany(() => Comment, 'postId')
173+
declare comments?: Comment[]
174+
}
175+
176+
const postsRepo = useRepo(Post)
177+
178+
postsRepo.save([
179+
{ id: 1, title: 'A', comments: [{ id: 1, body: 'first' }, { id: 2, body: 'second' }] },
180+
])
181+
182+
const post = postsRepo
183+
.with('comments', (query) => {
184+
query.where('body', 'second')
185+
})
186+
.first()
187+
188+
expect(post?.comments?.length).toBe(1)
189+
expect(post?.comments?.[0].body).toBe('second')
190+
})
156191
})

0 commit comments

Comments
 (0)