Deprecate passing non ember data records to unloadRecord and deleteRe…

…cord
emberjs · Aug 6, 2021 · 977eb6c · 977eb6c
1 parent 0a0157b
commit 977eb6c
Show file tree

Hide file tree

Showing 3 changed files with 81 additions and 9 deletions.
diff --git a/packages/-ember-data/tests/integration/records/unload-test.js b/packages/-ember-data/tests/integration/records/unload-test.js
@@ -2511,4 +2511,20 @@ module('integration/unload - Unloading Records', function (hooks) {
 
     return run(() => store.findRecord('person', 1, { backgroundReload: true }).then((person) => person.unloadRecord()));
   });
+
+  test('unloading a non store managed record is deprecated', function (assert) {
+    let didUnload = false;
+    let randomRecord = {
+      unloadRecord() {
+        didUnload = true;
+      },
+    };
+    assert.expectDeprecation(
+      () => {
+        store.didUnload(randomRecord);
+      },
+      { id: 'ember-data:unload-record-non-store' }
+    );
+    assert.ok(didUnload, 'Did unload a non store record');
+  });
 });
diff --git a/packages/-ember-data/tests/integration/store-test.js b/packages/-ember-data/tests/integration/store-test.js
@@ -1100,6 +1100,23 @@ module('integration/store - deleteRecord', function (hooks) {
     assert.ok(person.isDeleted, 'expect person to be isDeleted');
   });
 
+  test('deleting a non store managed record is deprecated', function (assert) {
+    let store = this.owner.lookup('service:store');
+    let didDelete = false;
+    let randomRecord = {
+      deleteRecord() {
+        didDelete = true;
+      },
+    };
+    assert.expectDeprecation(
+      () => {
+        store.deleteRecord(randomRecord);
+      },
+      { id: 'ember-data:delete-record-non-store' }
+    );
+    assert.ok(didDelete, 'Did delete a non store record');
+  });
+
   test('Store should accept a null value for `data`', function (assert) {
     assert.expect(0);
 

diff --git a/packages/store/addon/-private/system/core-store.ts b/packages/store/addon/-private/system/core-store.ts
@@ -56,7 +56,12 @@ import NotificationManager from './record-notification-manager';
 import { RecordReference } from './references';
 import { _bind, _guard, _objectIsAlive, guardDestroyedStore } from './store/common';
 import { _find, _findAll, _findBelongsTo, _findHasMany, _findMany, _query, _queryRecord } from './store/finders';
-import { internalModelFactoryFor, recordIdentifierFor, setRecordIdentifier } from './store/internal-model-factory';
+import {
+  internalModelFactoryFor,
+  peekRecordIdentifier,
+  recordIdentifierFor,
+  setRecordIdentifier,
+} from './store/internal-model-factory';
 import RecordDataStoreWrapper from './store/record-data-store-wrapper';
 import { normalizeResponseHelper } from './store/serializer-response';
 
@@ -713,10 +718,27 @@ abstract class CoreStore extends Service {
     }
     this._backburner.join(() => {
       if (CUSTOM_MODEL_CLASS) {
-        let identifier = recordIdentifierFor(record);
-        let internalModel = internalModelFactoryFor(this).peek(identifier);
-        if (internalModel) {
-          internalModel.deleteRecord();
+        let identifier = peekRecordIdentifier(record);
+        if (identifier) {
+          let internalModel = internalModelFactoryFor(this).peek(identifier);
+          if (internalModel) {
+            internalModel.deleteRecord();
+          }
+        } else {
+          deprecate(
+            `You passed a non ember-data managed record ${record} to store.deleteRecord. Ember Data store is not meant to manage non store records. This is not supported and will be removed`,
+            false,
+            {
+              id: 'ember-data:delete-record-non-store',
+              until: '4.0',
+              for: '@ember-data/store',
+              since: {
+                available: '3.28',
+                enabled: '3.28',
+              },
+            }
+          );
+          record.deleteRecord();
         }
       } else {
         record.deleteRecord();
@@ -745,10 +767,27 @@ abstract class CoreStore extends Service {
       assertDestroyingStore(this, 'unloadRecord');
     }
     if (CUSTOM_MODEL_CLASS) {
-      let identifier = recordIdentifierFor(record);
-      let internalModel = internalModelFactoryFor(this).peek(identifier);
-      if (internalModel) {
-        internalModel.unloadRecord();
+      let identifier = peekRecordIdentifier(record);
+      if (identifier) {
+        let internalModel = internalModelFactoryFor(this).peek(identifier);
+        if (internalModel) {
+          internalModel.unloadRecord();
+        }
+      } else {
+        deprecate(
+          `You passed a non ember-data managed record ${record} to store.unloadRecord. Ember Data store is not meant to manage non store records. This is not supported and will be removed`,
+          false,
+          {
+            id: 'ember-data:unload-record-non-store',
+            until: '4.0',
+            for: '@ember-data/store',
+            since: {
+              available: '3.28',
+              enabled: '3.28',
+            },
+          }
+        );
+        record.unloadRecord();
       }
     } else {
       record.unloadRecord();