diff --git a/packages/-ember-data/.npmignore b/packages/-ember-data/.npmignore index f081b33ade6..d4910ea2c6f 100644 --- a/packages/-ember-data/.npmignore +++ b/packages/-ember-data/.npmignore @@ -4,6 +4,7 @@ /tmp/ /types/ **/*.d.ts +yarn-error.log # dependencies /bower_components/ @@ -42,4 +43,3 @@ lib/scripts # whitelist yuidoc's data.json for api docs generation !/dist/docs/data.json - diff --git a/packages/-ember-data/tests/acceptance/tracking-promise-flags-test.js b/packages/-ember-data/tests/acceptance/tracking-promise-flags-test.js new file mode 100644 index 00000000000..9603d27e2b5 --- /dev/null +++ b/packages/-ember-data/tests/acceptance/tracking-promise-flags-test.js @@ -0,0 +1,71 @@ +import { render, settled } from '@ember/test-helpers'; + +import hbs from 'htmlbars-inline-precompile'; +import { module } from 'qunit'; + +import { setupRenderingTest } from 'ember-qunit'; + +import JSONAPIAdapter from '@ember-data/adapter/json-api'; +import Model, { attr } from '@ember-data/model'; +import Store from '@ember-data/store'; +import { deprecatedTest } from '@ember-data/unpublished-test-infra/test-support/deprecated-test'; + +class Widget extends Model { + @attr name; +} + +module('acceptance/tracking-promise-flags', function (hooks) { + setupRenderingTest(hooks); + + hooks.beforeEach(function () { + let { owner } = this; + owner.register('service:store', Store); + owner.register('model:widget', Widget); + owner.register( + 'serializer:application', + class { + normalizeResponse = (_, __, data) => data; + static create() { + return new this(); + } + } + ); + }); + + deprecatedTest( + 'can track isPending', + { id: 'ember-data:deprecate-promise-proxies', until: '5.0', count: 6 }, + async function (assert) { + const { owner } = this; + let resolve; + class TestAdapter extends JSONAPIAdapter { + findRecord() { + return new Promise((r) => { + resolve = r; + }); + } + } + owner.register('adapter:application', TestAdapter); + let store = owner.lookup('service:store'); + store.DISABLE_WAITER = true; + this.model = store.findRecord('widget', '1'); + + await render(hbs`{{#if this.model.isPending}}Pending{{else}}{{this.model.name}}{{/if}}`); + + assert.dom().containsText('Pending'); + + resolve({ + data: { + id: '1', + type: 'widget', + attributes: { + name: 'Contraption', + }, + }, + }); + await settled(); + + assert.dom().containsText('Contraption'); + } + ); +}); diff --git a/packages/-ember-data/tests/integration/records/save-test.js b/packages/-ember-data/tests/integration/records/save-test.js index 59569fee7a8..96f76976fef 100644 --- a/packages/-ember-data/tests/integration/records/save-test.js +++ b/packages/-ember-data/tests/integration/records/save-test.js @@ -1,5 +1,4 @@ import { run } from '@ember/runloop'; -import { DEBUG } from '@glimmer/env'; import { module, test } from 'qunit'; import { defer, reject, resolve } from 'rsvp'; @@ -39,37 +38,32 @@ module('integration/records/save - Save Record', function (hooks) { if (DEPRECATE_SAVE_PROMISE_ACCESS) { // `save` returns a PromiseObject which allows to call get on it - assert.strictEqual(saved.get('id'), undefined); + assert.strictEqual(saved.get('id'), undefined, `.get('id') is undefined before save resolves`); } deferred.resolve({ data: { id: '123', type: 'post' } }); let model = await saved; assert.ok(true, 'save operation was resolved'); if (DEPRECATE_SAVE_PROMISE_ACCESS) { - assert.strictEqual(saved.get('id'), '123'); - if (DEBUG) { - try { - saved.id; - assert.ok(false, 'access should error with .get assertion'); - } catch { - assert.ok(true, 'access errored correctly'); - } - } - - assert.strictEqual(model.id, '123'); + assert.strictEqual(saved.get('id'), '123', `.get('id') is '123' after save resolves`); + assert.strictEqual(model.id, '123', `record.id is '123' after save resolves`); } else { - assert.strictEqual(saved.id, undefined); - assert.strictEqual(model.id, '123'); + assert.strictEqual(saved.id, undefined, `.id is undefined after save resolves`); + assert.strictEqual(model.id, '123', `record.id is '123' after save resolves`); } assert.strictEqual(model, post, 'resolves with the model'); if (DEPRECATE_SAVE_PROMISE_ACCESS) { // We don't care about the exact value of the property, but accessing it // should not throw an error and only show a deprecation. saved.__ec_cancel__ = true; - assert.strictEqual(saved.__ec_cancel__, undefined); - assert.strictEqual(model.__ec_cancel__, undefined); + assert.true(saved.__ec_cancel__, '__ec_cancel__ can be accessed on the proxy'); + assert.strictEqual( + model.__ec_cancel__, + undefined, + '__ec_cancel__ can be accessed on the record but is not present' + ); - assert.expectDeprecation({ id: 'ember-data:model-save-promise', count: 4 }); + assert.expectDeprecation({ id: 'ember-data:model-save-promise', count: 10 }); } }); diff --git a/packages/model/addon/-private/deprecated-promise-proxy.ts b/packages/model/addon/-private/deprecated-promise-proxy.ts index 15f4143a10b..2276c0849f9 100644 --- a/packages/model/addon/-private/deprecated-promise-proxy.ts +++ b/packages/model/addon/-private/deprecated-promise-proxy.ts @@ -1,4 +1,6 @@ import { deprecate } from '@ember/debug'; +import { get } from '@ember/object'; +import { DEBUG } from '@glimmer/env'; import { resolve } from 'rsvp'; @@ -12,18 +14,36 @@ function promiseObject(promise: Promise): PromiseObject { // constructor is accessed in some internals but not including it in the copyright for the deprecation const ALLOWABLE_METHODS = ['constructor', 'then', 'catch', 'finally']; +const ALLOWABLE_PROPS = ['__ec_yieldable__', '__ec_cancel__']; const PROXIED_OBJECT_PROPS = ['content', 'isPending', 'isSettled', 'isRejected', 'isFulfilled', 'promise', 'reason']; +const ProxySymbolString = String(Symbol.for('PROXY_CONTENT')); + export function deprecatedPromiseObject(promise: Promise): PromiseObject { const promiseObjectProxy: PromiseObject = promiseObject(promise); + if (!DEBUG) { + return promiseObjectProxy; + } const handler = { - get(target: object, prop: string, receiver?: object): unknown { + get(target: object, prop: string, receiver: object): unknown { if (typeof prop === 'symbol') { + if (String(prop) === ProxySymbolString) { + return; + } return Reflect.get(target, prop, receiver); } + + if (prop === 'constructor') { + return target.constructor; + } + + if (ALLOWABLE_PROPS.includes(prop)) { + return target[prop]; + } + if (!ALLOWABLE_METHODS.includes(prop)) { deprecate( - `Accessing ${prop} is deprecated. The return type is being changed fomr PromiseObjectProxy to a Promise. The only available methods to access on this promise are .then, .catch and .finally`, + `Accessing ${prop} is deprecated. The return type is being changed from PromiseObjectProxy to a Promise. The only available methods to access on this promise are .then, .catch and .finally`, false, { id: 'ember-data:model-save-promise', @@ -35,15 +55,17 @@ export function deprecatedPromiseObject(promise: Promise): PromiseObject unknown).bind(target); } - const value: unknown = target[prop]; - if (value && typeof value === 'function' && typeof value.bind === 'function') { - return value.bind(target); + if (PROXIED_OBJECT_PROPS.includes(prop)) { + return target[prop]; } - if (PROXIED_OBJECT_PROPS.includes(prop)) { - return value; + const value: unknown = get(target, prop); + if (value && typeof value === 'function' && typeof value.bind === 'function') { + return value.bind(receiver); } return undefined; diff --git a/packages/store/addon/-private/proxies/promise-proxies.ts b/packages/store/addon/-private/proxies/promise-proxies.ts index 7cbce4a4137..e8b1f2321ba 100644 --- a/packages/store/addon/-private/proxies/promise-proxies.ts +++ b/packages/store/addon/-private/proxies/promise-proxies.ts @@ -1,6 +1,8 @@ import { deprecate } from '@ember/debug'; +import { get } from '@ember/object'; import type ComputedProperty from '@ember/object/computed'; import { reads } from '@ember/object/computed'; +import { DEBUG } from '@glimmer/env'; import { resolve } from 'rsvp'; @@ -105,6 +107,7 @@ function _promiseArray>(promise: Promise, labe // constructor is accessed in some internals but not including it in the copyright for the deprecation const ALLOWABLE_METHODS = ['constructor', 'then', 'catch', 'finally']; +const ALLOWABLE_PROPS = ['__ec_yieldable__', '__ec_cancel__']; const PROXIED_ARRAY_PROPS = [ 'length', '[]', @@ -123,11 +126,17 @@ const PROXIED_OBJECT_PROPS = ['content', 'isPending', 'isSettled', 'isRejected', export function promiseArray>(promise: Promise): PromiseArray { const promiseObjectProxy: PromiseArray = _promiseArray(promise); + if (!DEBUG) { + return promiseObjectProxy; + } const handler = { - get(target: object, prop: string, receiver?: object): unknown { + get(target: object, prop: string, receiver: object): unknown { if (typeof prop === 'symbol') { return Reflect.get(target, prop, receiver); } + if (ALLOWABLE_PROPS.includes(prop)) { + return receiver[prop]; + } if (!ALLOWABLE_METHODS.includes(prop)) { deprecate( `Accessing ${prop} on this PromiseArray is deprecated. The return type is being changed from PromiseArray to a Promise. The only available methods to access on this promise are .then, .catch and .finally`, @@ -160,13 +169,30 @@ export function promiseArray>(promise: Promise return new Proxy(promiseObjectProxy, handler); } +const ProxySymbolString = String(Symbol.for('PROXY_CONTENT')); + export function promiseObject(promise: Promise): PromiseObjectProxy { const promiseObjectProxy: PromiseObjectProxy = _promiseObject(promise); + if (!DEBUG) { + return promiseObjectProxy; + } const handler = { - get(target: object, prop: string, receiver?: object): unknown { + get(target: object, prop: string, receiver: object): unknown { if (typeof prop === 'symbol') { + if (String(prop) === ProxySymbolString) { + return; + } return Reflect.get(target, prop, receiver); } + + if (prop === 'constructor') { + return target.constructor; + } + + if (ALLOWABLE_PROPS.includes(prop)) { + return receiver[prop]; + } + if (!ALLOWABLE_METHODS.includes(prop)) { deprecate( `Accessing ${prop} on this PromiseObject is deprecated. The return type is being changed from PromiseObject to a Promise. The only available methods to access on this promise are .then, .catch and .finally`, @@ -181,15 +207,17 @@ export function promiseObject(promise: Promise): PromiseObjectProxy { }, } ); + } else { + return (target[prop] as () => unknown).bind(target); } - const value: unknown = target[prop]; - if (value && typeof value === 'function' && typeof value.bind === 'function') { - return value.bind(target); + if (PROXIED_OBJECT_PROPS.includes(prop)) { + return target[prop]; } - if (PROXIED_OBJECT_PROPS.includes(prop)) { - return value; + const value: unknown = get(target, prop); + if (value && typeof value === 'function' && typeof value.bind === 'function') { + return value.bind(receiver); } return undefined;