fix(pinia-orm): $isDirty() in hooks on model had wrong state

Author: CodeDredd

docs/content/1.guide/2.model/5.lifecycle-hooks.md

@@ -45,7 +45,7 @@ class User extends Model {
 Mutation lifecycle hooks are called when you mutate data in the store. The corresponding hook name are `creating`, `updating`, `saving`, `deleting`, `created`, `updated`, `deleted` and `saved`.
 
 When in `creating`, `updating` or `saving`, you can modify the record directly to mutate the data to be saved. You also have access
-to the original data. (Also available in `created`, `updated` and `saved`)
+to the data passed to the model. (Also available in `created`, `updated` and `saved`)
 
 ```js
 class Post extends Model {

packages/pinia-orm/.eslintcache

@@ -799,7 +799,7 @@ export class Model {
     return this
   }
 
-  protected $fillMeta (action = 'save') {
+  public $fillMeta (action = 'save') {
     const timestamp = Math.floor(Date.now() / 1000)
     if (action === 'save') {
       // @ts-expect-error Setting an object

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

@@ -837,17 +837,17 @@ export class Query<M extends Model = Model> {
       const record = elements[id]
       const existing = currentData[id]
       let model = existing
-        ? this.hydrate({ ...existing, ...record }, { operation: 'set', action: 'update' })
+        ? Object.assign(this.hydrate(existing, { operation: 'set', action: 'update' }), record)
         : this.hydrate(record, { operation: 'set', action: 'save' })
 
-      const isSaving = model.$self().saving(model, existing ?? {})
-      const isUpdatingOrCreating = existing ? model.$self().updating(model, existing ?? {}) : model.$self().creating(model, existing ?? {})
+      const isSaving = model.$self().saving(model, record)
+      const isUpdatingOrCreating = existing ? model.$self().updating(model, record) : model.$self().creating(model, record)
       if (isSaving === false || isUpdatingOrCreating === false) { continue }
 
       if (model.$isDirty()) { model = this.hydrate(model.$getAttributes(), { operation: 'set', action: existing ? 'update' : 'save' }) }
 
-      afterSavingHooks.push(() => model.$self().saved(model, existing ?? {}))
-      afterSavingHooks.push(() => existing ? model.$self().updated(model, existing ?? {}) : model.$self().created(model, existing ?? {}))
+      afterSavingHooks.push(() => model.$self().saved(model, record))
+      afterSavingHooks.push(() => existing ? model.$self().updated(model, record) : model.$self().created(model, record))
       newData[id] = model.$getAttributes()
       if (Object.values(model.$types()).length > 0 && !newData[id][model.$typeKey()]) { newData[id][model.$typeKey()] = record[model.$typeKey()] }
     }
@@ -893,9 +893,11 @@ export class Query<M extends Model = Model> {
     if (isEmpty(models)) { return [] }
 
     const newModels = models.map((model) => {
-      const newModel = this.hydrate({ ...model.$getAttributes(), ...record }, { action: 'update', operation: 'set' })
-      if (model.$self().updating(model, record) === false) { return model }
-      newModel.$self().updated(newModel)
+      const oldModelUpdate = Object.assign(this.hydrate(model.$getAttributes(), { action: 'update', operation: 'set' }), record)
+      if (model.$self().updating(oldModelUpdate, record) === false) { return model }
+
+      const newModel = oldModelUpdate.$isDirty() ? this.hydrate({ ...model.$getAttributes(), ...record }, { action: 'update', operation: 'set' }) : oldModelUpdate
+      newModel.$self().updated(newModel, record)
       return newModel
     })
 

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

@@ -1,6 +1,5 @@
 import { describe, expect, it, vi } from 'vitest'
 
-import type { Element } from '../../../src'
 import { BelongsTo as BelongsToClass, Model, useRepo } from '../../../src'
 import { Attr, BelongsTo, Num, Str, Uid } from '../../../src/decorators'
 import { assertState, mockUid } from '../../helpers'
@@ -38,11 +37,10 @@ describe('feature/hooks/saving', () => {
       @Num(0) declare age: number
       @BelongsTo(() => Post, 'postId') declare post: User | null
 
-      static saving (model: Model, record?: Element) {
-        expect(model.name).not.toBe(record.name)
+      static saving (model: User) {
         const fields = model.$fields()
         for (const name in fields) {
-          if (fields[name] instanceof BelongsToClass && record && record.name === 'John Doe') { model[(fields[name] as BelongsToClass).foreignKey] = null }
+          if (fields[name] instanceof BelongsToClass && model.name === 'John') { model[(fields[name] as BelongsToClass).foreignKey] = null }
         }
       }
     }

packages/pinia-orm/tests/feature/hooks/saving.spec.ts

@@ -49,6 +49,7 @@ describe('feature/hooks/updating', () => {
       @Num(0) age!: number
 
       static updating (model: Model) {
+        expect(model.$isDirty()).toBe(true)
         model.name = 'John'
       }
     }
@@ -83,7 +84,7 @@ describe('feature/hooks/updating', () => {
     })
   })
 
-  it('is stopping record to be saved', () => {
+  it('is not dirty if same value is set', () => {
     class User extends Model {
       static entity = 'users'
 
@@ -92,7 +93,40 @@ describe('feature/hooks/updating', () => {
       @Num(0) age!: number
 
       static updating (model: Model) {
+        expect(model.$isDirty()).toBe(false)
         model.name = 'John'
+      }
+    }
+
+    const updatingMethod = vi.spyOn(User, 'updating')
+
+    fillState({
+      users: {
+        1: { id: 1, name: 'John Doe', age: 10 },
+        2: { id: 2, name: 'John Doe', age: 10 },
+      },
+    })
+    useRepo(User).where('id', 1).update({ age: 10 })
+
+    expect(updatingMethod).toHaveBeenCalledTimes(1)
+
+    assertState({
+      users: {
+        1: { id: 1, name: 'John Doe', age: 10 },
+        2: { id: 2, name: 'John Doe', age: 10 },
+      },
+    })
+  })
+
+  it('is stopping record to be saved', () => {
+    class User extends Model {
+      static entity = 'users'
+
+      @Num(0) id!: number
+      @Str('') name!: string
+      @Num(0) age!: number
+
+      static updating () {
         return false
       }
     }
@@ -109,6 +143,10 @@ describe('feature/hooks/updating', () => {
 
     expect(updatingMethod).toHaveBeenCalledOnce()
 
+    useRepo(User).where('id', 1).update({ name: 'John Doe', age: 30 })
+
+    expect(updatingMethod).toHaveBeenCalled(2)
+
     assertState({
       users: {
         1: { id: 1, name: 'John', age: 50 },

packages/pinia-orm/tests/feature/hooks/updating.spec.ts

@@ -46,14 +46,32 @@ describe('performance/prevent_rerender_of_child_components', () => {
         })
       }
 
+      const updatePost = () => {
+        postRepo.where('id', 1).update({
+          id: 1,
+          title: `Test 1001`,
+        })
+      }
+
+      const savePost = () => {
+        postRepo.save({
+          id: 1,
+          title: `Test 1111`,
+        })
+      }
+
       return {
         posts,
         addPost,
+        updatePost,
+        savePost,
       }
     },
     template: `
     <div>
-    <button @click="addPost" > Click me </button>
+    <button id='insert' @click="addPost" > Click me </button>
+    <button id='update' @click="updatePost" > Update 1 </button>
+    <button id='save' @click="savePost" > Update 2 </button>
     <post-component v-for="post in posts" :post="post" :key="post.id" />
     </div>`,
   })
@@ -80,11 +98,11 @@ describe('performance/prevent_rerender_of_child_components', () => {
 
     const logger = vi.spyOn(console, 'warn')
 
-    await wrapper.find('button').trigger('click')
+    await wrapper.find('button#insert').trigger('click')
     await nextTick()
-    await wrapper.find('button').trigger('click')
+    await wrapper.find('button#insert').trigger('click')
     await nextTick()
-    await wrapper.find('button').trigger('click')
+    await wrapper.find('button#insert').trigger('click')
     await nextTick()
 
     expect(wrapper.html()).toContain('Test 1')
@@ -93,5 +111,11 @@ describe('performance/prevent_rerender_of_child_components', () => {
     expect(wrapper.html()).toContain('Test 13')
 
     expect(logger).not.toBeCalled()
+
+    await wrapper.find('button#update').trigger('click')
+    await nextTick()
+
+    await wrapper.find('button#save').trigger('click')
+    await nextTick()
   })
 })