automatic unwrapping about harmony-reflect HOT 5 CLOSED

The offending call to WeakMap.get is here

function makeUnwrapping0ArgMethod(primitive) {
  return function builtin() {
    var vHandler = directProxies.get(this);
    if (vHandler !== undefined) {
      return builtin.call(vHandler.target);
    } else {
      return primitive.call(this);
    }
  }
};

But there are a number of other identical var vHandler = directProxies.get(this); instances that are also reachable with this not being an object (Array.isArray, and Object.prototype.isPrototypeOf exposes makeUnwrapping1ArgMethod).

So I suggest a patch something like

-    var vHandler = directProxies.get(this);
+    var vHandler;
+    if (typeof this === 'object' && this !== null) {
+      vHandler = directProxies.get(this);
+    }

at each of those call sites, though perhaps you have a better way of specifying that test.

I'll send a PR which you are free to accept or improve upon.

from harmony-reflect.

You'll need to account for functions, since this is also used on Function.prototype.toString. Something like:

function isObject(o){
  var type = typeof o;
  return type === 'object' ? o !== null : type === 'function';
}

from harmony-reflect.

Thank you -- that's right.

I don't have a super deep background in Javascript, so I'll ask: is there a known way of nesting these checks in the right order for best performance, or is it ok (from a performance perspective) to just call WeakMap.get and let it figure out what's an object and throw errors which we catch, like:

try {
   vHandler = directProxies.get(this);
} catch (err) {
   vHandler = undefined;
}

from harmony-reflect.

@metamatt thanks for the PR. Couldn't have done it better myself. I merged it, added unit tests to testRegression.js and updated the NPM package.

from harmony-reflect.

Thanks Tom!

from harmony-reflect.