Skip to content

Commit

Permalink
Use the shimmed implementation of Object.defineProperty and
Browse files Browse the repository at this point in the history
Object.defineProperties when the native implementation will not asssign a
value to a function's prototype. cujojs#29
  • Loading branch information
jaredcacurak committed Apr 4, 2014
1 parent 7cb930e commit 29593e9
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 12 deletions.
39 changes: 27 additions & 12 deletions object.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,16 +120,22 @@ define(function (require) {
'object-isextensible': 'isExtensible',
'object-preventextensions': 'preventExtensions',
'object-defineproperty-obj': function () {
return hasDefineProperty({});
return hasDefineProperty({}, 'sentinel1', { value: true });
},
'object-defineproperty-dom': function () {
return doc && hasDefineProperty(testEl);
return doc && hasDefineProperty(testEl, 'sentinel1', { value: true });
},
'object-defineproperty-function': function () {
return hasDefineProperty(function () {}, 'prototype', { value: { test: true } });
},
'object-defineproperties-obj': function () {
return hasDefineProperties({});
return hasDefineProperties({}, { 'sentinel2': { value: true } });
},
'object-defineproperties-dom': function () {
return doc && hasDefineProperties(testEl);
return doc && hasDefineProperties(testEl, { 'sentinel2': { value: true } });
},
'object-defineproperties-function': function () {
return hasDefineProperties(function () {}, { 'prototype': { value: { test: true } } });
},
'object-getownpropertydescriptor-obj': function () {
return hasGetOwnPropertyDescriptor({});
Expand Down Expand Up @@ -189,15 +195,15 @@ define(function (require) {
= getOwnPropertyNames;
}

if (!has('object-defineproperties-obj')) {
if (!has('object-defineproperties-obj') || !has('object-defineproperties-function')) {
// check if dom has it (IE8)
Object.defineProperties = shims.defineProperties
= has('object-defineproperties-dom')
? useNativeForDom(Object.defineProperties, defineProperties)
: defineProperties;
}

if (!has('object-defineproperty-obj')) {
if (!has('object-defineproperty-obj') || !has('object-defineproperty-function')) {
// check if dom has it (IE8)
Object.defineProperty = shims.defineProperty
= has('object-defineproperty-dom')
Expand All @@ -221,25 +227,34 @@ define(function (require) {
: getOwnPropertyDescriptor;
}

function hasDefineProperty (object) {
function hasDefineProperty (object, prop, descriptor) {
if (('defineProperty' in Object)) {
try {
// test it
Object.defineProperty(object, 'sentinel1', { value: true })
return object['sentinel1'] === true;
Object.defineProperty(object, prop, descriptor)
return object[prop] === descriptor.value;
}
catch (ex) { /* squelch */ }
}
}

// Note: MSDN docs say that IE8 has this function, but tests show
// that it does not! JMH
function hasDefineProperties (object) {
function hasDefineProperties (object, props) {
var keys, property;

if (('defineProperties' in Object)) {
try {
// test it
Object.defineProperties(object, { 'sentinel2': { value: true } })
return object['sentinel2'] === true;
Object.defineProperties(object, props);
keys = _keys(props);

while (property = keys.pop()) {
if (object[property] !== props[property].value) {
return false;
}
}
return true;
}
catch (ex) { /* squelch */ }
}
Expand Down
7 changes: 7 additions & 0 deletions test/object.html
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,13 @@
Object.defineProperty(test, 'foo', { value: 5 });
return test.foo == 5;
});
tester.assertTrue("Object.defineProperty should define the prototype property correctly", function () {
function fn() {};

Object.defineProperty(fn, 'prototype', { value: { test: true } });

return fn.prototype.test === true && (new fn).test === true;
});
tester.assertTrue('Object.defineProperties should add several properties', function () {
var test = {};
// writable and get don't actually work, just putting them in here
Expand Down

1 comment on commit 29593e9

@unscriptable
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, @jaredcacurak. Ready for a PR? :)

Please sign in to comment.