From 04edfe97d61c8f1a0d3b2e84728d652c6fecc0cb Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Fri, 5 Aug 2022 16:42:50 -0700 Subject: [PATCH] 71 test failures to go, implemente PromiseManyArray deprecation from RFC https://github.com/emberjs/rfcs/pull/745 --- .../q/record-data-record-wrapper.ts | 12 +- ember-data-types/q/record-data.ts | 2 - .../acceptance/relationships/has-many-test.js | 276 ++++++++++++++++- .../adapter/json-api-adapter-test.js | 9 +- .../rest-adapter/create-record-test.js | 3 +- .../non-dasherized-lookups-test.js | 12 +- .../integration/records/delete-record-test.js | 7 +- .../relationships/one-to-one-test.js | 33 +- .../custom-class-model-test.ts | 4 +- .../tests/unit/store/unload-test.js | 3 +- packages/model/addon/-private/attr.js | 8 + .../-private/legacy-relationships-support.ts | 17 +- packages/model/addon/-private/model.js | 14 +- .../addon/-private/promise-many-array.ts | 291 +++++++++++------- .../addon/current-deprecations.ts | 1 + .../private-build-infra/addon/deprecations.ts | 2 +- .../record-data/addon/-private/record-data.ts | 28 -- .../adapter-populated-record-array.ts | 6 +- .../store/addon/-private/store-service.ts | 10 +- 19 files changed, 514 insertions(+), 224 deletions(-) diff --git a/ember-data-types/q/record-data-record-wrapper.ts b/ember-data-types/q/record-data-record-wrapper.ts index 8b0241ea189..079898863ac 100644 --- a/ember-data-types/q/record-data-record-wrapper.ts +++ b/ember-data-types/q/record-data-record-wrapper.ts @@ -7,7 +7,7 @@ import type { JsonApiValidationError } from './record-data-json-api'; @module @ember-data/store */ -export interface RecordDataRecordWrapper { +export interface RecordDataWrapper { rollbackAttributes(): string[]; changedAttributes(): ChangedAttributesHash; hasChangedAttributes(): boolean; @@ -16,17 +16,15 @@ export interface RecordDataRecordWrapper { getAttr(key: string): any; getHasMany(key: string): CollectionResourceRelationship; - addToHasMany(key: string, recordDatas: RecordDataRecordWrapper[], idx?: number): void; - removeFromHasMany(key: string, recordDatas: RecordDataRecordWrapper[]): void; - setDirtyHasMany(key: string, recordDatas: RecordDataRecordWrapper[]): void; + addToHasMany(key: string, recordDatas: RecordDataWrapper[], idx?: number): void; + removeFromHasMany(key: string, recordDatas: RecordDataWrapper[]): void; + setDirtyHasMany(key: string, recordDatas: RecordDataWrapper[]): void; getBelongsTo(key: string): SingleResourceRelationship; - setDirtyBelongsTo(name: string, recordData: RecordDataRecordWrapper | null): void; + setDirtyBelongsTo(name: string, recordData: RecordDataWrapper | null): void; // ----- unspecced - isAttrDirty(key: string): boolean; - removeFromInverseRelationships(): void; hasAttr(key: string): boolean; // new diff --git a/ember-data-types/q/record-data.ts b/ember-data-types/q/record-data.ts index 097a24ab7db..410e0391d32 100644 --- a/ember-data-types/q/record-data.ts +++ b/ember-data-types/q/record-data.ts @@ -45,8 +45,6 @@ export interface RecordData { didCommit(data: JsonApiResource | null): void; // ----- unspecced - isAttrDirty(key: string): boolean; - removeFromInverseRelationships(): void; hasAttr(key: string): boolean; isRecordInUse(): boolean; 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 0f29c76699f..b42884fcc05 100644 --- a/packages/-ember-data/tests/acceptance/relationships/has-many-test.js +++ b/packages/-ember-data/tests/acceptance/relationships/has-many-test.js @@ -18,6 +18,7 @@ import Model, { attr, belongsTo, hasMany } from '@ember-data/model'; import { LEGACY_SUPPORT } from '@ember-data/model/-private'; import JSONAPISerializer from '@ember-data/serializer/json-api'; import Store from '@ember-data/store'; +import { deprecatedTest } from '@ember-data/unpublished-test-infra/test-support/deprecated-test'; class Person extends Model { @attr() @@ -751,14 +752,212 @@ module('autotracking has-many', function (hooks) { assert.deepEqual(names, ['RGB', 'RGB'], 'rendered 2 children'); }); + deprecatedTest( + 'We can re-render hasMany w/PromiseManyArray.sortBy', + { id: 'ember-data:deprecate-promise-many-array-behaviors', until: '5.0', count: 6 }, + 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'); + } + ); + + deprecatedTest( + 'We can re-render hasMany with sort computed macro on PromiseManyArray', + { id: 'ember-data:deprecate-promise-many-array-behaviors', until: '5.0', count: 6 }, + 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}}

+ + `; + 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'); + } + ); + + deprecatedTest( + 'We can re-render hasMany with PromiseManyArray.objectAt', + { id: 'ember-data:deprecate-promise-many-array-behaviors', until: '5.0', count: 6 }, + 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'); + } + ); + + deprecatedTest( + 'We can re-render hasMany with PromiseManyArray.map', + { id: 'ember-data:deprecate-promise-many-array-behaviors', until: '5.0', count: 6 }, + 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', 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; @@ -770,9 +969,9 @@ module('autotracking has-many', function (hooks) { let layout = hbs` -

{{this.sortedChildren.length}}

+

{{@person.children.length}}

    - {{#each this.sortedChildren as |child|}} + {{#each @person.children as |child|}}
  • {{child.name}}
    • {{/each}}
@@ -805,7 +1004,7 @@ module('autotracking has-many', function (hooks) { @service store; sortProperties = ['name']; - @sort('args.person.children', 'sortProperties') sortedChildren; + @sort('args.children', 'sortProperties') sortedChildren; @action createChild() { @@ -830,8 +1029,9 @@ module('autotracking has-many', function (hooks) { store.createRecord('person', { id: '1', name: 'Doodad' }); this.person = store.peekRecord('person', '1'); + this.children = await this.person.children; - await render(hbs``); + await render(hbs``); let names = findAll('li').map((e) => e.textContent); @@ -853,7 +1053,7 @@ module('autotracking has-many', function (hooks) { @service store; get firstChild() { - return this.args.person.children.objectAt(0); + return this.args.children.objectAt(0); } @action @@ -874,8 +1074,9 @@ module('autotracking has-many', function (hooks) { store.createRecord('person', { id: '1', name: 'Doodad' }); this.person = store.peekRecord('person', '1'); + this.children = await this.person.children; - await render(hbs``); + await render(hbs``); assert.dom('h2').hasText('', 'rendered no children'); @@ -893,7 +1094,7 @@ module('autotracking has-many', function (hooks) { @service store; get children() { - return this.args.person.children.map((child) => child); + return this.args.children.map((child) => child); } @action @@ -919,8 +1120,59 @@ module('autotracking has-many', function (hooks) { store.createRecord('person', { id: '1', name: 'Doodad' }); this.person = store.peekRecord('person', '1'); + this.children = await this.person.children; - await render(hbs``); + 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 toArray', async function (assert) { + class ChildrenList extends Component { + @service store; + + get children() { + return this.args.children.toArray(); + } + + @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'); + this.children = await this.person.children; + + await render(hbs``); let names = findAll('li').map((e) => e.textContent); diff --git a/packages/-ember-data/tests/integration/adapter/json-api-adapter-test.js b/packages/-ember-data/tests/integration/adapter/json-api-adapter-test.js index 87397505871..fdb64043c13 100644 --- a/packages/-ember-data/tests/integration/adapter/json-api-adapter-test.js +++ b/packages/-ember-data/tests/integration/adapter/json-api-adapter-test.js @@ -200,15 +200,18 @@ module('integration/adapter/json-api-adapter - JSONAPIAdapter', function (hooks) 'The author for the last post is loaded and has the correct last name' ); - assert.strictEqual(posts.firstObject.comments.length, 0, 'First post doesnt have comments'); + const firstComments = await posts.firstObject.comments; + const lastComments = await posts.lastObject.comments; + + assert.strictEqual(firstComments.length, 0, 'First post doesnt have comments'); assert.strictEqual( - posts.lastObject.comments.firstObject.text, + lastComments.firstObject.text, 'This is the first comment', 'Loads first comment for second post' ); assert.strictEqual( - posts.lastObject.comments.lastObject.text, + lastComments.lastObject.text, 'This is the second comment', 'Loads second comment for second post' ); diff --git a/packages/-ember-data/tests/integration/adapter/rest-adapter/create-record-test.js b/packages/-ember-data/tests/integration/adapter/rest-adapter/create-record-test.js index 83e30108db4..6e89d8076da 100644 --- a/packages/-ember-data/tests/integration/adapter/rest-adapter/create-record-test.js +++ b/packages/-ember-data/tests/integration/adapter/rest-adapter/create-record-test.js @@ -185,7 +185,8 @@ module('integration/adapter/rest_adapter - REST Adapter - createRecord', functio const post = store.peekRecord('post', 1); const comment = store.createRecord('comment', { name: 'The Parley Letter' }); - post.get('comments').pushObject(comment); + const comments = await post.get('comments'); + comments.pushObject(comment); assert.strictEqual(comment.get('post'), post, 'the post has been set correctly'); diff --git a/packages/-ember-data/tests/integration/backwards-compat/non-dasherized-lookups-test.js b/packages/-ember-data/tests/integration/backwards-compat/non-dasherized-lookups-test.js index f042aa0729a..bf20ebc7d08 100644 --- a/packages/-ember-data/tests/integration/backwards-compat/non-dasherized-lookups-test.js +++ b/packages/-ember-data/tests/integration/backwards-compat/non-dasherized-lookups-test.js @@ -151,7 +151,7 @@ module( }); }); - test('looks up belongsTo using under_scored strings', function (assert) { + test('looks up belongsTo using under_scored strings', async function (assert) { assert.expect(1); let store = this.owner.lookup('service:store'); @@ -181,13 +181,11 @@ module( }); }); - run(() => { - store.findRecord('long_model_name', 1).then((longModelName) => { - const postNotes = get(longModelName, 'postNotes').toArray(); + const longModel = await store.findRecord('long_model_name', '1'); + const postNotesRel = await longModel.postNotes; + const postNotes = postNotesRel.toArray(); - assert.deepEqual(postNotes, [store.peekRecord('postNote', 1)], 'inverse records found'); - }); - }); + assert.deepEqual(postNotes, [store.peekRecord('postNote', 1)], 'inverse records found'); }); } ); diff --git a/packages/-ember-data/tests/integration/records/delete-record-test.js b/packages/-ember-data/tests/integration/records/delete-record-test.js index 921abafc9f5..5e46119609e 100644 --- a/packages/-ember-data/tests/integration/records/delete-record-test.js +++ b/packages/-ember-data/tests/integration/records/delete-record-test.js @@ -429,8 +429,9 @@ module('integration/deletedRecord - Deleting Records', function (hooks) { // Sanity Check assert.ok(group, 'expected group to be found'); assert.strictEqual(group.get('company.name'), 'Inc.', 'group belongs to our company'); - assert.strictEqual(group.get('employees.length'), 1, 'expected 1 related record before delete'); - employee = group.get('employees').objectAt(0); + assert.strictEqual(group.employees.length, 1, 'expected 1 related record before delete'); + const employees = await group.employees; + employee = employees.objectAt(0); assert.strictEqual(employee.get('name'), 'Adam Sunderland', 'expected related records to be loaded'); await group.destroyRecord(); @@ -444,6 +445,6 @@ module('integration/deletedRecord - Deleting Records', function (hooks) { store.push(jsonGroup); group = store.peekRecord('group', '1'); - assert.strictEqual(group.get('employees.length'), 1, 'expected 1 related record after delete and restore'); + assert.strictEqual(group.employees.length, 1, 'expected 1 related record after delete and restore'); }); }); diff --git a/packages/-ember-data/tests/integration/relationships/one-to-one-test.js b/packages/-ember-data/tests/integration/relationships/one-to-one-test.js index d2239fcf01a..766d0346915 100644 --- a/packages/-ember-data/tests/integration/relationships/one-to-one-test.js +++ b/packages/-ember-data/tests/integration/relationships/one-to-one-test.js @@ -1,4 +1,5 @@ import { run } from '@ember/runloop'; +import { settled } from '@ember/test-helpers'; import { module, test } from 'qunit'; import { Promise as EmberPromise, resolve } from 'rsvp'; @@ -1009,27 +1010,23 @@ module('integration/relationships/one_to_one_test - OneToOne relationships', fun }); }); - test('Rollbacking attributes of created record removes the relationship on both sides - sync', function (assert) { + test('Rollbacking attributes of created record removes the relationship on both sides - sync', async function (assert) { let store = this.owner.lookup('service:store'); - var user, job; - run(function () { - user = store.push({ - data: { - id: 1, - type: 'user', - attributes: { - name: 'Stanley', - }, + const user = store.push({ + data: { + id: 1, + type: 'user', + attributes: { + name: 'Stanley', }, - }); - - job = store.createRecord('job', { user: user }); - }); - run(function () { - job.rollbackAttributes(); + }, }); - assert.strictEqual(user.get('job'), null, 'Job got rollbacked correctly'); - assert.strictEqual(job.get('user'), null, 'Job does not have user anymore'); + const job = store.createRecord('job', { user: user }); + job.rollbackAttributes(); + await settled(); + + assert.strictEqual(user.job, null, 'Job got rollbacked correctly'); + assert.true(job.isDestroyed, 'Job is destroyed'); }); }); diff --git a/packages/-ember-data/tests/unit/custom-class-support/custom-class-model-test.ts b/packages/-ember-data/tests/unit/custom-class-support/custom-class-model-test.ts index 30df23e5a59..983d6008ba1 100644 --- a/packages/-ember-data/tests/unit/custom-class-support/custom-class-model-test.ts +++ b/packages/-ember-data/tests/unit/custom-class-support/custom-class-model-test.ts @@ -11,7 +11,7 @@ import Store from '@ember-data/store'; import type { RecordDataStoreWrapper, Snapshot } from '@ember-data/store/-private'; import type NotificationManager from '@ember-data/store/-private/managers/record-notification-manager'; import type { RecordIdentifier, StableRecordIdentifier } from '@ember-data/types/q/identifier'; -import type { RecordDataRecordWrapper } from '@ember-data/types/q/record-data-record-wrapper'; +import type { RecordDataWrapper } from '@ember-data/types/q/record-data-record-wrapper'; import type { AttributesSchema, RelationshipsSchema } from '@ember-data/types/q/record-data-schemas'; import type { RecordInstance } from '@ember-data/types/q/record-instance'; import type { SchemaDefinitionService } from '@ember-data/types/q/schema-definition-service'; @@ -313,7 +313,7 @@ module('unit/model - Custom Class Model', function (hooks) { }); test('store.deleteRecord', async function (assert) { - let rd: RecordDataRecordWrapper; + let rd: RecordDataWrapper; assert.expect(9); this.owner.register( 'adapter:application', diff --git a/packages/-ember-data/tests/unit/store/unload-test.js b/packages/-ember-data/tests/unit/store/unload-test.js index 797d884fc3c..e7bcf6ae673 100644 --- a/packages/-ember-data/tests/unit/store/unload-test.js +++ b/packages/-ember-data/tests/unit/store/unload-test.js @@ -9,6 +9,7 @@ import { setupTest } from 'ember-qunit'; import Adapter from '@ember-data/adapter'; import Model, { attr, belongsTo } from '@ember-data/model'; import JSONAPISerializer from '@ember-data/serializer/json-api'; +import { recordIdentifierFor } from '@ember-data/store'; import testInDebug from '@ember-data/unpublished-test-infra/test-support/test-in-debug'; let store, tryToFind; @@ -70,7 +71,7 @@ module('unit/store/unload - Store unloading records', function (hooks) { function () { record.unloadRecord(); }, - 'You can only unload a record which is not inFlight. `' + record.toString() + '`', + `You can only unload a record which is not inFlight. '` + recordIdentifierFor(record).toString() + `'`, 'can not unload dirty record' ); diff --git a/packages/model/addon/-private/attr.js b/packages/model/addon/-private/attr.js index ec53d9e57db..f1821b82bff 100644 --- a/packages/model/addon/-private/attr.js +++ b/packages/model/addon/-private/attr.js @@ -137,6 +137,14 @@ function attr(type, options) { } } let recordData = recordDataFor(this); + // TODO hasAttr is not spec'd + // essentially this is needed because + // there is a difference between "undefined" meaning never set + // and "undefined" meaning set to "undefined". In the "key present" + // case we want to return undefined. In the "key absent" case + // we want to return getDefaultValue. RecordDataV2 can fix this + // by providing the attributes blob such that we can make our + // own determination. if (recordData.hasAttr(key)) { return recordData.getAttr(key); } else { diff --git a/packages/model/addon/-private/legacy-relationships-support.ts b/packages/model/addon/-private/legacy-relationships-support.ts index 2a44477df31..4baac480a46 100644 --- a/packages/model/addon/-private/legacy-relationships-support.ts +++ b/packages/model/addon/-private/legacy-relationships-support.ts @@ -14,9 +14,9 @@ import type { UpgradedMeta } from '@ember-data/record-data/-private/graph/-edge- import type { RelationshipState } from '@ember-data/record-data/-private/graph/-state'; import type Store from '@ember-data/store'; import { recordDataFor, recordIdentifierFor, storeFor } from '@ember-data/store/-private'; -import type { IdentifierCache } from '@ember-data/store/-private/caches/identifier-cache'; +import { IdentifierCache } from '@ember-data/store/-private/caches/identifier-cache'; import type { DSModel } from '@ember-data/types/q/ds-model'; -import type { ResourceIdentifierObject } from '@ember-data/types/q/ember-data-json-api'; +import { ResourceIdentifierObject } from '@ember-data/types/q/ember-data-json-api'; import type { StableRecordIdentifier } from '@ember-data/types/q/identifier'; import type { RecordData } from '@ember-data/types/q/record-data'; import type { JsonApiRelationship } from '@ember-data/types/q/record-data-json-api'; @@ -140,8 +140,7 @@ export class LegacySupport { `You looked up the '${key}' relationship on a '${identifier.type}' with id ${ identifier.id || 'null' } but some of the associated records were not loaded. Either make sure they are all loaded together with the parent record, or specify that the relationship is async (\`belongsTo({ async: true })\`)`, - toReturn === null || - !store._instanceCache.peek({ identifier: relatedIdentifier, bucket: 'recordData' })?.isEmpty?.() + toReturn === null || store._instanceCache.recordIsLoaded(relatedIdentifier) ); return toReturn; } @@ -674,16 +673,6 @@ function anyUnloaded(store: Store, relationship: ManyRelationship) { return unloaded || false; } -/** - * Flag indicating whether all inverse records are available - * - * true if the inverse exists and is loaded (not empty) - * true if there is no inverse - * false if the inverse exists and is not loaded (empty) - * - * @internal - * @return {boolean} - */ function areAllInverseRecordsLoaded(store: Store, resource: JsonApiRelationship): boolean { const cache = store.identifierCache; diff --git a/packages/model/addon/-private/model.js b/packages/model/addon/-private/model.js index 25f2cf7ed31..a87289149f5 100644 --- a/packages/model/addon/-private/model.js +++ b/packages/model/addon/-private/model.js @@ -818,9 +818,13 @@ class Model extends EmberObject { */ rollbackAttributes() { const { currentState } = this; + const { isNew } = currentState; recordDataFor(this).rollbackAttributes(); this.errors.clear(); currentState.cleanErrorRequests(); + if (isNew) { + this.unloadRecord(); + } } /** @@ -1805,7 +1809,7 @@ class Model extends EmberObject { let fields = Blog.fields; fields.forEach(function(kind, field) { - console.log(field, kind); + // do thing }); // prints: @@ -1987,7 +1991,7 @@ class Model extends EmberObject { let attributes = Person.attributes attributes.forEach(function(meta, name) { - console.log(name, meta); + // do thing }); // prints: @@ -2064,7 +2068,7 @@ class Model extends EmberObject { let transformedAttributes = Person.transformedAttributes transformedAttributes.forEach(function(field, type) { - console.log(field, type); + // do thing }); // prints: @@ -2137,7 +2141,7 @@ class Model extends EmberObject { } PersonModel.eachAttribute(function(name, meta) { - console.log(name, meta); + // do thing }); // prints: @@ -2206,7 +2210,7 @@ class Model extends EmberObject { }); Person.eachTransformedAttribute(function(name, type) { - console.log(name, type); + // do thing }); // prints: diff --git a/packages/model/addon/-private/promise-many-array.ts b/packages/model/addon/-private/promise-many-array.ts index 6f8336055d6..8495c28598b 100644 --- a/packages/model/addon/-private/promise-many-array.ts +++ b/packages/model/addon/-private/promise-many-array.ts @@ -1,6 +1,6 @@ import ArrayMixin, { NativeArray } from '@ember/array'; import type ArrayProxy from '@ember/array/proxy'; -import { assert } from '@ember/debug'; +import { assert, deprecate } from '@ember/debug'; import { dependentKeyCompat } from '@ember/object/compat'; import { tracked } from '@glimmer/tracking'; import Ember from 'ember'; @@ -9,8 +9,10 @@ import { resolve } from 'rsvp'; import type { ManyArray } from 'ember-data/-private'; +import { DEPRECATE_PROMISE_MANY_ARRAY_BEHAVIORS } from '@ember-data/private-build-infra/deprecations'; import { StableRecordIdentifier } from '@ember-data/types/q/identifier'; import type { RecordInstance } from '@ember-data/types/q/record-instance'; +import { FindOptions } from '@ember-data/types/q/store'; export interface HasManyProxyCreateArgs { promise: Promise; @@ -24,16 +26,10 @@ export interface HasManyProxyCreateArgs { This class is returned as the result of accessing an async hasMany relationship on an instance of a Model extending from `@ember-data/model`. - A PromiseManyArray is an array-like proxy that also proxies certain method calls - to the underlying ManyArray in addition to being "promisified". + A PromiseManyArray is an iterable proxy that allows templates to consume related + ManyArrays and update once their contents are no longer pending. - Right now we proxy: - - * `reload()` - * `createRecord()` - - This promise-proxy behavior is primarily to ensure that async relationship interact - nicely with templates. In your JS code you should resolve the promise first. + In your JS code you should resolve the promise first. ```js const comments = await post.comments; @@ -42,10 +38,14 @@ export interface HasManyProxyCreateArgs { @class PromiseManyArray @public */ -export default interface PromiseManyArray extends Omit, 'destroy'> {} +export default interface PromiseManyArray extends Omit, 'destroy'> { + createRecord(): RecordInstance; + reload(options: FindOptions): PromiseManyArray; +} export default class PromiseManyArray { declare promise: Promise | null; declare isDestroyed: boolean; + // @deprecated (isDestroyed is not deprecated) declare isDestroying: boolean; constructor(promise: Promise, content?: ManyArray) { @@ -90,6 +90,8 @@ export default class PromiseManyArray { /** * Iterate the proxied content. Called by the glimmer iterator in #each + * We do not guarantee that forEach will always be available. This + * may eventually be made to use Symbol.Iterator once glimmer supports it. * * @method forEach * @param cb @@ -201,19 +203,6 @@ export default class PromiseManyArray { return this.content ? this.content.meta : undefined; } - /** - * Reload the relationship - * @method reload - * @public - * @param options - * @returns - */ - reload(options) { - assert('You are trying to reload an async manyArray before it has been created', this.content); - this.content.reload(options); - return this; - } - //---- Our own stuff _update(promise: Promise, content?: ManyArray) { @@ -227,22 +216,79 @@ export default class PromiseManyArray { static create({ promise, content }: HasManyProxyCreateArgs): PromiseManyArray { return new this(promise, content); } +} - // Methods on ManyArray which people should resolve the relationship first before calling - createRecord(...args) { +if (DEPRECATE_PROMISE_MANY_ARRAY_BEHAVIORS) { + // TODO update RFC to also note reload deprecation + /** + * Reload the relationship + * @method reload + * @deprecated + * @public + * @param options + * @returns + */ + PromiseManyArray.prototype.reload = function reload(options: FindOptions) { + deprecate( + `The reload method on ember-data's PromiseManyArray is deprecated. await the promise and work with the ManyArray directly or use the HasManyReference interface.`, + false, + { + id: 'ember-data:deprecate-promise-many-array-behaviors', + until: '5.0', + since: { enabled: '4.8', available: '4.8' }, + for: 'ember-data', + } + ); + assert('You are trying to reload an async manyArray before it has been created', this.content); + this.content.reload(options); + return this; + }; + PromiseManyArray.prototype.createRecord = function createRecord(...args) { + deprecate( + `The createRecord method on ember-data's PromiseManyArray is deprecated. await the promise and work with the ManyArray directly.`, + false, + { + id: 'ember-data:deprecate-promise-many-array-behaviors', + until: '5.0', + since: { enabled: '4.8', available: '4.8' }, + for: 'ember-data', + } + ); assert('You are trying to createRecord on an async manyArray before it has been created', this.content); return this.content.createRecord(...args); - } - - // Properties/Methods on ArrayProxy we should deprecate - - get firstObject() { - return this.content ? this.content.firstObject : undefined; - } + }; - get lastObject() { - return this.content ? this.content.lastObject : undefined; - } + Object.defineProperty(PromiseManyArray.prototype, 'firstObject', { + get() { + deprecate( + `The firstObject property on ember-data's PromiseManyArray is deprecated. await the promise and work with the ManyArray directly.`, + false, + { + id: 'ember-data:deprecate-promise-many-array-behaviors', + until: '5.0', + since: { enabled: '4.8', available: '4.8' }, + for: 'ember-data', + } + ); + return this.content ? this.content.firstObject : undefined; + }, + }); + + Object.defineProperty(PromiseManyArray.prototype, 'lastObject', { + get() { + deprecate( + `The lastObject property on ember-data's PromiseManyArray is deprecated. await the promise and work with the ManyArray directly.`, + false, + { + id: 'ember-data:deprecate-promise-many-array-behaviors', + until: '5.0', + since: { enabled: '4.8', available: '4.8' }, + for: 'ember-data', + } + ); + return this.content ? this.content.lastObject : undefined; + }, + }); } function tapPromise(proxy: PromiseManyArray, promise: Promise) { @@ -268,78 +314,101 @@ function tapPromise(proxy: PromiseManyArray, promise: Promise) { ); } -const EmberObjectMethods = [ - 'addObserver', - 'cacheFor', - 'decrementProperty', - 'get', - 'getProperties', - 'incrementProperty', - 'notifyPropertyChange', - 'removeObserver', - 'set', - 'setProperties', - 'toggleProperty', -]; -EmberObjectMethods.forEach((method) => { - PromiseManyArray.prototype[method] = function delegatedMethod(...args) { - return Ember[method](this, ...args); - }; -}); - -const InheritedProxyMethods = [ - 'addArrayObserver', - 'addObject', - 'addObjects', - 'any', - 'arrayContentDidChange', - 'arrayContentWillChange', - 'clear', - 'compact', - 'every', - 'filter', - 'filterBy', - 'find', - 'findBy', - 'getEach', - 'includes', - 'indexOf', - 'insertAt', - 'invoke', - 'isAny', - 'isEvery', - 'lastIndexOf', - 'map', - 'mapBy', - 'objectAt', - 'objectsAt', - 'popObject', - 'pushObject', - 'pushObjects', - 'reduce', - 'reject', - 'rejectBy', - 'removeArrayObserver', - 'removeAt', - 'removeObject', - 'removeObjects', - 'replace', - 'reverseObjects', - 'setEach', - 'setObjects', - 'shiftObject', - 'slice', - 'sortBy', - 'toArray', - 'uniq', - 'uniqBy', - 'unshiftObject', - 'unshiftObjects', - 'without', -]; -InheritedProxyMethods.forEach((method) => { - PromiseManyArray.prototype[method] = function proxiedMethod(...args) { - assert(`Cannot call ${method} before content is assigned.`, this.content); - return this.content[method](...args); - }; -}); +if (DEPRECATE_PROMISE_MANY_ARRAY_BEHAVIORS) { + const EmberObjectMethods = [ + 'addObserver', + 'cacheFor', + 'decrementProperty', + 'get', + 'getProperties', + 'incrementProperty', + 'notifyPropertyChange', + 'removeObserver', + 'set', + 'setProperties', + 'toggleProperty', + ]; + EmberObjectMethods.forEach((method) => { + PromiseManyArray.prototype[method] = function delegatedMethod(...args) { + deprecate( + `The ${method} method on ember-data's PromiseManyArray is deprecated. await the promise and work with the ManyArray directly.`, + false, + { + id: 'ember-data:deprecate-promise-many-array-behaviors', + until: '5.0', + since: { enabled: '4.8', available: '4.8' }, + for: 'ember-data', + } + ); + return Ember[method](this, ...args); + }; + }); + + const InheritedProxyMethods = [ + 'addArrayObserver', + 'addObject', + 'addObjects', + 'any', + 'arrayContentDidChange', + 'arrayContentWillChange', + 'clear', + 'compact', + 'every', + 'filter', + 'filterBy', + 'find', + 'findBy', + 'getEach', + 'includes', + 'indexOf', + 'insertAt', + 'invoke', + 'isAny', + 'isEvery', + 'lastIndexOf', + 'map', + 'mapBy', + // TODO update RFC to note objectAt was deprecated (forEach was left for iteration) + 'objectAt', + 'objectsAt', + 'popObject', + 'pushObject', + 'pushObjects', + 'reduce', + 'reject', + 'rejectBy', + 'removeArrayObserver', + 'removeAt', + 'removeObject', + 'removeObjects', + 'replace', + 'reverseObjects', + 'setEach', + 'setObjects', + 'shiftObject', + 'slice', + 'sortBy', + 'toArray', + 'uniq', + 'uniqBy', + 'unshiftObject', + 'unshiftObjects', + 'without', + ]; + InheritedProxyMethods.forEach((method) => { + PromiseManyArray.prototype[method] = function proxiedMethod(...args) { + deprecate( + `The ${method} method on ember-data's PromiseManyArray is deprecated. await the promise and work with the ManyArray directly.`, + false, + { + id: 'ember-data:deprecate-promise-many-array-behaviors', + until: '5.0', + since: { enabled: '4.8', available: '4.8' }, + for: 'ember-data', + } + ); + assert(`Cannot call ${method} before content is assigned.`, this.content); + return this.content[method](...args); + }; + }); +} diff --git a/packages/private-build-infra/addon/current-deprecations.ts b/packages/private-build-infra/addon/current-deprecations.ts index 64b70f3a597..df8eea70cb6 100644 --- a/packages/private-build-infra/addon/current-deprecations.ts +++ b/packages/private-build-infra/addon/current-deprecations.ts @@ -50,4 +50,5 @@ export default { DEPRECATE_EARLY_STATIC: '4.8', DEPRECATE_CLASSIC: '4.9', DEPRECATE_HELPERS: '4.8', + DEPRECATE_PROMISE_MANY_ARRAY_BEHAVIORS: '4.8', }; diff --git a/packages/private-build-infra/addon/deprecations.ts b/packages/private-build-infra/addon/deprecations.ts index f0dd718a135..acf33fd4e0a 100644 --- a/packages/private-build-infra/addon/deprecations.ts +++ b/packages/private-build-infra/addon/deprecations.ts @@ -18,4 +18,4 @@ export const DEPRECATE_JSON_API_FALLBACK = deprecationState('DEPRECATE_JSON_API_ export const DEPRECATE_MODEL_REOPEN = deprecationState('DEPRECATE_MODEL_REOPEN'); export const DEPRECATE_EARLY_STATIC = deprecationState('DEPRECATE_EARLY_STATIC'); export const DEPRECATE_CLASSIC = deprecationState('DEPRECATE_CLASSIC'); -export const DEPRECATE_HELPERS = deprecationState('DEPRECATE_HELPERS'); +export const DEPRECATE_PROMISE_MANY_ARRAY_BEHAVIORS = deprecationState('DEPRECATE_PROMISE_MANY_ARRAY_BEHAVIORS'); diff --git a/packages/record-data/addon/-private/record-data.ts b/packages/record-data/addon/-private/record-data.ts index 393c3470863..dfbdb59ab6c 100644 --- a/packages/record-data/addon/-private/record-data.ts +++ b/packages/record-data/addon/-private/record-data.ts @@ -556,20 +556,6 @@ export default class RecordDataDefault implements RelationshipRecordData { return array; } - isAttrDirty(key: string): boolean { - if (this._attributes[key] === undefined) { - return false; - } - let originalValue; - if (this._inFlightAttributes[key] !== undefined) { - originalValue = this._inFlightAttributes[key]; - } else { - originalValue = this._data[key]; - } - - return originalValue !== this._attributes[key]; - } - get _attributes() { if (this.__attributes === null) { this.__attributes = Object.create(null); @@ -661,20 +647,6 @@ export default class RecordDataDefault implements RelationshipRecordData { return createOptions; } - /* - TODO IGOR AND DAVID this shouldn't be public - This method should only be called by records in the `isNew()` state OR once the record - has been deleted and that deletion has been persisted. - - It will remove this record from any associated relationships. - - If `isNew` is true (default false), it will also completely reset all - relationships to an empty state as well. - - @method removeFromInverseRelationships - @param {Boolean} isNew whether to unload from the `isNew` perspective - @private - */ removeFromInverseRelationships() { graphFor(this.storeWrapper).push({ op: 'deleteRecord', diff --git a/packages/store/addon/-private/record-arrays/adapter-populated-record-array.ts b/packages/store/addon/-private/record-arrays/adapter-populated-record-array.ts index b47e5c38856..98a643ccc76 100644 --- a/packages/store/addon/-private/record-arrays/adapter-populated-record-array.ts +++ b/packages/store/addon/-private/record-arrays/adapter-populated-record-array.ts @@ -48,9 +48,7 @@ export interface AdapterPopulatedRecordArrayCreateArgs { // GET /users?isAdmin=true store.query('user', { isAdmin: true }).then(function(admins) { - admins.then(function() { - console.log(admins.get("length")); // 42 - }); + admins.get("length"); // 42 // somewhere later in the app code, when new admins have been created // in the meantime @@ -58,7 +56,7 @@ export interface AdapterPopulatedRecordArrayCreateArgs { // GET /users?isAdmin=true admins.update().then(function() { admins.get('isUpdating'); // false - console.log(admins.get("length")); // 123 + admins.get("length"); // 123 }); admins.get('isUpdating'); // true diff --git a/packages/store/addon/-private/store-service.ts b/packages/store/addon/-private/store-service.ts index b2c33a781ed..b84369b4cb3 100644 --- a/packages/store/addon/-private/store-service.ts +++ b/packages/store/addon/-private/store-service.ts @@ -33,7 +33,7 @@ import type { MinimumAdapterInterface } from '@ember-data/types/q/minimum-adapte import type { MinimumSerializerInterface } from '@ember-data/types/q/minimum-serializer-interface'; import type { RecordData } from '@ember-data/types/q/record-data'; import { JsonApiValidationError } from '@ember-data/types/q/record-data-json-api'; -import type { RecordDataRecordWrapper } from '@ember-data/types/q/record-data-record-wrapper'; +import type { RecordDataWrapper } from '@ember-data/types/q/record-data-record-wrapper'; import type { RecordInstance } from '@ember-data/types/q/record-instance'; import type { SchemaDefinitionService } from '@ember-data/types/q/schema-definition-service'; import type { FindOptions } from '@ember-data/types/q/store'; @@ -285,7 +285,7 @@ class Store extends Service { instantiateRecord( identifier: StableRecordIdentifier, createRecordArgs: { [key: string]: unknown }, - recordDataFor: (identifier: StableRecordIdentifier) => RecordDataRecordWrapper, + recordDataFor: (identifier: StableRecordIdentifier) => RecordDataWrapper, notificationManager: NotificationManager ): DSModel | RecordInstance { if (HAS_MODEL_PACKAGE) { @@ -466,7 +466,7 @@ class Store extends Service { const identifier = this.identifierCache.peekRecordIdentifier(resource as ResourceIdentifierObject); assert( - `The id ${properties.id} has already been used with another '${normalizeModelName}' record.`, + `The id ${properties.id} has already been used with another '${normalizedModelName}' record.`, !identifier ); } @@ -1352,7 +1352,7 @@ class Store extends Service { ```javascript store.queryRecord('user', {}).then(function(user) { let username = user.get('username'); - console.log(`Currently logged in as ${username}`); + // do thing }); ``` @@ -1409,7 +1409,7 @@ class Store extends Service { ```javascript store.queryRecord('user', { username: 'unique' }).then(function(user) { - console.log(user); // null + // user is null }); ```