Merge pull request #16692 from lupestro/deprecate-copy-copyable

[RFC 322] Implemented copy/Copyable deprecation

packages/ember-routing/lib/system/route.js

@@ -6,14 +6,7 @@ import { get, set, getProperties, setProperties, computed, isEmpty } from 'ember
 import { assert, deprecate, info, isTesting } from '@ember/debug';
 import { DEBUG } from '@glimmer/env';
 import { classify } from '@ember/string';
-import {
-  typeOf,
-  copy,
-  Object as EmberObject,
-  A as emberA,
-  Evented,
-  ActionHandler,
-} from 'ember-runtime';
+import { typeOf, Object as EmberObject, A as emberA, Evented, ActionHandler } from 'ember-runtime';
 import generateController from './generate_controller';
 import {
   stashParamNames,
@@ -1593,7 +1586,7 @@ let Route = EmberObject.extend(ActionHandler, Evented, {
 
     if (!name) {
       if (sawParams) {
-        return copy(params);
+        return Object.assign({}, params);
       } else {
         if (transition.resolveIndex < 1) {
           return;

packages/ember-runtime/lib/copy.js

@@ -1,10 +1,12 @@
-import { assert } from '@ember/debug';
+import { assert, deprecate } from '@ember/debug';
 import EmberObject from './system/object';
 import Copyable from './mixins/copyable';
 
 /**
  @module @ember/object
-*/
+ @private
+ @deprecated Use 'ember-copy' addon instead
+ */
 function _copy(obj, deep, seen, copies) {
   // primitive data types are immutable, just return them.
   if (typeof obj !== 'object' || obj === null) {
@@ -18,11 +20,6 @@ function _copy(obj, deep, seen, copies) {
     return copies[loc];
   }
 
-  assert(
-    'Cannot clone an EmberObject that does not implement Copyable',
-    !(obj instanceof EmberObject) || Copyable.detect(obj)
-  );
-
   // IMPORTANT: this specific test will detect a native array only. Any other
   // object will need to implement Copyable.
   if (Array.isArray(obj)) {
@@ -40,6 +37,11 @@ function _copy(obj, deep, seen, copies) {
   } else if (obj instanceof Date) {
     ret = new Date(obj.getTime());
   } else {
+    assert(
+      'Cannot clone an EmberObject that does not implement Copyable',
+      !(obj instanceof EmberObject) || Copyable.detect(obj)
+    );
+
     ret = {};
     let key;
     for (key in obj) {
@@ -86,12 +88,18 @@ function _copy(obj, deep, seen, copies) {
   @public
 */
 export default function copy(obj, deep) {
+  deprecate('Use ember-copy addon instead of copy method and Copyable mixin.', false, {
+    id: 'ember-runtime.deprecate-copy-copyable',
+    until: '4.0.0',
+    url: 'https://emberjs.com/deprecations/v3.x/#toc_ember-runtime-deprecate-copy-copyable',
+  });
+
   // fast paths
   if ('object' !== typeof obj || obj === null) {
     return obj; // can't copy primitives
   }
 
-  if (Copyable.detect(obj)) {
+  if (!Array.isArray(obj) && Copyable.detect(obj)) {
     return obj.copy(deep);
   }
 

packages/ember-runtime/lib/mixins/array.js

@@ -1628,7 +1628,6 @@ const MutableArray = Mixin.create(ArrayMixin, MutableEnumerable, {
   @class Ember.NativeArray
   @uses MutableArray
   @uses Observable
-  @uses Ember.Copyable
   @public
 */
 let NativeArray = Mixin.create(MutableArray, Observable, {

packages/ember-runtime/lib/mixins/copyable.js

@@ -15,6 +15,7 @@ import { Mixin } from 'ember-metal';
   @class Copyable
   @namespace Ember
   @since Ember 0.9
+  @deprecated Use 'ember-copy' addon instead
   @private
 */
 export default Mixin.create({

packages/ember-runtime/tests/core/copy_test.js

@@ -6,28 +6,40 @@ moduleFor(
   class extends AbstractTestCase {
     ['@test Ember.copy null'](assert) {
       let obj = { field: null };
-
-      assert.equal(copy(obj, true).field, null, 'null should still be null');
+      let copied = null;
+      expectDeprecation(() => {
+        copied = copy(obj, true);
+      }, 'Use ember-copy addon instead of copy method and Copyable mixin.');
+      assert.equal(copied.field, null, 'null should still be null');
     }
 
     ['@test Ember.copy date'](assert) {
       let date = new Date(2014, 7, 22);
-      let dateCopy = copy(date);
-
+      let dateCopy = null;
+      expectDeprecation(() => {
+        dateCopy = copy(date);
+      }, 'Use ember-copy addon instead of copy method and Copyable mixin.');
       assert.equal(date.getTime(), dateCopy.getTime(), 'dates should be equivalent');
     }
 
     ['@test Ember.copy null prototype object'](assert) {
       let obj = Object.create(null);
 
       obj.foo = 'bar';
+      let copied = null;
+      expectDeprecation(() => {
+        copied = copy(obj);
+      }, 'Use ember-copy addon instead of copy method and Copyable mixin.');
 
-      assert.equal(copy(obj).foo, 'bar', 'bar should still be bar');
+      assert.equal(copied.foo, 'bar', 'bar should still be bar');
     }
 
     ['@test Ember.copy Array'](assert) {
       let array = [1, null, new Date(2015, 9, 9), 'four'];
-      let arrayCopy = copy(array);
+      let arrayCopy = null;
+      expectDeprecation(() => {
+        arrayCopy = copy(array);
+      }, 'Use ember-copy addon instead of copy method and Copyable mixin.');
 
       assert.deepEqual(array, arrayCopy, 'array content cloned successfully in new array');
     }

packages/ember/tests/routing/decoupled_basic_test.js

@@ -5,7 +5,7 @@ import { compile } from 'ember-template-compiler';
 import { ENV } from 'ember-environment';
 import { Route, NoneLocation, HistoryLocation } from 'ember-routing';
 import Controller from '@ember/controller';
-import { Object as EmberObject, A as emberA, copy } from 'ember-runtime';
+import { Object as EmberObject, A as emberA } from 'ember-runtime';
 import { moduleFor, ApplicationTestCase, runDestroy } from 'internal-test-helpers';
 import { run } from '@ember/runloop';
 import { Mixin, computed, set, addObserver, observer } from 'ember-metal';
@@ -1035,8 +1035,11 @@ moduleFor(
         actions: {
           showStuff(obj) {
             assert.ok(this instanceof HomeRoute, 'the handler is an App.HomeRoute');
-            // Using Ember.copy removes any private Ember vars which older IE would be confused by
-            assert.deepEqual(copy(obj, true), { name: 'Tom Dale' }, 'the context is correct');
+            assert.deepEqual(
+              Object.assign({}, obj),
+              { name: 'Tom Dale' },
+              'the context is correct'
+            );
             done();
           },
         },
@@ -1070,8 +1073,11 @@ moduleFor(
         actions: {
           showStuff(obj) {
             assert.ok(this instanceof RootRoute, 'the handler is an App.HomeRoute');
-            // Using Ember.copy removes any private Ember vars which older IE would be confused by
-            assert.deepEqual(copy(obj, true), { name: 'Tom Dale' }, 'the context is correct');
+            assert.deepEqual(
+              Object.assign({}, obj),
+              { name: 'Tom Dale' },
+              'the context is correct'
+            );
             done();
           },
         },
@@ -1210,10 +1216,13 @@ moduleFor(
         actions: {
           showStuff(obj1, obj2) {
             assert.ok(this instanceof RootRoute, 'the handler is an App.HomeRoute');
-            // Using Ember.copy removes any private Ember vars which older IE would be confused by
-            assert.deepEqual(copy(obj1, true), { name: 'Tilde' }, 'the first context is correct');
             assert.deepEqual(
-              copy(obj2, true),
+              Object.assign({}, obj1),
+              { name: 'Tilde' },
+              'the first context is correct'
+            );
+            assert.deepEqual(
+              Object.assign({}, obj2),
               { name: 'Tom Dale' },
               'the second context is correct'
             );