From a3874bbdf843be73ae3467b8769addd64eb75a08 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Fri, 9 Oct 2020 08:16:25 -0400 Subject: [PATCH] Merge pull request #7330 from emberjs/sn/has-many-consume [BUG]: Consume array access to autotrack hasMany --- .../acceptance/relationships/has-many-test.js | 370 ++++++++++++++++-- .../acceptance/tracking-model-id-test.js | 150 ++++--- .../model/addon/-private/system/many-array.js | 15 +- 3 files changed, 422 insertions(+), 113 deletions(-) diff --git a/packages/-ember-data/tests/acceptance/relationships/has-many-test.js b/packages/-ember-data/tests/acceptance/relationships/has-many-test.js index b22912336a7..cc5ead6f720 100644 --- a/packages/-ember-data/tests/acceptance/relationships/has-many-test.js +++ b/packages/-ember-data/tests/acceptance/relationships/has-many-test.js @@ -1,4 +1,8 @@ -import { render } from '@ember/test-helpers'; +import { action } from '@ember/object'; +import { sort } from '@ember/object/computed'; +import { inject as service } from '@ember/service'; +import { click, findAll, render } from '@ember/test-helpers'; +import Component from '@glimmer/component'; import Ember from 'ember'; import hbs from 'htmlbars-inline-precompile'; @@ -13,10 +17,6 @@ import Model, { attr, belongsTo, hasMany } from '@ember-data/model'; import JSONAPISerializer from '@ember-data/serializer/json-api'; import Store from '@ember-data/store'; -function domListToArray(domList) { - return Array.prototype.slice.call(domList); -} - class Person extends Model { @attr() name; @@ -231,8 +231,7 @@ module('async has-many rendering tests', function(hooks) { `); - let items = this.element.querySelectorAll('li'); - let names = domListToArray(items).map(e => e.textContent); + let names = findAll('li').map(e => e.textContent); assert.deepEqual(names, ['Selena has a parent', 'Sedona has a parent'], 'We rendered the names'); }); @@ -259,20 +258,19 @@ module('async has-many rendering tests', function(hooks) { `); - let items = this.element.querySelectorAll('li'); - let names = domListToArray(items).map(e => e.textContent); + let items = findAll('li'); + let names = items.map(e => e.textContent); assert.deepEqual(names, ['Selena has a parent', 'Sedona has a parent'], 'We rendered the names'); this.set('parent', null); - items = this.element.querySelectorAll('li'); - assert.ok(items.length === 0, 'We have no items'); + items = findAll('li'); + assert.equal(items.length, 0, 'We have no items'); this.set('parent', parent); - items = this.element.querySelectorAll('li'); - names = domListToArray(items).map(e => e.textContent); + names = findAll('li').map(e => e.textContent); assert.deepEqual(names, ['Selena has a parent', 'Sedona has a parent'], 'We rendered the names'); }); @@ -310,8 +308,7 @@ module('async has-many rendering tests', function(hooks) { `); - let items = this.element.querySelectorAll('li'); - let names = domListToArray(items).map(e => e.textContent); + let names = findAll('li').map(e => e.textContent); assert.deepEqual(names, ['Selena has a parent'], 'We rendered only the names for successful requests'); @@ -352,8 +349,7 @@ module('async has-many rendering tests', function(hooks) { `); - let items = this.element.querySelectorAll('li'); - let names = domListToArray(items).map(e => e.textContent); + let names = findAll('li').map(e => e.textContent); assert.deepEqual(names, ['Selena has a parent', 'Sedona has a parent'], 'We rendered the names'); }); @@ -377,20 +373,19 @@ module('async has-many rendering tests', function(hooks) { `); - let items = this.element.querySelectorAll('li'); - let names = domListToArray(items).map(e => e.textContent); + let items = findAll('li'); + let names = items.map(e => e.textContent); assert.deepEqual(names, ['Selena has a parent', 'Sedona has a parent'], 'We rendered the names'); this.set('parent', null); - items = this.element.querySelectorAll('li'); - assert.ok(items.length === 0, 'We have no items'); + items = findAll('li'); + assert.equal(items.length, 0, 'We have no items'); this.set('parent', parent); - items = this.element.querySelectorAll('li'); - names = domListToArray(items).map(e => e.textContent); + names = findAll('li').map(e => e.textContent); assert.deepEqual(names, ['Selena has a parent', 'Sedona has a parent'], 'We rendered the names'); }); @@ -429,8 +424,7 @@ module('async has-many rendering tests', function(hooks) { `); - let items = this.element.querySelectorAll('li'); - let names = domListToArray(items).map(e => e.textContent); + let names = findAll('li').map(e => e.textContent); assert.deepEqual(names, [], 'We rendered no names'); @@ -475,8 +469,7 @@ module('async has-many rendering tests', function(hooks) { `); - let items = this.element.querySelectorAll('li'); - let names = domListToArray(items).map(e => e.textContent); + let names = findAll('li').map(e => e.textContent); assert.deepEqual(names, ['Selena has a parent', 'Sedona has a parent'], 'We rendered the names'); }); @@ -502,8 +495,7 @@ module('async has-many rendering tests', function(hooks) { `); - let items = this.element.querySelectorAll('li'); - let names = domListToArray(items).map(e => e.textContent); + let names = findAll('li').map(e => e.textContent); assert.deepEqual(names, ['Selena has a parent', 'Sedona has a parent'], 'We rendered the names'); }); @@ -527,22 +519,330 @@ module('async has-many rendering tests', function(hooks) { `); - let items = this.element.querySelectorAll('li'); - let names = domListToArray(items).map(e => e.textContent); + let items = findAll('li'); + let names = items.map(e => e.textContent); assert.deepEqual(names, ['Selena has a parent', 'Sedona has a parent'], 'We rendered the names'); this.set('parent', null); - items = this.element.querySelectorAll('li'); - assert.ok(items.length === 0, 'We have no items'); + items = findAll('li'); + assert.equal(items.length, 0, 'We have no items'); this.set('parent', parent); - items = this.element.querySelectorAll('li'); - names = domListToArray(items).map(e => e.textContent); + names = findAll('li').map(e => e.textContent); assert.deepEqual(names, ['Selena has a parent', 'Sedona has a parent'], 'We rendered the names'); }); }); }); + +module('autotracking has-many', function(hooks) { + setupRenderingTest(hooks); + + let store; + + hooks.beforeEach(function() { + let { owner } = this; + owner.register('model:person', Person); + owner.register('adapter:application', TestAdapter); + owner.register('serializer:application', JSONAPISerializer); + owner.register('service:store', Store); + store = owner.lookup('service:store'); + }); + + test('We can re-render a pojo', async function(assert) { + class ChildrenList extends Component { + @service store; + + get children() { + return this.args.model.children; + } + + get sortedChildren() { + return this.children.sortBy('name'); + } + + @action + createChild() { + const parent = this.args.model.person; + const name = 'RGB'; + this.store.createRecord('person', { name, parent }); + } + } + + let layout = hbs` + + +

{{this.sortedChildren.length}}

+ + `; + this.owner.register('component:children-list', ChildrenList); + this.owner.register('template:components/children-list', layout); + + store.createRecord('person', { id: '1', name: 'Doodad' }); + let person = store.peekRecord('person', '1'); + let children = await person.children; + this.model = { person, children }; + + await render(hbs``); + + let names = findAll('li').map(e => e.textContent); + + assert.deepEqual(names, [], 'rendered no children'); + + await click('#createChild'); + + names = findAll('li').map(e => e.textContent); + assert.deepEqual(names, ['RGB'], 'rendered 1 child'); + + await click('#createChild'); + + names = findAll('li').map(e => e.textContent); + assert.deepEqual(names, ['RGB', 'RGB'], 'rendered 2 children'); + }); + + test('We can re-render hasMany', async function(assert) { + class ChildrenList extends Component { + @service store; + + get sortedChildren() { + return this.args.person.children.sortBy('name'); + } + + @action + createChild() { + const parent = this.args.person; + const name = 'RGB'; + this.store.createRecord('person', { name, parent }); + } + } + + let layout = hbs` + + +

{{this.sortedChildren.length}}

+ + `; + this.owner.register('component:children-list', ChildrenList); + this.owner.register('template:components/children-list', layout); + + store.createRecord('person', { id: '1', name: 'Doodad' }); + this.person = store.peekRecord('person', '1'); + + await render(hbs``); + + let names = findAll('li').map(e => e.textContent); + + assert.deepEqual(names, [], 'rendered no children'); + + await click('#createChild'); + + names = findAll('li').map(e => e.textContent); + assert.deepEqual(names, ['RGB'], 'rendered 1 child'); + + await click('#createChild'); + + names = findAll('li').map(e => e.textContent); + assert.deepEqual(names, ['RGB', 'RGB'], 'rendered 2 children'); + }); + + test('We can re-render hasMany with sort computed macro', async function(assert) { + class ChildrenList extends Component { + @service store; + + sortProperties = ['name']; + @sort('args.person.children', 'sortProperties') sortedChildren; + + @action + createChild() { + const parent = this.args.person; + const name = 'RGB'; + this.store.createRecord('person', { name, parent }); + } + } + + let layout = hbs` + + +

{{this.sortedChildren.length}}

+
    + {{#each this.sortedChildren as |child|}} +
  • {{child.name}}
    • + {{/each}} +
+ `; + this.owner.register('component:children-list', ChildrenList); + this.owner.register('template:components/children-list', layout); + + store.createRecord('person', { id: '1', name: 'Doodad' }); + this.person = store.peekRecord('person', '1'); + + await render(hbs``); + + let names = findAll('li').map(e => e.textContent); + + assert.deepEqual(names, [], 'rendered no children'); + + await click('#createChild'); + + names = findAll('li').map(e => e.textContent); + assert.deepEqual(names, ['RGB'], 'rendered 1 child'); + + await click('#createChild'); + + names = findAll('li').map(e => e.textContent); + assert.deepEqual(names, ['RGB', 'RGB'], 'rendered 2 children'); + }); + + test('We can re-render hasMany with objectAt', async function(assert) { + class ChildrenList extends Component { + @service store; + + get firstChild() { + return this.args.person.children.objectAt(0); + } + + @action + createChild() { + const parent = this.args.person; + const name = 'RGB'; + this.store.createRecord('person', { name, parent }); + } + } + + let layout = hbs` + + +

{{this.firstChild.name}}

+ `; + this.owner.register('component:children-list', ChildrenList); + this.owner.register('template:components/children-list', layout); + + store.createRecord('person', { id: '1', name: 'Doodad' }); + this.person = store.peekRecord('person', '1'); + + await render(hbs``); + + assert.dom('h2').hasText('', 'rendered no children'); + + await click('#createChild'); + + assert.dom('h2').hasText('RGB', 'renders first child'); + + await click('#createChild'); + + assert.dom('h2').hasText('RGB', 'renders first child'); + }); + + test('We can re-render hasMany with native map', async function(assert) { + class ChildrenList extends Component { + @service store; + + get children() { + return this.args.person.children.map(child => child); + } + + @action + createChild() { + const parent = this.args.person; + const name = 'RGB'; + this.store.createRecord('person', { name, parent }); + } + } + + let layout = hbs` + + +

{{this.children.length}}

+
    + {{#each this.children as |child|}} +
  • {{child.name}}
    • + {{/each}} +
+ `; + this.owner.register('component:children-list', ChildrenList); + this.owner.register('template:components/children-list', layout); + + store.createRecord('person', { id: '1', name: 'Doodad' }); + this.person = store.peekRecord('person', '1'); + + await render(hbs``); + + let names = findAll('li').map(e => e.textContent); + + assert.deepEqual(names, [], 'rendered no children'); + + await click('#createChild'); + + names = findAll('li').map(e => e.textContent); + assert.deepEqual(names, ['RGB'], 'rendered 1 child'); + + await click('#createChild'); + + names = findAll('li').map(e => e.textContent); + assert.deepEqual(names, ['RGB', 'RGB'], 'rendered 2 children'); + }); + + test('We can re-render hasMany with peekAll', async function(assert) { + class PeopleList extends Component { + @service store; + + constructor() { + super(...arguments); + + this.model = { people: this.store.peekAll('person') }; + } + + get allPeople() { + return this.model.people.map(child => child); + } + + @action + createPerson() { + const name = 'RGB'; + this.store.createRecord('person', { name }); + } + } + + let layout = hbs` + + +

{{this.allPeople.length}}

+
    + {{#each this.allPeople as |person|}} +
  • {{person.name}}
    • + {{/each}} +
+ `; + this.owner.register('component:people-list', PeopleList); + this.owner.register('template:components/people-list', layout); + + store.createRecord('person', { id: '1', name: 'Doodad' }); + + await render(hbs``); + + let names = findAll('li').map(e => e.textContent); + + assert.deepEqual(names, ['Doodad'], 'rendered no people'); + + await click('#createPerson'); + + names = findAll('li').map(e => e.textContent); + assert.deepEqual(names, ['Doodad', 'RGB'], 'rendered 1 person'); + + await click('#createPerson'); + + names = findAll('li').map(e => e.textContent); + assert.deepEqual(names, ['Doodad', 'RGB', 'RGB'], 'rendered 2 people'); + }); +}); diff --git a/packages/-ember-data/tests/acceptance/tracking-model-id-test.js b/packages/-ember-data/tests/acceptance/tracking-model-id-test.js index 0090a68f0ad..043d76251ae 100644 --- a/packages/-ember-data/tests/acceptance/tracking-model-id-test.js +++ b/packages/-ember-data/tests/acceptance/tracking-model-id-test.js @@ -3,10 +3,8 @@ import Component from '@glimmer/component'; import hbs from 'htmlbars-inline-precompile'; import { module, test } from 'qunit'; -import { has } from 'require'; import { resolve } from 'rsvp'; -import { gte } from 'ember-compatibility-helpers'; import { setupRenderingTest } from 'ember-qunit'; import JSONAPIAdapter from '@ember-data/adapter/json-api'; @@ -14,89 +12,87 @@ import Model, { attr } from '@ember-data/model'; import JSONAPISerializer from '@ember-data/serializer/json-api'; import Store from '@ember-data/store'; -if (gte('3.13.0') && has('@glimmer/component')) { - class Widget extends Model { - @attr() name; +class Widget extends Model { + @attr() name; - get numericId() { - return Number(this.id); - } + get numericId() { + return Number(this.id); } +} - class WidgetList extends Component { - get sortedWidgets() { - let { widgets } = this.args; +class WidgetList extends Component { + get sortedWidgets() { + let { widgets } = this.args; - return widgets.slice().sort((a, b) => b.numericId - a.numericId); - } + return widgets.slice().sort((a, b) => b.numericId - a.numericId); } +} - let layout = hbs` -
    - {{#each this.sortedWidgets as |widget index|}} -
  • -
    ID: {{widget.id}}
    -
    Numeric ID: {{widget.numericId}}
    -
    Name: {{widget.name}}
    -
    -
    • - {{/each}} -
- `; - - class TestAdapter extends JSONAPIAdapter { - createRecord() { - return resolve({ - data: { - id: '4', - type: 'widget', - attributes: { - name: 'Contraption', - }, +let layout = hbs` +
    + {{#each this.sortedWidgets as |widget index|}} +
  • +
    ID: {{widget.id}}
    +
    Numeric ID: {{widget.numericId}}
    +
    Name: {{widget.name}}
    +
    +
    • + {{/each}} +
+`; + +class TestAdapter extends JSONAPIAdapter { + createRecord() { + return resolve({ + data: { + id: '4', + type: 'widget', + attributes: { + name: 'Contraption', }, - }); - } + }, + }); } +} - module('acceptance/tracking-model-id - tracking model id', function(hooks) { - setupRenderingTest(hooks); - - hooks.beforeEach(function() { - let { owner } = this; - owner.register('service:store', Store); - owner.register('model:widget', Widget); - owner.register('component:widget-list', WidgetList); - owner.register('template:components/widget-list', layout); - owner.register('adapter:application', TestAdapter); - owner.register('serializer:application', JSONAPISerializer); - }); +module('acceptance/tracking-model-id - tracking model id', function(hooks) { + setupRenderingTest(hooks); + + hooks.beforeEach(function() { + let { owner } = this; + owner.register('service:store', Store); + owner.register('model:widget', Widget); + owner.register('component:widget-list', WidgetList); + owner.register('template:components/widget-list', layout); + owner.register('adapter:application', TestAdapter); + owner.register('serializer:application', JSONAPISerializer); + }); - test("can track model id's without using get", async function(assert) { - let store = this.owner.lookup('service:store'); - store.createRecord('widget', { id: '1', name: 'Doodad' }); - store.createRecord('widget', { id: '3', name: 'Gizmo' }); - store.createRecord('widget', { id: '2', name: 'Gadget' }); - this.widgets = store.peekAll('widget'); - - await render(hbs` - - `); - await settled(); - - assert.dom('ul>li+li+li').exists; - assert.dom('ul>li.widget0>div.name').containsText('Gizmo'); - assert.dom('ul>li.widget1>div.name').containsText('Gadget'); - assert.dom('ul>li.widget2>div.name').containsText('Doodad'); - - let contraption = store.createRecord('widget', { name: 'Contraption' }); - await contraption.save(); - await settled(); - - assert.dom('ul>li+li+li+li').exists; - assert.dom('ul>li.widget0>div.name').containsText('Contraption'); - assert.dom('ul>li.widget1>div.name').containsText('Gizmo'); - assert.dom('ul>li.widget2>div.name').containsText('Gadget'); - assert.dom('ul>li.widget3>div.name').containsText('Doodad'); - }); + test("can track model id's without using get", async function(assert) { + let store = this.owner.lookup('service:store'); + store.createRecord('widget', { id: '1', name: 'Doodad' }); + store.createRecord('widget', { id: '3', name: 'Gizmo' }); + store.createRecord('widget', { id: '2', name: 'Gadget' }); + this.widgets = store.peekAll('widget'); + + await render(hbs` + + `); + await settled(); + + assert.dom('ul>li+li+li').exists(); + assert.dom('ul>li.widget0>div.name').containsText('Gizmo'); + assert.dom('ul>li.widget1>div.name').containsText('Gadget'); + assert.dom('ul>li.widget2>div.name').containsText('Doodad'); + + let contraption = store.createRecord('widget', { name: 'Contraption' }); + await contraption.save(); + await settled(); + + assert.dom('ul>li+li+li+li').exists(); + assert.dom('ul>li.widget0>div.name').containsText('Contraption'); + assert.dom('ul>li.widget1>div.name').containsText('Gizmo'); + assert.dom('ul>li.widget2>div.name').containsText('Gadget'); + assert.dom('ul>li.widget3>div.name').containsText('Doodad'); }); -} +}); diff --git a/packages/model/addon/-private/system/many-array.js b/packages/model/addon/-private/system/many-array.js index 2af7375b67c..c4b78efa4dc 100644 --- a/packages/model/addon/-private/system/many-array.js +++ b/packages/model/addon/-private/system/many-array.js @@ -68,7 +68,9 @@ export default EmberObject.extend(MutableArray, DeprecatedEvented, { @property {Boolean} isLoaded */ this.isLoaded = this.isLoaded || false; - this.length = 0; + + // async has-many rendering tests + this._length = 0; /** Used for async `hasMany` arrays @@ -167,6 +169,17 @@ export default EmberObject.extend(MutableArray, DeprecatedEvented, { return false; }, + get length() { + // By using `get()`, the tracking system knows to pay attention to changes that occur. + get(this, '[]'); + + return this._length; + }, + + set length(value) { + return (this._length = value); + }, + objectAt(index) { // TODO we likely need to force flush here /*