From 7a2ec1b1af6b04f3b0cb6a3a8b5cff9b56f1c430 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Mu=C3=B1oz?= Date: Wed, 10 Jan 2018 01:11:22 -0500 Subject: [PATCH 1/4] Remove private ArrayProxy contentArray{Will,Did}Change hooks --- .../ember-runtime/lib/system/array_proxy.js | 80 +------------------ .../system/array_proxy/content_update_test.js | 25 ------ .../watching_and_listening_test.js | 12 +-- 3 files changed, 7 insertions(+), 110 deletions(-) delete mode 100644 packages/ember-runtime/tests/system/array_proxy/content_update_test.js diff --git a/packages/ember-runtime/lib/system/array_proxy.js b/packages/ember-runtime/lib/system/array_proxy.js index b4a8fa5b0f4..17ead5bde2b 100644 --- a/packages/ember-runtime/lib/system/array_proxy.js +++ b/packages/ember-runtime/lib/system/array_proxy.js @@ -125,81 +125,6 @@ export default EmberObject.extend(MutableArray, { get(this, 'content').replace(idx, amt, objects); }, - /** - Invoked when the content property is about to change. Notifies observers that the - entire array content will change. - - @private - @method _contentWillChange - */ - _contentWillChange: _beforeObserver('content', function() { - this._teardownContent(); - }), - - _teardownContent() { - let content = get(this, 'content'); - - if (content) { - removeArrayObserver(content, this, { - willChange: 'contentArrayWillChange', - didChange: 'contentArrayDidChange' - }); - } - }, - - /** - Override to implement content array `willChange` observer. - - @method contentArrayWillChange - - @param {EmberArray} contentArray the content array - @param {Number} start starting index of the change - @param {Number} removeCount count of items removed - @param {Number} addCount count of items added - @private - */ - contentArrayWillChange: K, - /** - Override to implement content array `didChange` observer. - - @method contentArrayDidChange - - @param {EmberArray} contentArray the content array - @param {Number} start starting index of the change - @param {Number} removeCount count of items removed - @param {Number} addCount count of items added - @private - */ - contentArrayDidChange: K, - - /** - Invoked when the content property changes. Notifies observers that the - entire array content has changed. - - @private - @method _contentDidChange - */ - _contentDidChange: observer('content', function() { - let content = get(this, 'content'); - - assert('Can\'t set ArrayProxy\'s content to itself', content !== this); - - this._setupContent(); - }), - - _setupContent() { - let content = get(this, 'content'); - - if (content) { - assert(`ArrayProxy expects an Array or Ember.ArrayProxy, but you passed ${typeof content}`, isArray(content) || content.isDestroyed); - - addArrayObserver(content, this, { - willChange: 'contentArrayWillChange', - didChange: 'contentArrayDidChange' - }); - } - }, - _arrangedContentWillChange: _beforeObserver('arrangedContent', function() { let arrangedContent = get(this, 'arrangedContent'); let len = arrangedContent ? get(arrangedContent, 'length') : 0; @@ -214,8 +139,6 @@ export default EmberObject.extend(MutableArray, { let arrangedContent = get(this, 'arrangedContent'); let len = arrangedContent ? get(arrangedContent, 'length') : 0; - assert('Can\'t set ArrayProxy\'s content to itself', arrangedContent !== this); - this._setupArrangedContent(); this.arrangedContentDidChange(this); @@ -226,6 +149,7 @@ export default EmberObject.extend(MutableArray, { let arrangedContent = get(this, 'arrangedContent'); if (arrangedContent) { + assert('Can\'t set ArrayProxy\'s content to itself', arrangedContent !== this); assert(`ArrayProxy expects an Array or Ember.ArrayProxy, but you passed ${typeof arrangedContent}`, isArray(arrangedContent) || arrangedContent.isDestroyed); @@ -376,12 +300,10 @@ export default EmberObject.extend(MutableArray, { init() { this._super(...arguments); - this._setupContent(); this._setupArrangedContent(); }, willDestroy() { this._teardownArrangedContent(); - this._teardownContent(); } }); diff --git a/packages/ember-runtime/tests/system/array_proxy/content_update_test.js b/packages/ember-runtime/tests/system/array_proxy/content_update_test.js deleted file mode 100644 index 41fb1b61418..00000000000 --- a/packages/ember-runtime/tests/system/array_proxy/content_update_test.js +++ /dev/null @@ -1,25 +0,0 @@ -import { computed } from 'ember-metal'; -import ArrayProxy from '../../../system/array_proxy'; -import { A as emberA } from '../../../system/native_array'; - -QUnit.module('Ember.ArrayProxy - content update'); - -QUnit.test('The `contentArrayDidChange` method is invoked after `content` is updated.', function(assert) { - let observerCalled = false; - let proxy = ArrayProxy.extend({ - arrangedContent: computed('content', function() { - return emberA(this.get('content').slice()); - }), - - contentArrayDidChange(array, idx, removedCount, addedCount) { - observerCalled = true; - return this._super(array, idx, removedCount, addedCount); - } - }).create({ - content: emberA() - }); - - proxy.pushObject(1); - - assert.ok(observerCalled, 'contentArrayDidChange is invoked'); -}); diff --git a/packages/ember-runtime/tests/system/array_proxy/watching_and_listening_test.js b/packages/ember-runtime/tests/system/array_proxy/watching_and_listening_test.js index ce6d2f41b6d..844a85750b9 100644 --- a/packages/ember-runtime/tests/system/array_proxy/watching_and_listening_test.js +++ b/packages/ember-runtime/tests/system/array_proxy/watching_and_listening_test.js @@ -28,11 +28,11 @@ QUnit.test(`setting 'content' adds listeners correctly`, function(assert) { assert.deepEqual( sortedListenersFor(content, '@array:before'), - [[proxy, 'contentArrayWillChange'], [proxy, 'arrangedContentArrayWillChange']] + [[proxy, 'arrangedContentArrayWillChange']] ); assert.deepEqual( sortedListenersFor(content, '@array:change'), - [[proxy, 'contentArrayDidChange'], [proxy, 'arrangedContentArrayDidChange']] + [[proxy, 'arrangedContentArrayDidChange']] ); }); @@ -43,11 +43,11 @@ QUnit.test(`changing 'content' adds and removes listeners correctly`, function(a assert.deepEqual( sortedListenersFor(content1, '@array:before'), - [[proxy, 'contentArrayWillChange'], [proxy, 'arrangedContentArrayWillChange']] + [[proxy, 'arrangedContentArrayWillChange']] ); assert.deepEqual( sortedListenersFor(content1, '@array:change'), - [[proxy, 'contentArrayDidChange'], [proxy, 'arrangedContentArrayDidChange']] + [[proxy, 'arrangedContentArrayDidChange']] ); proxy.set('content', content2); @@ -56,11 +56,11 @@ QUnit.test(`changing 'content' adds and removes listeners correctly`, function(a assert.deepEqual(sortedListenersFor(content1, '@array:change'), []); assert.deepEqual( sortedListenersFor(content2, '@array:before'), - [[proxy, 'contentArrayWillChange'], [proxy, 'arrangedContentArrayWillChange']] + [[proxy, 'arrangedContentArrayWillChange']] ); assert.deepEqual( sortedListenersFor(content2, '@array:change'), - [[proxy, 'contentArrayDidChange'], [proxy, 'arrangedContentArrayDidChange']] + [[proxy, 'arrangedContentArrayDidChange']] ); }); From 30f66e590373ed8b79cb43bed73655156b7f7570 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Mu=C3=B1oz?= Date: Wed, 10 Jan 2018 01:37:45 -0500 Subject: [PATCH 2/4] Remove undocumented ArrayProxy arrangedContent{Will,Did}Change hooks --- .../ember-runtime/lib/system/array_proxy.js | 7 --- .../system/array_proxy/content_change_test.js | 48 ------------------- 2 files changed, 55 deletions(-) diff --git a/packages/ember-runtime/lib/system/array_proxy.js b/packages/ember-runtime/lib/system/array_proxy.js index 17ead5bde2b..e4c83e15fd1 100644 --- a/packages/ember-runtime/lib/system/array_proxy.js +++ b/packages/ember-runtime/lib/system/array_proxy.js @@ -27,8 +27,6 @@ import { assert, Error as EmberError } from 'ember-debug'; const OUT_OF_RANGE_EXCEPTION = 'Index out of range'; const EMPTY = []; -function K() { return this; } - /** An ArrayProxy wraps any other object that implements `Ember.Array` and/or @@ -130,7 +128,6 @@ export default EmberObject.extend(MutableArray, { let len = arrangedContent ? get(arrangedContent, 'length') : 0; this.arrangedContentArrayWillChange(this, 0, len, undefined); - this.arrangedContentWillChange(this); this._teardownArrangedContent(arrangedContent); }), @@ -141,7 +138,6 @@ export default EmberObject.extend(MutableArray, { this._setupArrangedContent(); - this.arrangedContentDidChange(this); this.arrangedContentArrayDidChange(this, 0, undefined, len); }), @@ -171,9 +167,6 @@ export default EmberObject.extend(MutableArray, { } }, - arrangedContentWillChange: K, - arrangedContentDidChange: K, - objectAt(idx) { return get(this, 'content') && this.objectAtContent(idx); }, diff --git a/packages/ember-runtime/tests/system/array_proxy/content_change_test.js b/packages/ember-runtime/tests/system/array_proxy/content_change_test.js index 82dbfb3666a..5e715aa0014 100644 --- a/packages/ember-runtime/tests/system/array_proxy/content_change_test.js +++ b/packages/ember-runtime/tests/system/array_proxy/content_change_test.js @@ -35,54 +35,6 @@ QUnit.test('should update length for null content when there is a computed prope assert.equal(proxy.get('length'), 0, 'length updates'); }); -QUnit.test('The `arrangedContentWillChange` method is invoked before `content` is changed.', function(assert) { - let callCount = 0; - let expectedLength; - - let proxy = ArrayProxy.extend({ - arrangedContentWillChange() { - assert.equal(this.get('arrangedContent.length'), expectedLength, 'hook should be invoked before array has changed'); - callCount++; - } - }).create({ content: emberA([1, 2, 3]) }); - - proxy.pushObject(4); - assert.equal(callCount, 0, 'pushing content onto the array doesn\'t trigger it'); - - proxy.get('content').pushObject(5); - assert.equal(callCount, 0, 'pushing content onto the content array doesn\'t trigger it'); - - expectedLength = 5; - proxy.set('content', emberA(['a', 'b'])); - assert.equal(callCount, 1, 'replacing the content array triggers the hook'); -}); - -QUnit.test('The `arrangedContentDidChange` method is invoked after `content` is changed.', function(assert) { - let callCount = 0; - let expectedLength; - - let proxy = ArrayProxy.extend({ - arrangedContentDidChange() { - assert.equal(this.get('arrangedContent.length'), expectedLength, 'hook should be invoked after array has changed'); - callCount++; - } - }).create({ - content: emberA([1, 2, 3]) - }); - - assert.equal(callCount, 0, 'hook is not called after creating the object'); - - proxy.pushObject(4); - assert.equal(callCount, 0, 'pushing content onto the array doesn\'t trigger it'); - - proxy.get('content').pushObject(5); - assert.equal(callCount, 0, 'pushing content onto the content array doesn\'t trigger it'); - - expectedLength = 2; - proxy.set('content', emberA(['a', 'b'])); - assert.equal(callCount, 1, 'replacing the content array triggers the hook'); -}); - QUnit.test('The ArrayProxy doesn\'t explode when assigned a destroyed object', function(assert) { let proxy1 = ArrayProxy.create(); let proxy2 = ArrayProxy.create(); From 3c483686f3c5c6fd34ceca2a124a4d9df732b8e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Mu=C3=B1oz?= Date: Wed, 10 Jan 2018 01:38:53 -0500 Subject: [PATCH 3/4] Add addArrayObserver sanity test for ArrayProxy --- .../system/array_proxy/content_change_test.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/packages/ember-runtime/tests/system/array_proxy/content_change_test.js b/packages/ember-runtime/tests/system/array_proxy/content_change_test.js index 5e715aa0014..4877d210b17 100644 --- a/packages/ember-runtime/tests/system/array_proxy/content_change_test.js +++ b/packages/ember-runtime/tests/system/array_proxy/content_change_test.js @@ -74,3 +74,21 @@ QUnit.test('arrayContent{Will,Did}Change are called when the content changes', f assert.equal(willChangeCallCount, 2); assert.equal(didChangeCallCount, 2); }); + +QUnit.test('addArrayObserver works correctly', function(assert) { + let content = emberA([]); + let proxy = ArrayProxy.create({ content }); + + assert.expect(2); + + proxy.addArrayObserver({ + arrayWillChange(arr) { + assert.equal(arr.get('length'), 0); + }, + arrayDidChange(arr) { + assert.equal(arr.get('length'), 3); + } + }); + + content.replace(0, 0, ['a', 'b', 'c']); +}); From 78f94cb6488630991f5ac7219b91baf1857da1e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Mu=C3=B1oz?= Date: Wed, 10 Jan 2018 02:00:48 -0500 Subject: [PATCH 4/4] ArrayProxy's content and arrangedContent should be @public --- packages/ember-runtime/lib/system/array_proxy.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/ember-runtime/lib/system/array_proxy.js b/packages/ember-runtime/lib/system/array_proxy.js index e4c83e15fd1..fc9835caa31 100644 --- a/packages/ember-runtime/lib/system/array_proxy.js +++ b/packages/ember-runtime/lib/system/array_proxy.js @@ -74,7 +74,7 @@ export default EmberObject.extend(MutableArray, { @property content @type EmberArray - @private + @public */ content: null, @@ -84,7 +84,7 @@ export default EmberObject.extend(MutableArray, { can override this property to provide things like sorting and filtering. @property arrangedContent - @private + @public */ arrangedContent: alias('content'),