Skip to content

Commit

Permalink
Merge pull request #19297 from emberjs/bugfix/make-custom-tag-for-non…
Browse files Browse the repository at this point in the history
…-enumerable

[BUGFIX] Updates to fix the CUSTOM_TAG_FOR enumerability bug
  • Loading branch information
Chris Garrett authored Dec 6, 2020
2 parents bcb0a49 + 2c0874f commit 23ee5d1
Show file tree
Hide file tree
Showing 5 changed files with 197 additions and 186 deletions.
24 changes: 12 additions & 12 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,19 +74,19 @@
},
"devDependencies": {
"@babel/preset-env": "^7.9.5",
"@glimmer/compiler": "0.68.0",
"@glimmer/compiler": "0.68.1",
"@glimmer/env": "^0.1.7",
"@glimmer/global-context": "0.68.0",
"@glimmer/interfaces": "0.68.0",
"@glimmer/manager": "0.68.0",
"@glimmer/destroyable": "0.68.0",
"@glimmer/owner": "0.68.0",
"@glimmer/node": "0.68.0",
"@glimmer/opcode-compiler": "0.68.0",
"@glimmer/program": "0.68.0",
"@glimmer/reference": "0.68.0",
"@glimmer/runtime": "0.68.0",
"@glimmer/validator": "0.68.0",
"@glimmer/global-context": "0.68.1",
"@glimmer/interfaces": "0.68.1",
"@glimmer/manager": "0.68.1",
"@glimmer/destroyable": "0.68.1",
"@glimmer/owner": "0.68.1",
"@glimmer/node": "0.68.1",
"@glimmer/opcode-compiler": "0.68.1",
"@glimmer/program": "0.68.1",
"@glimmer/reference": "0.68.1",
"@glimmer/runtime": "0.68.1",
"@glimmer/validator": "0.68.1",
"@simple-dom/document": "^1.4.0",
"@types/qunit": "^2.9.1",
"@types/rsvp": "^4.0.3",
Expand Down
14 changes: 11 additions & 3 deletions packages/@ember/-internals/metal/lib/tags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,17 @@ import { isObject, setupMandatorySetter, symbol, toString } from '@ember/-intern
import { assert } from '@ember/debug';
import { isDestroyed } from '@glimmer/destroyable';
import { DEBUG } from '@glimmer/env';
import { CUSTOM_TAG_FOR } from '@glimmer/manager';
import { getCustomTagFor } from '@glimmer/manager';
import { CONSTANT_TAG, dirtyTagFor, Tag, tagFor, TagMeta } from '@glimmer/validator';

/////////

type CustomTagFnWithMandatorySetter = (
obj: object,
key: string | symbol,
addMandatorySetter: boolean
) => Tag;

// This is exported for `@tracked`, but should otherwise be avoided. Use `tagForObject`.
export const SELF_TAG: string | symbol = symbol('SELF_TAG');

Expand All @@ -16,8 +22,10 @@ export function tagForProperty(
addMandatorySetter = false,
meta?: TagMeta
): Tag {
if (typeof obj[CUSTOM_TAG_FOR] === 'function') {
return obj[CUSTOM_TAG_FOR](propertyKey, addMandatorySetter);
let customTagFor = getCustomTagFor(obj);

if (customTagFor !== undefined) {
return (customTagFor as CustomTagFnWithMandatorySetter)(obj, propertyKey, addMandatorySetter);
}

let tag = tagFor(obj, propertyKey, meta);
Expand Down
59 changes: 30 additions & 29 deletions packages/@ember/-internals/runtime/lib/mixins/-proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
import { setProxy, setupMandatorySetter, isObject } from '@ember/-internals/utils';
import { assert } from '@ember/debug';
import { DEBUG } from '@glimmer/env';
import { CUSTOM_TAG_FOR } from '@glimmer/manager';
import { setCustomTagFor } from '@glimmer/manager';
import { combine, updateTag, tagFor, tagMetaFor } from '@glimmer/validator';

export function contentFor(proxy) {
Expand All @@ -24,6 +24,34 @@ export function contentFor(proxy) {
return content;
}

function customTagForProxy(proxy, key, addMandatorySetter) {
let meta = tagMetaFor(proxy);
let tag = tagFor(proxy, key, meta);

if (DEBUG) {
// TODO: Replace this with something more first class for tracking tags in DEBUG
tag._propertyKey = key;
}

if (key in proxy) {
if (DEBUG && addMandatorySetter) {
setupMandatorySetter(tag, proxy, key);
}

return tag;
} else {
let tags = [tag, tagFor(proxy, 'content', meta)];

let content = contentFor(proxy);

if (isObject(content)) {
tags.push(tagForProperty(content, key, addMandatorySetter));
}

return combine(tags);
}
}

/**
`Ember.ProxyMixin` forwards all properties not defined by the proxy itself
to a proxied `content` object. See ObjectProxy for more details.
Expand All @@ -47,6 +75,7 @@ export default Mixin.create({
this._super(...arguments);
setProxy(this);
tagForObject(this);
setCustomTagFor(this, customTagForProxy);
},

willDestroy() {
Expand All @@ -58,34 +87,6 @@ export default Mixin.create({
return Boolean(get(this, 'content'));
}),

[CUSTOM_TAG_FOR](key, addMandatorySetter) {
let meta = tagMetaFor(this);
let tag = tagFor(this, key, meta);

if (DEBUG) {
// TODO: Replace this with something more first class for tracking tags in DEBUG
tag._propertyKey = key;
}

if (key in this) {
if (DEBUG && addMandatorySetter) {
setupMandatorySetter(tag, this, key);
}

return tag;
} else {
let tags = [tag, tagFor(this, 'content', meta)];

let content = contentFor(this);

if (isObject(content)) {
tags.push(tagForProperty(content, key, addMandatorySetter));
}

return combine(tags);
}
},

unknownProperty(key) {
let content = contentFor(this);
if (content) {
Expand Down
32 changes: 17 additions & 15 deletions packages/@ember/-internals/runtime/lib/system/array_proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,28 @@ import { isObject } from '@ember/-internals/utils';
import EmberObject from './object';
import { isArray, MutableArray } from '../mixins/array';
import { assert } from '@ember/debug';
import { CUSTOM_TAG_FOR } from '@glimmer/manager';
import { setCustomTagFor } from '@glimmer/manager';
import { combine, consumeTag, validateTag, valueForTag, tagFor } from '@glimmer/validator';

const ARRAY_OBSERVER_MAPPING = {
willChange: '_arrangedContentArrayWillChange',
didChange: '_arrangedContentArrayDidChange',
};

function customTagForArrayProxy(proxy, key) {
if (key === '[]') {
proxy._revalidate();

return proxy._arrTag;
} else if (key === 'length') {
proxy._revalidate();

return proxy._lengthTag;
}

return tagFor(proxy, key);
}

/**
An ArrayProxy wraps any other object that implements `Array` and/or
`MutableArray,` forwarding all requests. This makes it very useful for
Expand Down Expand Up @@ -111,26 +125,14 @@ export default class ArrayProxy extends EmberObject {
this._arrangedContentRevision = null;
this._lengthTag = null;
this._arrTag = null;

setCustomTagFor(this, customTagForArrayProxy);
}

[PROPERTY_DID_CHANGE]() {
this._revalidate();
}

[CUSTOM_TAG_FOR](key) {
if (key === '[]') {
this._revalidate();

return this._arrTag;
} else if (key === 'length') {
this._revalidate();

return this._lengthTag;
}

return tagFor(this, key);
}

willDestroy() {
this._removeArrangedContentArrayObserver();
}
Expand Down
Loading

0 comments on commit 23ee5d1

Please sign in to comment.