Skip to content

Commit

Permalink
Merge pull request #12491 from stefanpenner/fix-es5-watched-getter
Browse files Browse the repository at this point in the history 
[BUGFIX beta] allow watching of ES5+ Getter
  • Loading branch information
@stefanpenner
stefanpenner committed Jan 21, 2016
2 parents 4ea97e0 + 4e052ff commit 2e7991e
Show file tree
Hide file tree
Showing 9 changed files with 545 additions and 69 deletions.
50 changes: 46 additions & 4 deletions packages/ember-metal/lib/properties.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,33 @@ export function Descriptor() {
this.isDescriptor = true;
}

const REDEFINE_SUPPORTED = (function () {
// https://github.com/spalger/kibana/commit/b7e35e6737df585585332857a4c397dc206e7ff9
var a = Object.create(Object.prototype, {
prop: {
configurable: true,
value: 1
}
});

Object.defineProperty(a, 'prop', {
configurable: true,
value: 2
});

return a.prop === 2;
}());
// ..........................................................
// DEFINING PROPERTIES API
//

export function MANDATORY_SETTER_FUNCTION(name) {
return function SETTER_FUNCTION(value) {
function SETTER_FUNCTION(value) {
assert(`You must use Ember.set() to set the \`${name}\` property (of ${this}) to \`${value}\`.`, false);
};
}

SETTER_FUNCTION.isMandatorySetter = true;
return SETTER_FUNCTION;
}

export function DEFAULT_GETTER_FUNCTION(name) {
Expand All @@ -38,6 +57,16 @@ export function DEFAULT_GETTER_FUNCTION(name) {
};
}

export function INHERITING_GETTER_FUNCTION(name) {
function IGETTER_FUNCTION() {
var proto = Object.getPrototypeOf(this);
return proto && proto[name];
}

IGETTER_FUNCTION.isInheritingGetter = true;
return IGETTER_FUNCTION;
}

/**
NOTE: This is a low-level method used by other parts of the API. You almost
never want to call this method directly. Instead you should use
Expand Down Expand Up @@ -123,12 +152,19 @@ export function defineProperty(obj, keyName, desc, data, meta) {
if (isEnabled('mandatory-setter')) {
if (watching) {
meta.writeValues(keyName, data);
Object.defineProperty(obj, keyName, {

let defaultDescriptor = {
configurable: true,
enumerable: true,
set: MANDATORY_SETTER_FUNCTION(keyName),
get: DEFAULT_GETTER_FUNCTION(keyName)
});
};

if (REDEFINE_SUPPORTED) {
Object.defineProperty(obj, keyName, defaultDescriptor);
} else {
handleBrokenPhantomDefineProperty(obj, keyName, defaultDescriptor);
}
} else {
obj[keyName] = data;
}
Expand All @@ -153,3 +189,9 @@ export function defineProperty(obj, keyName, desc, data, meta) {

return this;
}

function handleBrokenPhantomDefineProperty(obj, keyName, desc) {
// https://github.com/ariya/phantomjs/issues/11856
Object.defineProperty(obj, keyName, { configurable: true, writable: true, value: 'iCry' });
Object.defineProperty(obj, keyName, desc);
}
15 changes: 1 addition & 14 deletions packages/ember-metal/lib/property_get.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,12 @@

import Ember from 'ember-metal/core';
import { assert } from 'ember-metal/debug';
import isEnabled from 'ember-metal/features';
import EmberError from 'ember-metal/error';
import {
isGlobal as detectIsGlobal,
isPath,
hasThis as pathHasThis
} from 'ember-metal/path_cache';
import {
peekMeta
} from 'ember-metal/meta';

var FIRST_KEY = /^([^\.]+)/;

Expand Down Expand Up @@ -60,7 +56,6 @@ export function get(obj, keyName) {
return obj;
}

var meta = peekMeta(obj);
var value = obj[keyName];
var desc = (value !== null && typeof value === 'object' && value.isDescriptor) ? value : undefined;
var ret;
Expand All @@ -72,15 +67,7 @@ export function get(obj, keyName) {
if (desc) {
return desc.get(obj, keyName);
} else {
if (isEnabled('mandatory-setter')) {
if (meta && meta.peekWatching(keyName) > 0) {
ret = meta.peekValues(keyName);
} else {
ret = value;
}
} else {
ret = value;
}
ret = value;

if (ret === undefined &&
'object' === typeof obj && !(keyName in obj) && 'function' === typeof obj.unknownProperty) {
Expand Down
22 changes: 14 additions & 8 deletions packages/ember-metal/lib/property_set.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ import {
peekMeta
} from 'ember-metal/meta';

import {
lookupDescriptor
} from 'ember-metal/utils';

/**
Sets the value of a property on an object, respecting computed properties
and notifying observers and other listeners of the change. If the
Expand Down Expand Up @@ -69,23 +73,25 @@ export function set(obj, keyName, value, tolerant) {
obj.setUnknownProperty(keyName, value);
} else if (meta && meta.peekWatching(keyName) > 0) {
if (meta.proto !== obj) {
if (isEnabled('mandatory-setter')) {
currentValue = meta.peekValues(keyName);
} else {
currentValue = obj[keyName];
}
currentValue = obj[keyName];
}
// only trigger a change if the value has changed
if (value !== currentValue) {
propertyWillChange(obj, keyName);

if (isEnabled('mandatory-setter')) {
if (
(currentValue === undefined && !(keyName in obj)) ||
if ((currentValue === undefined && !(keyName in obj)) ||
!Object.prototype.propertyIsEnumerable.call(obj, keyName)
) {
defineProperty(obj, keyName, null, value); // setup mandatory setter
} else {
meta.writeValues(keyName, value);
let descriptor = lookupDescriptor(obj, keyName);
let isMandatorySetter = descriptor && descriptor.set && descriptor.set.isMandatorySetter;
if (isMandatorySetter) {
meta.writeValues(keyName, value);
} else {
obj[keyName] = value;
}
}
} else {
obj[keyName] = value;
Expand Down
17 changes: 17 additions & 0 deletions packages/ember-metal/lib/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -504,8 +504,25 @@ export function applyStr(t, m, a) {
}
}

export function lookupDescriptor(obj, keyName) {
let current = obj;
while (current) {
let descriptor = Object.getOwnPropertyDescriptor(current, keyName);

if (descriptor) {
return descriptor;
}

current = Object.getPrototypeOf(current);
}

return null;
}

export {
GUID_KEY,
makeArray,
canInvoke
};


82 changes: 40 additions & 42 deletions packages/ember-metal/lib/watch_key.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ import {
} from 'ember-metal/meta';
import {
MANDATORY_SETTER_FUNCTION,
DEFAULT_GETTER_FUNCTION
DEFAULT_GETTER_FUNCTION,
INHERITING_GETTER_FUNCTION
} from 'ember-metal/properties';
import { lookupDescriptor } from 'ember-metal/utils';

let handleMandatorySetter, lookupDescriptor;
let handleMandatorySetter;

export function watchKey(obj, keyName, meta) {
// can't watch length on Array - it is special...
Expand All @@ -20,14 +22,17 @@ export function watchKey(obj, keyName, meta) {
m.writeWatching(keyName, 1);

var possibleDesc = obj[keyName];
var desc = (possibleDesc !== null && typeof possibleDesc === 'object' && possibleDesc.isDescriptor) ? possibleDesc : undefined;
var desc = (possibleDesc !== null &&
typeof possibleDesc === 'object' &&
possibleDesc.isDescriptor) ? possibleDesc : undefined;
if (desc && desc.willWatch) { desc.willWatch(obj, keyName); }

if ('function' === typeof obj.willWatchProperty) {
obj.willWatchProperty(keyName);
}

if (isEnabled('mandatory-setter')) {
// NOTE: this is dropped for prod + minified builds
handleMandatorySetter(m, obj, keyName);
}
} else {
Expand All @@ -37,48 +42,38 @@ export function watchKey(obj, keyName, meta) {


if (isEnabled('mandatory-setter')) {
// It is true, the following code looks quite WAT. But have no fear, It
// exists purely to improve development ergonomics and is removed from
// ember.min.js and ember.prod.js builds.
//
// Some further context: Once a property is watched by ember, bypassing `set`
// for mutation, will bypass observation. This code exists to assert when
// that occurs, and attempt to provide more helpful feedback. The alternative
// is tricky to debug partially observable properties.
lookupDescriptor = function lookupDescriptor(obj, keyName) {
let current = obj;
while (current) {
let descriptor = Object.getOwnPropertyDescriptor(current, keyName);

if (descriptor) {
return descriptor;
}

current = Object.getPrototypeOf(current);
}

return null;
};

// Future traveler, although this code looks scary. It merely exists in
// development to aid in development asertions. Production builds of
// ember strip this entire block out
handleMandatorySetter = function handleMandatorySetter(m, obj, keyName) {
let descriptor = lookupDescriptor(obj, keyName);
var configurable = descriptor ? descriptor.configurable : true;
var isWritable = descriptor ? descriptor.writable : true;
var hasValue = descriptor ? 'value' in descriptor : true;
var possibleDesc = descriptor && descriptor.value;
var isDescriptor = possibleDesc !== null && typeof possibleDesc === 'object' && possibleDesc.isDescriptor;
var isDescriptor = possibleDesc !== null &&
typeof possibleDesc === 'object' &&
possibleDesc.isDescriptor;

if (isDescriptor) { return; }

// this x in Y deopts, so keeping it in this function is better;
if (configurable && isWritable && hasValue && keyName in obj) {
m.writeValues(keyName, obj[keyName]);
Object.defineProperty(obj, keyName, {
let desc = {
configurable: true,
enumerable: Object.prototype.propertyIsEnumerable.call(obj, keyName),
set: MANDATORY_SETTER_FUNCTION(keyName),
get: DEFAULT_GETTER_FUNCTION(keyName)
});
get: undefined
};

if (Object.prototype.hasOwnProperty.call(obj, keyName)) {
m.writeValues(keyName, obj[keyName]);
desc.get = DEFAULT_GETTER_FUNCTION(keyName);
} else {
desc.get = INHERITING_GETTER_FUNCTION(keyName);
}

Object.defineProperty(obj, keyName, desc);
}
};
}
Expand All @@ -90,7 +85,10 @@ export function unwatchKey(obj, keyName, meta) {
m.writeWatching(keyName, 0);

var possibleDesc = obj[keyName];
var desc = (possibleDesc !== null && typeof possibleDesc === 'object' && possibleDesc.isDescriptor) ? possibleDesc : undefined;
var desc = (possibleDesc !== null &&
typeof possibleDesc === 'object' &&
possibleDesc.isDescriptor) ? possibleDesc : undefined;

if (desc && desc.didUnwatch) { desc.didUnwatch(obj, keyName); }

if ('function' === typeof obj.didUnwatchProperty) {
Expand All @@ -107,21 +105,21 @@ export function unwatchKey(obj, keyName, meta) {
// that occurs, and attempt to provide more helpful feedback. The alternative
// is tricky to debug partially observable properties.
if (!desc && keyName in obj) {
Object.defineProperty(obj, keyName, {
configurable: true,
enumerable: Object.prototype.propertyIsEnumerable.call(obj, keyName),
set(val) {
// redefine to set as enumerable
let maybeMandatoryDescriptor = lookupDescriptor(obj, keyName);

if (maybeMandatoryDescriptor.set && maybeMandatoryDescriptor.set.isMandatorySetter) {
if (maybeMandatoryDescriptor.get && maybeMandatoryDescriptor.get.isInheritingGetter) {
delete obj[keyName];
} else {
Object.defineProperty(obj, keyName, {
configurable: true,
enumerable: Object.prototype.propertyIsEnumerable.call(obj, keyName),
writable: true,
enumerable: true,
value: val
value: m.peekValues(keyName)
});
m.deleteFromValues(keyName);
},
get: DEFAULT_GETTER_FUNCTION(keyName)
});
}
}
}
}
} else if (count > 1) {
Expand Down
Loading

0 comments on commit 2e7991e

Please sign in to comment.