aurelia / polyfills

The minimal set of polyfills needed to run Aurelia.
MIT License
26 stars 27 forks source link

fix(reflect): fix target-is-object check #25

Closed eriktim closed 8 years ago

eriktim commented 8 years ago

Fix checking for non-null objects as suggested by doktordirk.

EisenbergEffect commented 8 years ago

@jdanyow Where is this at exactly? Can we merge this? Or does it need further work?

jdanyow commented 8 years ago

I think we need to get rid of the check that disallows Reflect.defineProperty on a function.

EisenbergEffect commented 8 years ago

Yes, if we can remove that, it's probably good to go.

doktordirk commented 8 years ago

maybe i missunderstand you, but it's not disallowing, it's allowing

EisenbergEffect commented 8 years ago

Gotcha. I was looking at the snippet and didn't see what was going on. I think this is ok then. @jdanyow Confirm?

eriktim commented 8 years ago

I'd say it should become something like this:

if (typeof Reflect.defineProperty !== 'function') {
  function isObject(obj) {
    return typeof obj === 'object' ? obj !== null : typeof obj === 'function';
  };
  Reflect.defineProperty = function(target, propertyKey, descriptor) {
      if (!isObject(target) || !isObject(descriptor)) {
        throw new TypeError();
      }
      try {
        Object.defineProperty(target, propertyKey, descriptor);
        return true;
      } catch (e) {
        return false;
      }
  };
}

(The error message is browser specific anyway.)

jdanyow commented 8 years ago

looks good