Skip to content

Commit 4f10138

Browse files
rwjbluerwjblue
authored
Merge pull request #18721 from emberjs/backport/autotracking-bugfixes
[BUGFIX release] Backport autotracking bugfixes
2 parents 23d3ff0 + a33a246 commit 4f10138

File tree

12 files changed

+215
-66
lines changed

12 files changed

+215
-66
lines changed

‎packages/@ember/-internals/glimmer/lib/component-managers/custom.ts

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { ARGS_PROXY_TAGS, consume } from '@ember/-internals/metal';
1+
import { consume, CUSTOM_TAG_FOR } from '@ember/-internals/metal';
22
import { Factory } from '@ember/-internals/owner';
33
import { HAS_NATIVE_PROXY } from '@ember/-internals/utils';
44
import { OwnedTemplateMeta } from '@ember/-internals/views';
@@ -192,34 +192,41 @@ export default class CustomComponentManager<ComponentInstance>
192192
): CustomComponentState<ComponentInstance> {
193193
const { delegate } = definition;
194194
const capturedArgs = args.capture();
195+
const namedArgs = capturedArgs.named;
195196

196197
let value;
197198
let namedArgsProxy = {};
198199

199200
if (EMBER_CUSTOM_COMPONENT_ARG_PROXY) {
201+
let getTag = (key: string) => {
202+
return namedArgs.get(key).tag;
203+
};
204+
200205
if (HAS_NATIVE_PROXY) {
201206
let handler: ProxyHandler<{}> = {
202207
get(_target, prop) {
203-
if (capturedArgs.named.has(prop as string)) {
204-
let ref = capturedArgs.named.get(prop as string);
208+
if (namedArgs.has(prop as string)) {
209+
let ref = namedArgs.get(prop as string);
205210
consume(ref.tag);
206211

207212
return ref.value();
213+
} else if (prop === CUSTOM_TAG_FOR) {
214+
return getTag;
208215
}
209216
},
210217

211218
has(_target, prop) {
212-
return capturedArgs.named.has(prop as string);
219+
return namedArgs.has(prop as string);
213220
},
214221

215222
ownKeys(_target) {
216-
return capturedArgs.named.names;
223+
return namedArgs.names;
217224
},
218225

219226
getOwnPropertyDescriptor(_target, prop) {
220227
assert(
221228
'args proxies do not have real property descriptors, so you should never need to call getOwnPropertyDescriptor yourself. This code exists for enumerability, such as in for-in loops and Object.keys()',
222-
capturedArgs.named.has(prop as string)
229+
namedArgs.has(prop as string)
223230
);
224231

225232
return {
@@ -243,12 +250,18 @@ export default class CustomComponentManager<ComponentInstance>
243250

244251
namedArgsProxy = new Proxy(namedArgsProxy, handler);
245252
} else {
246-
capturedArgs.named.names.forEach(name => {
253+
Object.defineProperty(namedArgsProxy, CUSTOM_TAG_FOR, {
254+
configurable: false,
255+
enumerable: false,
256+
value: getTag,
257+
});
258+
259+
namedArgs.names.forEach(name => {
247260
Object.defineProperty(namedArgsProxy, name, {
248261
enumerable: true,
249262
configurable: true,
250263
get() {
251-
let ref = capturedArgs.named.get(name);
264+
let ref = namedArgs.get(name);
252265
consume(ref.tag);
253266

254267
return ref.value();
@@ -257,8 +270,6 @@ export default class CustomComponentManager<ComponentInstance>
257270
});
258271
}
259272

260-
ARGS_PROXY_TAGS.set(namedArgsProxy, capturedArgs.named);
261-
262273
value = {
263274
named: namedArgsProxy,
264275
positional: capturedArgs.positional.value(),

‎packages/@ember/-internals/glimmer/tests/integration/components/tracked-test.js

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { Object as EmberObject, A, ArrayProxy, PromiseProxyMixin } from '@ember/-internals/runtime';
22
import { EMBER_CUSTOM_COMPONENT_ARG_PROXY } from '@ember/canary-features';
3-
import { computed, tracked, nativeDescDecorator as descriptor } from '@ember/-internals/metal';
3+
import { computed, get, tracked, nativeDescDecorator as descriptor } from '@ember/-internals/metal';
44
import { Promise } from 'rsvp';
55
import { moduleFor, RenderingTestCase, strip, runTask } from 'internal-test-helpers';
66
import GlimmerishComponent from '../../utils/glimmerish-component';
@@ -568,6 +568,50 @@ if (EMBER_CUSTOM_COMPONENT_ARG_PROXY) {
568568
this.assertText('hello!');
569569
}
570570

571+
'@test args can be accessed with get()'() {
572+
class TestComponent extends GlimmerishComponent {
573+
get text() {
574+
return get(this, 'args.text');
575+
}
576+
}
577+
578+
this.registerComponent('test', {
579+
ComponentClass: TestComponent,
580+
template: '<p>{{this.text}}</p>',
581+
});
582+
583+
this.render('<Test @text={{this.text}}/>', {
584+
text: 'hello!',
585+
});
586+
587+
this.assertText('hello!');
588+
589+
runTask(() => this.context.set('text', 'hello world!'));
590+
this.assertText('hello world!');
591+
592+
runTask(() => this.context.set('text', 'hello!'));
593+
this.assertText('hello!');
594+
}
595+
596+
'@test args can be accessed with get() if no value is passed'() {
597+
class TestComponent extends GlimmerishComponent {
598+
get text() {
599+
return get(this, 'args.text') || 'hello!';
600+
}
601+
}
602+
603+
this.registerComponent('test', {
604+
ComponentClass: TestComponent,
605+
template: '<p>{{this.text}}</p>',
606+
});
607+
608+
this.render('<Test/>', {
609+
text: 'hello!',
610+
});
611+
612+
this.assertText('hello!');
613+
}
614+
571615
'@test named args are enumerable'() {
572616
class TestComponent extends GlimmerishComponent {
573617
get objectKeys() {

‎packages/@ember/-internals/glimmer/tests/integration/helpers/get-test.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { RenderingTestCase, moduleFor, runTask } from 'internal-test-helpers';
33
import { set, get } from '@ember/-internals/metal';
44

55
import { Component } from '../../utils/helpers';
6+
import GlimmerishComponent from '../../utils/glimmerish-component';
67

78
moduleFor(
89
'Helpers test: {{get}}',
@@ -616,5 +617,31 @@ moduleFor(
616617

617618
assert.strictEqual(this.$('#get-input').val(), 'mcintosh');
618619
}
620+
621+
'@test should be able to get an object value with a path from this.args in a glimmer component'() {
622+
class PersonComponent extends GlimmerishComponent {
623+
options = ['first', 'last', 'age'];
624+
}
625+
626+
this.registerComponent('person-wrapper', {
627+
ComponentClass: PersonComponent,
628+
template: '{{#each this.options as |option|}}{{get this.args option}}{{/each}}',
629+
});
630+
631+
this.render('<PersonWrapper @first={{first}} @last={{last}} @age={{age}}/>', {
632+
first: 'miguel',
633+
last: 'andrade',
634+
});
635+
636+
this.assertText('miguelandrade');
637+
638+
runTask(() => this.rerender());
639+
640+
this.assertText('miguelandrade');
641+
642+
runTask(() => set(this.context, 'age', 30));
643+
644+
this.assertText('miguelandrade30');
645+
}
619646
}
620647
);

‎packages/@ember/-internals/glimmer/tests/integration/syntax/each-test.js

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { moduleFor, RenderingTestCase, applyMixins, strip, runTask } from 'internal-test-helpers';
22

3-
import { get, set, notifyPropertyChange } from '@ember/-internals/metal';
3+
import { get, set, notifyPropertyChange, computed } from '@ember/-internals/metal';
44
import { A as emberA, ArrayProxy, RSVP } from '@ember/-internals/runtime';
55
import { HAS_NATIVE_SYMBOL } from '@ember/-internals/utils';
66

@@ -1089,6 +1089,25 @@ moduleFor(
10891089
}
10901090
);
10911091

1092+
moduleFor(
1093+
'Syntax test: {{#each}} with array proxies, arrangedContent depends on external content',
1094+
class extends EachTest {
1095+
createList(items) {
1096+
let wrapped = emberA(items);
1097+
let proxy = ArrayProxy.extend({
1098+
arrangedContent: computed('wrappedItems.[]', function() {
1099+
// Slice the items to ensure that updates must be propogated
1100+
return this.wrappedItems.slice();
1101+
}),
1102+
}).create({
1103+
wrappedItems: wrapped,
1104+
});
1105+
1106+
return { list: proxy, delegate: wrapped };
1107+
}
1108+
}
1109+
);
1110+
10921111
moduleFor(
10931112
'Syntax test: {{#each as}} undefined path',
10941113
class extends RenderingTestCase {

‎packages/@ember/-internals/metal/index.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ export {
4444
isClassicDecorator,
4545
setClassicDecorator,
4646
} from './lib/descriptor_map';
47-
export { getChainTagsForKey, ARGS_PROXY_TAGS } from './lib/chain-tags';
47+
export { getChainTagsForKey } from './lib/chain-tags';
4848
export { default as libraries, Libraries } from './lib/libraries';
4949
export { default as getProperties } from './lib/get_properties';
5050
export { default as setProperties } from './lib/set_properties';
@@ -53,7 +53,13 @@ export { default as expandProperties } from './lib/expand_properties';
5353
export { addObserver, activateObserver, removeObserver, flushAsyncObservers } from './lib/observer';
5454
export { Mixin, aliasMethod, mixin, observer, applyMixin } from './lib/mixin';
5555
export { default as inject, DEBUG_INJECTION_FUNCTIONS } from './lib/injected_property';
56-
export { tagForProperty, tagFor, markObjectAsDirty, UNKNOWN_PROPERTY_TAG } from './lib/tags';
56+
export {
57+
tagForProperty,
58+
createTagForProperty,
59+
tagFor,
60+
markObjectAsDirty,
61+
CUSTOM_TAG_FOR,
62+
} from './lib/tags';
5763
export {
5864
consume,
5965
Tracker,

‎packages/@ember/-internals/metal/lib/array_events.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ export function arrayContentDidChange<T extends { length: number }>(
3232
array: T,
3333
startIdx: number,
3434
removeAmt: number,
35-
addAmt: number
35+
addAmt: number,
36+
notify = true
3637
): T {
3738
// if no args are passed assume everything changes
3839
if (startIdx === undefined) {
@@ -50,11 +51,13 @@ export function arrayContentDidChange<T extends { length: number }>(
5051

5152
let meta = peekMeta(array);
5253

53-
if (addAmt < 0 || removeAmt < 0 || addAmt - removeAmt !== 0) {
54-
notifyPropertyChange(array, 'length', meta);
55-
}
54+
if (notify) {
55+
if (addAmt < 0 || removeAmt < 0 || addAmt - removeAmt !== 0) {
56+
notifyPropertyChange(array, 'length', meta);
57+
}
5658

57-
notifyPropertyChange(array, '[]', meta);
59+
notifyPropertyChange(array, '[]', meta);
60+
}
5861

5962
sendEvent(array, '@array:change', [array, startIdx, removeAmt, addAmt]);
6063

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

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ import { getLastRevisionFor, peekCacheFor } from './computed_cache';
66
import { descriptorForProperty } from './descriptor_map';
77
import { tagForProperty } from './tags';
88

9-
export const ARGS_PROXY_TAGS = new WeakMap();
10-
119
export function finishLazyChains(obj: any, key: string, value: any) {
1210
let meta = peekMeta(obj);
1311
let lazyTags = meta !== null ? meta.readableLazyChainsFor(key) : undefined;
@@ -141,40 +139,6 @@ export function getChainTagsForKey(obj: any, path: string) {
141139
break;
142140
}
143141

144-
// If the segment is linking to an args proxy, we need to manually access
145-
// the tags for the args, since they are direct references and don't have a
146-
// tagForProperty. We then continue chaining like normal after it, since
147-
// you could chain off an arg if it were an object, for instance.
148-
if (segment === 'args' && ARGS_PROXY_TAGS.has(current.args)) {
149-
assert(
150-
`When watching the 'args' on a GlimmerComponent, you must watch a value on the args. You cannot watch the object itself, as it never changes.`,
151-
segmentEnd !== pathLength
152-
);
153-
154-
lastSegmentEnd = segmentEnd + 1;
155-
segmentEnd = path.indexOf('.', lastSegmentEnd);
156-
157-
if (segmentEnd === -1) {
158-
segmentEnd = pathLength;
159-
}
160-
161-
segment = path.slice(lastSegmentEnd, segmentEnd)!;
162-
163-
let namedArgs = ARGS_PROXY_TAGS.get(current.args);
164-
let ref = namedArgs.get(segment);
165-
166-
chainTags.push(ref.tag);
167-
168-
// We still need to break if we're at the end of the path.
169-
if (segmentEnd === pathLength) {
170-
break;
171-
}
172-
173-
// Otherwise, set the current value and then continue to the next segment
174-
current = ref.value();
175-
continue;
176-
}
177-
178142
// TODO: Assert that current[segment] isn't an undecorated, non-MANDATORY_SETTER/dependentKeyCompat getter
179143

180144
let propertyTag = tagForProperty(current, segment);

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

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,28 @@ import { DEBUG } from '@glimmer/env';
55
import { CONSTANT_TAG, createUpdatableTag, dirty, Tag } from '@glimmer/reference';
66
import { assertTagNotConsumed } from './tracked';
77

8-
export const UNKNOWN_PROPERTY_TAG = symbol('UNKNOWN_PROPERTY_TAG');
8+
export const CUSTOM_TAG_FOR = symbol('CUSTOM_TAG_FOR');
99

1010
export function tagForProperty(object: any, propertyKey: string | symbol, _meta?: Meta): Tag {
1111
let objectType = typeof object;
1212
if (objectType !== 'function' && (objectType !== 'object' || object === null)) {
1313
return CONSTANT_TAG;
1414
}
15-
let meta = _meta === undefined ? metaFor(object) : _meta;
1615

17-
if (!(propertyKey in object) && typeof object[UNKNOWN_PROPERTY_TAG] === 'function') {
18-
return object[UNKNOWN_PROPERTY_TAG](propertyKey);
16+
if (typeof object[CUSTOM_TAG_FOR] === 'function') {
17+
return object[CUSTOM_TAG_FOR](propertyKey);
1918
}
2019

20+
return createTagForProperty(object, propertyKey);
21+
}
22+
23+
export function createTagForProperty(
24+
object: object,
25+
propertyKey: string | symbol,
26+
_meta?: Meta
27+
): Tag {
28+
let meta = _meta === undefined ? metaFor(object) : _meta;
29+
2130
let tags = meta.writableTags();
2231
let tag = tags[propertyKey];
2332
if (tag) {
@@ -27,7 +36,7 @@ export function tagForProperty(object: any, propertyKey: string | symbol, _meta?
2736
let newTag = createUpdatableTag();
2837

2938
if (DEBUG) {
30-
setupMandatorySetter!(object, propertyKey);
39+
setupMandatorySetter!(newTag, object, propertyKey);
3140

3241
(newTag as any)._propertyKey = propertyKey;
3342
}

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,9 @@ import {
99
defineProperty,
1010
Mixin,
1111
tagFor,
12+
createTagForProperty,
1213
computed,
13-
UNKNOWN_PROPERTY_TAG,
14+
CUSTOM_TAG_FOR,
1415
getChainTagsForKey,
1516
} from '@ember/-internals/metal';
1617
import { setProxy } from '@ember/-internals/utils';
@@ -61,8 +62,14 @@ export default Mixin.create({
6162
return Boolean(get(this, 'content'));
6263
}),
6364

64-
[UNKNOWN_PROPERTY_TAG](key) {
65-
return combine(getChainTagsForKey(this, `content.${key}`));
65+
[CUSTOM_TAG_FOR](key) {
66+
let tag = createTagForProperty(this, key);
67+
68+
if (key in this) {
69+
return tag;
70+
} else {
71+
return combine([tag, ...getChainTagsForKey(this, `content.${key}`)]);
72+
}
6673
},
6774

6875
unknownProperty(key) {

0 commit comments

Comments
 (0)