Skip to content

Commit

Permalink
fix: fix MandatorySetter error for Proxied PromiseProxy, silence embe…
Browse files Browse the repository at this point in the history
…r-concurrency deprecations (#8206)

* npm ignore publish for yarn-error.log

* dont wrap in deprecation proxy in prod builds

* add failing test to replicate bug

* add fix

* restore original deprecation

* cleanup

* more cleanup

* dont deprecate for ec, fix prod test
  • Loading branch information
runspired authored Oct 14, 2022
1 parent 24645d3 commit 1b35d2a
Show file tree
Hide file tree
Showing 5 changed files with 148 additions and 33 deletions.
2 changes: 1 addition & 1 deletion packages/-ember-data/.npmignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
/tmp/
/types/
**/*.d.ts
yarn-error.log

# dependencies
/bower_components/
Expand Down Expand Up @@ -42,4 +43,3 @@ lib/scripts

# whitelist yuidoc's data.json for api docs generation
!/dist/docs/data.json

Original file line number Diff line number Diff line change
@@ -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');
}
);
});
30 changes: 12 additions & 18 deletions packages/-ember-data/tests/integration/records/save-test.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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, `<proxy>.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', `<proxy>.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, `<proxy>.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 });
}
});

Expand Down
36 changes: 29 additions & 7 deletions packages/model/addon/-private/deprecated-promise-proxy.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { deprecate } from '@ember/debug';
import { get } from '@ember/object';
import { DEBUG } from '@glimmer/env';

import { resolve } from 'rsvp';

Expand All @@ -12,18 +14,36 @@ function promiseObject<T>(promise: Promise<T>): PromiseObject<T> {

// 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<T>(promise: Promise<T>): PromiseObject<T> {
const promiseObjectProxy: PromiseObject<T> = 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',
Expand All @@ -35,15 +55,17 @@ export function deprecatedPromiseObject<T>(promise: Promise<T>): PromiseObject<T
},
}
);
} 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;
Expand Down
42 changes: 35 additions & 7 deletions packages/store/addon/-private/proxies/promise-proxies.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -105,6 +107,7 @@ function _promiseArray<I, T extends EmberArrayLike<I>>(promise: Promise<T>, 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',
'[]',
Expand All @@ -123,11 +126,17 @@ const PROXIED_OBJECT_PROPS = ['content', 'isPending', 'isSettled', 'isRejected',

export function promiseArray<I, T extends EmberArrayLike<I>>(promise: Promise<T>): PromiseArray<I, T> {
const promiseObjectProxy: PromiseArray<I, T> = _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`,
Expand Down Expand Up @@ -160,13 +169,30 @@ export function promiseArray<I, T extends EmberArrayLike<I>>(promise: Promise<T>
return new Proxy(promiseObjectProxy, handler);
}

const ProxySymbolString = String(Symbol.for('PROXY_CONTENT'));

export function promiseObject<T>(promise: Promise<T>): PromiseObjectProxy<T> {
const promiseObjectProxy: PromiseObjectProxy<T> = _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`,
Expand All @@ -181,15 +207,17 @@ export function promiseObject<T>(promise: Promise<T>): PromiseObjectProxy<T> {
},
}
);
} 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;
Expand Down

0 comments on commit 1b35d2a

Please sign in to comment.