Skip to content

Commit 0aa4e5f

Browse files
Chris GarrettRobert Jackson
Chris Garrett
authored and
Robert Jackson
committed
[BUGFIX release] Fixes tag chaining on Proxy mixins
Currently, the Proxy mixin uses the private `UNKNOWN_PROPERTY_TAG` API to provide the tags for the content property that the mixin is proxying to. However, previously the chains system would do this _and_ still entangle changes to the local property. This allowed users to manually notify in subclasses of Proxy. This PR adds the tag for the property back to the returned tag, so it works the same as before. It also refactors `setupMandatorySetter`, since we now have to do that work in two places (to avoid re-entry).
1 parent dceadb9 commit 0aa4e5f

File tree

4 files changed

+47
-5
lines changed

4 files changed

+47
-5
lines changed

packages/@ember/-internals/metal/lib/tags.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ export function createTagForProperty(object: object, propertyKey: string | symbo
3232
let newTag = createUpdatableTag();
3333

3434
if (DEBUG) {
35-
setupMandatorySetter!(object, propertyKey);
35+
setupMandatorySetter!(newTag, object, propertyKey);
3636

3737
(newTag as any)._propertyKey = propertyKey;
3838
}

packages/@ember/-internals/runtime/lib/mixins/-proxy.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,12 @@ export default Mixin.create({
6363
}),
6464

6565
[CUSTOM_TAG_FOR](key) {
66+
let tag = createTagForProperty(this, key);
67+
6668
if (key in this) {
67-
return createTagForProperty(this, key);
69+
return tag;
6870
} else {
69-
return combine(getChainTagsForKey(this, `content.${key}`));
71+
return combine([tag, ...getChainTagsForKey(this, `content.${key}`)]);
7072
}
7173
},
7274

packages/@ember/-internals/runtime/tests/system/object_proxy_test.js

+28
Original file line numberDiff line numberDiff line change
@@ -338,5 +338,33 @@ moduleFor(
338338
observe: observer('foo', function() {}),
339339
}).create();
340340
}
341+
342+
async '@test custom proxies should be able to notify property changes manually'(assert) {
343+
let proxy = ObjectProxy.extend({
344+
locals: { foo: 123 },
345+
346+
unknownProperty(key) {
347+
return this.locals[key];
348+
},
349+
350+
setUnknownProperty(key, value) {
351+
this.locals[key] = value;
352+
this.notifyPropertyChange(key);
353+
},
354+
}).create();
355+
356+
let count = 0;
357+
358+
proxy.addObserver('foo', function() {
359+
count++;
360+
});
361+
362+
proxy.set('foo', 456);
363+
await runLoopSettled();
364+
365+
assert.equal(count, 1);
366+
assert.equal(proxy.get('foo'), 456);
367+
assert.equal(proxy.get('locals.foo'), 456);
368+
}
341369
}
342370
);

packages/@ember/-internals/utils/lib/mandatory-setter.ts

+14-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
import { assert } from '@ember/debug';
2+
import { _WeakSet as WeakSet } from '@ember/polyfills';
23
import { DEBUG } from '@glimmer/env';
4+
import { Tag } from '@glimmer/reference';
35
import lookupDescriptor from './lookup-descriptor';
46

5-
export let setupMandatorySetter: ((obj: object, keyName: string | symbol) => void) | undefined;
7+
export let setupMandatorySetter:
8+
| ((tag: Tag, obj: object, keyName: string | symbol) => void)
9+
| undefined;
610
export let teardownMandatorySetter: ((obj: object, keyName: string | symbol) => void) | undefined;
711
export let setWithMandatorySetter:
812
| ((obj: object, keyName: string | symbol, value: any) => void)
@@ -11,6 +15,8 @@ export let setWithMandatorySetter:
1115
type PropertyDescriptorWithMeta = PropertyDescriptor & { hadOwnProperty?: boolean };
1216

1317
if (DEBUG) {
18+
let SEEN_TAGS = new WeakSet();
19+
1420
let MANDATORY_SETTERS: WeakMap<
1521
object,
1622
// @ts-ignore
@@ -21,7 +27,13 @@ if (DEBUG) {
2127
return Object.prototype.propertyIsEnumerable.call(obj, key);
2228
};
2329

24-
setupMandatorySetter = function(obj: object, keyName: string | symbol) {
30+
setupMandatorySetter = function(tag: Tag, obj: object, keyName: string | symbol) {
31+
if (SEEN_TAGS.has(tag)) {
32+
return;
33+
}
34+
35+
SEEN_TAGS!.add(tag);
36+
2537
let desc = (lookupDescriptor(obj, keyName) as PropertyDescriptorWithMeta) || {};
2638

2739
if (desc.get || desc.set) {

0 commit comments

Comments
 (0)