Avoid syncing dirty props on store update

This is probably a better default, following principle of least
surprise, unless we hear otherwise. If you load instanceA, and 1m later
load instanceB with new data from the server, instanceA will change. But
if instanceA has already updated the relevant attribute (it is dirty),
avoid making that change.

Relatedly, if instanceA syncs with a new store update, it would see that
property as dirty, as it didn't match the attributes it was instantiated
with. This commit also ensures synced properties are not seen as dirty.

Author: Lee Richmond

.gitignore

@@ -9,3 +9,5 @@ build/
 test-old/
 dist/
 coverage/
+lib/
+lib-esm/

src/model.ts

@@ -439,14 +439,20 @@ export class JSORMBase {
     if (this._onStoreChange) return this._onStoreChange
     this._onStoreChange = (event, attrs) => {
       let diff = {} as any
+      // Update all non-dirty attributes
       Object.keys(attrs).forEach(k => {
         let self = this as any
-        if (self[k] !== attrs[k]) {
+        let changes = this.changes() as any
+        if (self[k] !== attrs[k] && !changes[k]) {
           diff[k] = [self[k], attrs[k]]
           self[k] = attrs[k]
+
+          // ensure this property is not marked as dirty
+          self._originalAttributes[k] = attrs[k]
         }
       })
 
+      // fire afterSync hook if applicable
       let hasDiff = Object.keys(diff).length > 0
       let hasAfterSync = typeof this.afterSync !== "undefined"
       if (hasDiff && hasAfterSync) this.afterSync(diff)

test/integration/id-map.test.ts

@@ -6,18 +6,19 @@ afterEach(() => {
   ApplicationRecord.store.data = {}
 })
 
+let responsePayload = (firstName: string) => {
+  return {
+    data: {
+      id: "1",
+      type: "authors",
+      attributes: { firstName }
+    }
+  }
+}
+
 describe("ID Map", () => {
   beforeEach(() => {
     ApplicationRecord.sync = true
-    let responsePayload = (firstName: string) => {
-      return {
-        data: {
-          id: "1",
-          type: "authors",
-          attributes: { firstName }
-        }
-      }
-    }
 
     fetchMock.get(
       "http://example.com/api/v1/authors/1",
@@ -38,7 +39,7 @@ describe("ID Map", () => {
   })
 
   afterEach(() => {
-    fetchMock.reset()
+    fetchMock.restore()
   })
 
   describe("when fetching from server", () => {
@@ -61,36 +62,71 @@ describe("ID Map", () => {
       expect(stored["authors-1"]).to.deep.eq(data.attributes)
     })
 
-    it("syncs with id map", async () => {
+    it("syncs non-dirty attributes with id map", async () => {
       let author1 = (await Author.find(1)).data
-      author1.firstName = "updated"
+      expect(author1.firstName).to.eq("John")
+
+      fetchMock.restore()
+      fetchMock.get(
+        "http://example.com/api/v1/authors/1",
+        responsePayload("Jane")
+      )
+
       let author2 = (await Author.find(1)).data
+      expect(author2.firstName).to.eq("Jane")
+      expect(author1.firstName).to.eq("Jane")
+    })
+
+    it("does not see mark newly-synced attributes as dirty", async () => {
+      let author1 = (await Author.find(1)).data
       expect(author1.firstName).to.eq("John")
+
+      fetchMock.restore()
+      fetchMock.get(
+        "http://example.com/api/v1/authors/1",
+        responsePayload("Jane")
+      )
+
+      await Author.find(1)
+      expect(author1.firstName).to.eq("Jane")
+      expect(author1.isDirty()).to.eq(false)
     })
 
-    describe("when implementing afterSync hook", () => {
-      let originalAfterSync: any
+    it("does not sync dirty attributes with id map", async () => {
+      let author1 = (await Author.find(1)).data
+      author1.firstName = "updated"
 
-      beforeEach(() => {
-        originalAfterSync = Author.prototype.afterSync
-        Author.prototype.afterSync = sinon.spy()
-      })
+      fetchMock.restore()
+      fetchMock.get(
+        "http://example.com/api/v1/authors/1",
+        responsePayload("Jane")
+      )
 
-      afterEach(() => {
-        Author.prototype.afterSync = originalAfterSync
-      })
+      let author2 = (await Author.find(1)).data
+      expect(author2.firstName).to.eq("Jane")
+      expect(author1.firstName).to.eq("updated")
+    })
 
+    describe("when implementing afterSync hook", () => {
       it("fires after sync, passing changes as an argument", async() => {
         let author1 = (await Author.find(1)).data
-        author1.firstName = 'updated'
+        author1.afterSync = sinon.spy()
+
+        fetchMock.restore()
+        fetchMock.get(
+          "http://example.com/api/v1/authors/1",
+          responsePayload("Jane")
+        )
+
         let author2 = (await Author.find(1)).data
         expect(author1.afterSync).to
-          .have.been.calledWith({ firstName: ["updated", "John"] })
+          .have.been.calledWith({ firstName: ["John", "Jane"] })
       })
 
       describe("when store updates, but no changes", () => {
         it("does not fire the hook", async() => {
           let author1 = (await Author.find(1)).data
+          author1.afterSync = sinon.spy()
           let author2 = (await Author.find(1)).data
           let called = (<any>author1.afterSync).called
           expect(called).to.eq(false)