Deprecate passing non ember data records to unloadRecord and deleteRe…

…cord
emberjs · Aug 8, 2021 · 399ff63 · 399ff63
1 parent 0a0157b
commit 399ff63
Show file tree

Hide file tree

Showing 3 changed files with 87 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
@@ -9,6 +9,7 @@ import { all, resolve } from 'rsvp';
 import { setupTest } from 'ember-qunit';
 
 import JSONAPIAdapter from '@ember-data/adapter/json-api';
+import { CUSTOM_MODEL_CLASS } from '@ember-data/canary-features';
 import Model, { attr, belongsTo, hasMany } from '@ember-data/model';
 import JSONAPISerializer from '@ember-data/serializer/json-api';
 import { recordDataFor } from '@ember-data/store/-private';
@@ -2511,4 +2512,22 @@ module('integration/unload - Unloading Records', function (hooks) {
 
     return run(() => store.findRecord('person', 1, { backgroundReload: true }).then((person) => person.unloadRecord()));
   });
+
+  if (CUSTOM_MODEL_CLASS) {
+    test('unloading a non store managed record is deprecated', function (assert) {
+      let didUnload = false;
+      let randomRecord = {
+        unloadRecord() {
+          didUnload = true;
+        },
+      };
+      assert.expectDeprecation(
+        () => {
+          store.unloadRecord(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
@@ -9,6 +9,7 @@ import DS from 'ember-data';
 import { setupTest } from 'ember-qunit';
 
 import RESTAdapter from '@ember-data/adapter/rest';
+import { CUSTOM_MODEL_CLASS } from '@ember-data/canary-features';
 import JSONAPISerializer from '@ember-data/serializer/json-api';
 import RESTSerializer from '@ember-data/serializer/rest';
 import deepCopy from '@ember-data/unpublished-test-infra/test-support/deep-copy';
@@ -1100,6 +1101,25 @@ module('integration/store - deleteRecord', function (hooks) {
     assert.ok(person.isDeleted, 'expect person to be isDeleted');
   });
 
+  if (CUSTOM_MODEL_CLASS) {
+    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();