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 beta] Fix various issues with descriptor trap. #16169

Merged
merged 4 commits into from
Jan 23, 2018
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
40 changes: 35 additions & 5 deletions packages/ember-metal/lib/properties.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,12 @@ if (EMBER_METAL_ES5_GETTERS) {
}

DESCRIPTOR_GETTER_FUNCTION = function(name, descriptor) {
let trap;
return function CPGETTER_FUNCTION() {
return trapFor(this, name, descriptor);
if (trap) { return trap; }

trap = trapFor(this, name, descriptor);
return trap;
};
};
}
Expand Down Expand Up @@ -231,23 +235,42 @@ export function defineProperty(obj, keyName, desc, data, meta) {
}
}

// used to track if the the property being defined be enumerable
let enumerable = true;

// Ember.NativeArray is a normal Ember.Mixin that we mix into `Array.prototype` when prototype extensions are enabled
// mutating a native object prototype like this should _not_ result in enumerable properties being added (or we have significant
// issues with things like deep equality checks from test frameworks, or things like jQuery.extend(true, [], [])).
//
// this is a hack, and we should stop mutating the array prototype by default 😫
if (obj === Array.prototype) {
enumerable = false;
}

let value;
if (desc instanceof Descriptor) {
value = desc;

if (EMBER_METAL_ES5_GETTERS || DESCRIPTOR_TRAP) {
Object.defineProperty(obj, keyName, {
configurable: true,
enumerable: true,
enumerable,
get: DESCRIPTOR_GETTER_FUNCTION(keyName, value)
});
} else if (MANDATORY_SETTER && watching) {
Object.defineProperty(obj, keyName, {
configurable: true,
enumerable: true,
enumerable,
writable: true,
value
});
} else if (enumerable === false) {
Object.defineProperty(obj, keyName, {
configurable: true,
writable: true,
enumerable,
value
});
} else {
obj[keyName] = value;
}
Expand All @@ -267,7 +290,7 @@ export function defineProperty(obj, keyName, desc, data, meta) {

let defaultDescriptor = {
configurable: true,
enumerable: true,
enumerable,
set: MANDATORY_SETTER_FUNCTION(keyName),
get: DEFAULT_GETTER_FUNCTION(keyName)
};
Expand All @@ -276,7 +299,14 @@ export function defineProperty(obj, keyName, desc, data, meta) {
} else if ((EMBER_METAL_ES5_GETTERS || DESCRIPTOR_TRAP) && wasDescriptor) {
Object.defineProperty(obj, keyName, {
configurable: true,
enumerable: true,
enumerable,
writable: true,
value
});
} else if (enumerable === false) {
Object.defineProperty(obj, keyName, {
configurable: true,
enumerable,
writable: true,
value
});
Expand Down
70 changes: 45 additions & 25 deletions packages/ember-runtime/lib/ext/function.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,14 @@ if (ENV.EXTEND_PROTOTYPES.Function) {
@for Function
@public
*/
FunctionPrototype.property = function () {
return computed(...arguments, this);
};
Object.defineProperty(FunctionPrototype, 'property', {
configurable: true,
enumerable: false,
writable: true,
value: function () {
return computed(...arguments, this);
}
});

/**
The `observes` extension of Javascript's Function prototype is available
Expand Down Expand Up @@ -109,28 +114,37 @@ if (ENV.EXTEND_PROTOTYPES.Function) {
@for Function
@public
*/
FunctionPrototype.observes = function() {
return observer(...arguments, this);
};


FunctionPrototype._observesImmediately = function () {
assert(
'Immediate observers must observe internal properties only, ' +
'not properties on other objects.',
function checkIsInternalProperty() {
for (let i = 0; i < arguments.length; i++) {
if (arguments[i].indexOf('.') !== -1) {
return false;
Object.defineProperty(FunctionPrototype, 'observes', {
configurable: true,
enumerable: false,
writable: true,
value: function () {
return observer(...arguments, this);
}
});

Object.defineProperty(FunctionPrototype, '_observesImmediately', {
configurable: true,
enumerable: false,
writable: true,
value: function () {
assert(
'Immediate observers must observe internal properties only, ' +
'not properties on other objects.',
function checkIsInternalProperty() {
for (let i = 0; i < arguments.length; i++) {
if (arguments[i].indexOf('.') !== -1) {
return false;
}
}
return true;
}
return true;
}
);
);

// observes handles property expansion
return this.observes(...arguments);
};
// observes handles property expansion
return this.observes(...arguments);
}
});

/**
The `on` extension of Javascript's Function prototype is available
Expand All @@ -156,7 +170,13 @@ if (ENV.EXTEND_PROTOTYPES.Function) {
@for Function
@public
*/
FunctionPrototype.on = function () {
return on(...arguments, this);
};

Object.defineProperty(FunctionPrototype, 'on', {
configurable: true,
enumerable: false,
writable: true,
value: function () {
return on(...arguments, this);
}
});
}
88 changes: 64 additions & 24 deletions packages/ember-runtime/lib/ext/string.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,14 @@ if (ENV.EXTEND_PROTOTYPES.String) {
@static
@private
*/
StringPrototype.w = function () {
return w(this);
};
Object.defineProperty(StringPrototype, 'w', {
configurable: true,
enumerable: false,
writeable: true,
value: function() {
return w(this);
}
});

/**
See [String.loc](/api/ember/release/classes/String/methods/loc?anchor=loc).
Expand All @@ -37,9 +42,14 @@ if (ENV.EXTEND_PROTOTYPES.String) {
@static
@private
*/
StringPrototype.loc = function (...args) {
return loc(this, args);
};
Object.defineProperty(StringPrototype, 'loc', {
configurable: true,
enumerable: false,
writeable: true,
value: function(...args) {
return loc(this, args);
}
});

/**
See [String.camelize](/api/ember/release/classes/String/methods/camelize?anchor=camelize).
Expand All @@ -49,9 +59,14 @@ if (ENV.EXTEND_PROTOTYPES.String) {
@static
@private
*/
StringPrototype.camelize = function () {
return camelize(this);
};
Object.defineProperty(StringPrototype, 'camelize', {
configurable: true,
enumerable: false,
writeable: true,
value: function() {
return camelize(this);
}
});

/**
See [String.decamelize](/api/ember/release/classes/String/methods/decamelize?anchor=decamelize).
Expand All @@ -61,9 +76,14 @@ if (ENV.EXTEND_PROTOTYPES.String) {
@static
@private
*/
StringPrototype.decamelize = function () {
return decamelize(this);
};
Object.defineProperty(StringPrototype, 'decamelize', {
configurable: true,
enumerable: false,
writeable: true,
value: function() {
return decamelize(this);
}
});

/**
See [String.dasherize](/api/ember/release/classes/String/methods/dasherize?anchor=dasherize).
Expand All @@ -73,9 +93,14 @@ if (ENV.EXTEND_PROTOTYPES.String) {
@static
@private
*/
StringPrototype.dasherize = function () {
return dasherize(this);
};
Object.defineProperty(StringPrototype, 'dasherize', {
configurable: true,
enumerable: false,
writeable: true,
value: function() {
return dasherize(this);
}
});

/**
See [String.underscore](/api/ember/release/classes/String/methods/underscore?anchor=underscore).
Expand All @@ -85,9 +110,14 @@ if (ENV.EXTEND_PROTOTYPES.String) {
@static
@private
*/
StringPrototype.underscore = function () {
return underscore(this);
};
Object.defineProperty(StringPrototype, 'underscore', {
configurable: true,
enumerable: false,
writeable: true,
value: function() {
return underscore(this);
}
});

/**
See [String.classify](/api/ember/release/classes/String/methods/classify?anchor=classify).
Expand All @@ -97,9 +127,14 @@ if (ENV.EXTEND_PROTOTYPES.String) {
@static
@private
*/
StringPrototype.classify = function () {
return classify(this);
};
Object.defineProperty(StringPrototype, 'classify', {
configurable: true,
enumerable: false,
writeable: true,
value: function() {
return classify(this);
}
});

/**
See [String.capitalize](/api/ember/release/classes/String/methods/capitalize?anchor=capitalize).
Expand All @@ -109,7 +144,12 @@ if (ENV.EXTEND_PROTOTYPES.String) {
@static
@private
*/
StringPrototype.capitalize = function () {
return capitalize(this);
};
Object.defineProperty(StringPrototype, 'capitalize', {
configurable: true,
enumerable: false,
writeable: true,
value: function() {
return capitalize(this);
}
});
}