Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX release] Backport autotracking bugfixes #18721

Merged
merged 4 commits into from
Feb 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 21 additions & 10 deletions packages/@ember/-internals/glimmer/lib/component-managers/custom.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ARGS_PROXY_TAGS, consume } from '@ember/-internals/metal';
import { consume, CUSTOM_TAG_FOR } from '@ember/-internals/metal';
import { Factory } from '@ember/-internals/owner';
import { HAS_NATIVE_PROXY } from '@ember/-internals/utils';
import { OwnedTemplateMeta } from '@ember/-internals/views';
Expand Down Expand Up @@ -192,34 +192,41 @@ export default class CustomComponentManager<ComponentInstance>
): CustomComponentState<ComponentInstance> {
const { delegate } = definition;
const capturedArgs = args.capture();
const namedArgs = capturedArgs.named;

let value;
let namedArgsProxy = {};

if (EMBER_CUSTOM_COMPONENT_ARG_PROXY) {
let getTag = (key: string) => {
return namedArgs.get(key).tag;
};

if (HAS_NATIVE_PROXY) {
let handler: ProxyHandler<{}> = {
get(_target, prop) {
if (capturedArgs.named.has(prop as string)) {
let ref = capturedArgs.named.get(prop as string);
if (namedArgs.has(prop as string)) {
let ref = namedArgs.get(prop as string);
consume(ref.tag);

return ref.value();
} else if (prop === CUSTOM_TAG_FOR) {
return getTag;
}
},

has(_target, prop) {
return capturedArgs.named.has(prop as string);
return namedArgs.has(prop as string);
},

ownKeys(_target) {
return capturedArgs.named.names;
return namedArgs.names;
},

getOwnPropertyDescriptor(_target, prop) {
assert(
'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()',
capturedArgs.named.has(prop as string)
namedArgs.has(prop as string)
);

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

namedArgsProxy = new Proxy(namedArgsProxy, handler);
} else {
capturedArgs.named.names.forEach(name => {
Object.defineProperty(namedArgsProxy, CUSTOM_TAG_FOR, {
configurable: false,
enumerable: false,
value: getTag,
});

namedArgs.names.forEach(name => {
Object.defineProperty(namedArgsProxy, name, {
enumerable: true,
configurable: true,
get() {
let ref = capturedArgs.named.get(name);
let ref = namedArgs.get(name);
consume(ref.tag);

return ref.value();
Expand All @@ -257,8 +270,6 @@ export default class CustomComponentManager<ComponentInstance>
});
}

ARGS_PROXY_TAGS.set(namedArgsProxy, capturedArgs.named);

value = {
named: namedArgsProxy,
positional: capturedArgs.positional.value(),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Object as EmberObject, A, ArrayProxy, PromiseProxyMixin } from '@ember/-internals/runtime';
import { EMBER_CUSTOM_COMPONENT_ARG_PROXY } from '@ember/canary-features';
import { computed, tracked, nativeDescDecorator as descriptor } from '@ember/-internals/metal';
import { computed, get, tracked, nativeDescDecorator as descriptor } from '@ember/-internals/metal';
import { Promise } from 'rsvp';
import { moduleFor, RenderingTestCase, strip, runTask } from 'internal-test-helpers';
import GlimmerishComponent from '../../utils/glimmerish-component';
Expand Down Expand Up @@ -568,6 +568,50 @@ if (EMBER_CUSTOM_COMPONENT_ARG_PROXY) {
this.assertText('hello!');
}

'@test args can be accessed with get()'() {
class TestComponent extends GlimmerishComponent {
get text() {
return get(this, 'args.text');
}
}

this.registerComponent('test', {
ComponentClass: TestComponent,
template: '<p>{{this.text}}</p>',
});

this.render('<Test @text={{this.text}}/>', {
text: 'hello!',
});

this.assertText('hello!');

runTask(() => this.context.set('text', 'hello world!'));
this.assertText('hello world!');

runTask(() => this.context.set('text', 'hello!'));
this.assertText('hello!');
}

'@test args can be accessed with get() if no value is passed'() {
class TestComponent extends GlimmerishComponent {
get text() {
return get(this, 'args.text') || 'hello!';
}
}

this.registerComponent('test', {
ComponentClass: TestComponent,
template: '<p>{{this.text}}</p>',
});

this.render('<Test/>', {
text: 'hello!',
});

this.assertText('hello!');
}

'@test named args are enumerable'() {
class TestComponent extends GlimmerishComponent {
get objectKeys() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { RenderingTestCase, moduleFor, runTask } from 'internal-test-helpers';
import { set, get } from '@ember/-internals/metal';

import { Component } from '../../utils/helpers';
import GlimmerishComponent from '../../utils/glimmerish-component';

moduleFor(
'Helpers test: {{get}}',
Expand Down Expand Up @@ -616,5 +617,31 @@ moduleFor(

assert.strictEqual(this.$('#get-input').val(), 'mcintosh');
}

'@test should be able to get an object value with a path from this.args in a glimmer component'() {
class PersonComponent extends GlimmerishComponent {
options = ['first', 'last', 'age'];
}

this.registerComponent('person-wrapper', {
ComponentClass: PersonComponent,
template: '{{#each this.options as |option|}}{{get this.args option}}{{/each}}',
});

this.render('<PersonWrapper @first={{first}} @last={{last}} @age={{age}}/>', {
first: 'miguel',
last: 'andrade',
});

this.assertText('miguelandrade');

runTask(() => this.rerender());

this.assertText('miguelandrade');

runTask(() => set(this.context, 'age', 30));

this.assertText('miguelandrade30');
}
}
);
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { moduleFor, RenderingTestCase, applyMixins, strip, runTask } from 'internal-test-helpers';

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

Expand Down Expand Up @@ -1089,6 +1089,25 @@ moduleFor(
}
);

moduleFor(
'Syntax test: {{#each}} with array proxies, arrangedContent depends on external content',
class extends EachTest {
createList(items) {
let wrapped = emberA(items);
let proxy = ArrayProxy.extend({
arrangedContent: computed('wrappedItems.[]', function() {
// Slice the items to ensure that updates must be propogated
return this.wrappedItems.slice();
}),
}).create({
wrappedItems: wrapped,
});

return { list: proxy, delegate: wrapped };
}
}
);

moduleFor(
'Syntax test: {{#each as}} undefined path',
class extends RenderingTestCase {
Expand Down
10 changes: 8 additions & 2 deletions packages/@ember/-internals/metal/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export {
isClassicDecorator,
setClassicDecorator,
} from './lib/descriptor_map';
export { getChainTagsForKey, ARGS_PROXY_TAGS } from './lib/chain-tags';
export { getChainTagsForKey } from './lib/chain-tags';
export { default as libraries, Libraries } from './lib/libraries';
export { default as getProperties } from './lib/get_properties';
export { default as setProperties } from './lib/set_properties';
Expand All @@ -53,7 +53,13 @@ export { default as expandProperties } from './lib/expand_properties';
export { addObserver, activateObserver, removeObserver, flushAsyncObservers } from './lib/observer';
export { Mixin, aliasMethod, mixin, observer, applyMixin } from './lib/mixin';
export { default as inject, DEBUG_INJECTION_FUNCTIONS } from './lib/injected_property';
export { tagForProperty, tagFor, markObjectAsDirty, UNKNOWN_PROPERTY_TAG } from './lib/tags';
export {
tagForProperty,
createTagForProperty,
tagFor,
markObjectAsDirty,
CUSTOM_TAG_FOR,
} from './lib/tags';
export {
consume,
Tracker,
Expand Down
13 changes: 8 additions & 5 deletions packages/@ember/-internals/metal/lib/array_events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ export function arrayContentDidChange<T extends { length: number }>(
array: T,
startIdx: number,
removeAmt: number,
addAmt: number
addAmt: number,
notify = true
): T {
// if no args are passed assume everything changes
if (startIdx === undefined) {
Expand All @@ -50,11 +51,13 @@ export function arrayContentDidChange<T extends { length: number }>(

let meta = peekMeta(array);

if (addAmt < 0 || removeAmt < 0 || addAmt - removeAmt !== 0) {
notifyPropertyChange(array, 'length', meta);
}
if (notify) {
if (addAmt < 0 || removeAmt < 0 || addAmt - removeAmt !== 0) {
notifyPropertyChange(array, 'length', meta);
}

notifyPropertyChange(array, '[]', meta);
notifyPropertyChange(array, '[]', meta);
}

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

Expand Down
36 changes: 0 additions & 36 deletions packages/@ember/-internals/metal/lib/chain-tags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import { getLastRevisionFor, peekCacheFor } from './computed_cache';
import { descriptorForProperty } from './descriptor_map';
import { tagForProperty } from './tags';

export const ARGS_PROXY_TAGS = new WeakMap();

export function finishLazyChains(obj: any, key: string, value: any) {
let meta = peekMeta(obj);
let lazyTags = meta !== null ? meta.readableLazyChainsFor(key) : undefined;
Expand Down Expand Up @@ -141,40 +139,6 @@ export function getChainTagsForKey(obj: any, path: string) {
break;
}

// If the segment is linking to an args proxy, we need to manually access
// the tags for the args, since they are direct references and don't have a
// tagForProperty. We then continue chaining like normal after it, since
// you could chain off an arg if it were an object, for instance.
if (segment === 'args' && ARGS_PROXY_TAGS.has(current.args)) {
assert(
`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.`,
segmentEnd !== pathLength
);

lastSegmentEnd = segmentEnd + 1;
segmentEnd = path.indexOf('.', lastSegmentEnd);

if (segmentEnd === -1) {
segmentEnd = pathLength;
}

segment = path.slice(lastSegmentEnd, segmentEnd)!;

let namedArgs = ARGS_PROXY_TAGS.get(current.args);
let ref = namedArgs.get(segment);

chainTags.push(ref.tag);

// We still need to break if we're at the end of the path.
if (segmentEnd === pathLength) {
break;
}

// Otherwise, set the current value and then continue to the next segment
current = ref.value();
continue;
}

// TODO: Assert that current[segment] isn't an undecorated, non-MANDATORY_SETTER/dependentKeyCompat getter

let propertyTag = tagForProperty(current, segment);
Expand Down
19 changes: 14 additions & 5 deletions packages/@ember/-internals/metal/lib/tags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,28 @@ import { DEBUG } from '@glimmer/env';
import { CONSTANT_TAG, createUpdatableTag, dirty, Tag } from '@glimmer/reference';
import { assertTagNotConsumed } from './tracked';

export const UNKNOWN_PROPERTY_TAG = symbol('UNKNOWN_PROPERTY_TAG');
export const CUSTOM_TAG_FOR = symbol('CUSTOM_TAG_FOR');

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

if (!(propertyKey in object) && typeof object[UNKNOWN_PROPERTY_TAG] === 'function') {
return object[UNKNOWN_PROPERTY_TAG](propertyKey);
if (typeof object[CUSTOM_TAG_FOR] === 'function') {
return object[CUSTOM_TAG_FOR](propertyKey);
}

return createTagForProperty(object, propertyKey);
}

export function createTagForProperty(
object: object,
propertyKey: string | symbol,
_meta?: Meta
): Tag {
let meta = _meta === undefined ? metaFor(object) : _meta;

let tags = meta.writableTags();
let tag = tags[propertyKey];
if (tag) {
Expand All @@ -27,7 +36,7 @@ export function tagForProperty(object: any, propertyKey: string | symbol, _meta?
let newTag = createUpdatableTag();

if (DEBUG) {
setupMandatorySetter!(object, propertyKey);
setupMandatorySetter!(newTag, object, propertyKey);

(newTag as any)._propertyKey = propertyKey;
}
Expand Down
13 changes: 10 additions & 3 deletions packages/@ember/-internals/runtime/lib/mixins/-proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ import {
defineProperty,
Mixin,
tagFor,
createTagForProperty,
computed,
UNKNOWN_PROPERTY_TAG,
CUSTOM_TAG_FOR,
getChainTagsForKey,
} from '@ember/-internals/metal';
import { setProxy } from '@ember/-internals/utils';
Expand Down Expand Up @@ -61,8 +62,14 @@ export default Mixin.create({
return Boolean(get(this, 'content'));
}),

[UNKNOWN_PROPERTY_TAG](key) {
return combine(getChainTagsForKey(this, `content.${key}`));
[CUSTOM_TAG_FOR](key) {
let tag = createTagForProperty(this, key);

if (key in this) {
return tag;
} else {
return combine([tag, ...getChainTagsForKey(this, `content.${key}`)]);
}
},

unknownProperty(key) {
Expand Down
Loading