5

I can not figure out why the following code does not work:

var os = new Proxy(require('os'), {});
console.log( os.cpus() ); // TypeError: Illegal invocation

whereas

var os = require('os');
console.log(Reflect.apply(os.cpus, os, []));

or

var os = new Proxy(require('os'), {});
console.log( os.platform() );

works as expected.

Franck Freiburger
  • 26,310
  • 20
  • 70
  • 95

2 Answers2

6

Having just skim read the source for the os package in the Node repo, it appears that the cpus() is exported from binding.getCPUs which is a C hook in the Node runtime environment.

cpus() therefore has the binding object as a function context, which is then lost through the proxy, giving you the IllegalInvocation error because there is no context to the function when you call it — although I'm hazy on the details.

platform() on the other hand is exported as function () { return process.platform; }, and hence it's just a function that returns an object, and doesn't need to be run under a specific context because Node function contexts will have the process variable specified by default (unless it has been overridden).

The following behaviour shows that applying the os as a context to the cpus function will work — proxies on function objects evidently lose the function context when calling properties.

const os = require('os');
const proxy = new Proxy(os, {});  // proxy of object, functions called get proxy context rather than os context
const cpus = new Proxy(os.cpus, {});  // proxy of function, still has os context

console.log(os.cpus());  // works (duh)
console.log(cpus());     // works
console.log(proxy.cpus.apply(os, []));  // works
console.log(proxy.cpus());  // fails with IllegalInvocation

Note: If someone can clear up the details on the JS function context for an answer I'd love to read it too.

Liam Gray
  • 1,089
  • 9
  • 16
2

How about composition: 

const os = require('os');
const proxy = new Proxy(os, {});
Object.getOwnPropertyNames(os).forEach(k => {
    var v = os[k];
    if(typeof v === "function") proxy[k] = v.bind(os);
});

//the `!!` because I don't want the actual print
//only a `true` or an `Error`
console.log(!!os.cpus());
console.log(!!proxy.cpus());
console.log(!!proxy.cpus.apply(proxy, []));

and all this as a utility function to "replace" new Proxy(), where handler.bindTargetFunctions can be

  • either an array of keyNames to be bound (so you can be specific)
  • or any truthy or falsy value to determine wether all functions on the target should be bound

the code:

function proxy(target, handler){
    const _proxy = new Proxy(target, handler);
    if(handler.bindTargetFunctions){
        let bindTargetFunctions = handler.bindTargetFunctions;
        if(!Array.isArray(bindTargetFunctions)){
            bindTargetFunctions = Object.getOwnPropertyNames(target)
                .filter(key => typeof target[key] === "function");
        }
        bindTargetFunctions.forEach(key => {
            _proxy[key] = target[key].bind(target);
        });
    }
    return _proxy;
}

const os = proxy(require('os'), { bindTargetFunctions: true });
//or
//const os = proxy(require('os'), { bindTargetFunctions: ["cpus"] });

console.log(os.cpus());

Edit:

Currently I try to bind functions directly in my get handler (see github.com/FranckFreiburger/module-invalidate/blob/master/…)‌​, the drawback of my solution is that each access to a function returns a new binding.

I entioned caching in the comments. This is how this cache could look like:

function createProxy(mod){
    var cache = Object.create(null);

    return new Proxy(function(){}, {
        get(target, property, receiver) {
            var val = Reflect.get(mod._exports, property, receiver);
            if(typeof val === "function"){
                if(!(property in cache) || cache[property].original !== val){
                    cache[property] = {
                        original: val,
                        bound: bal.bind(mod._exports)
                    }
                }
                val = cache[property].bound;
            }else if(property in cache){
                delete cache[property];
            }

            return val;
        }
    });
}

And No, I don't consider this cache a regular object. Not because it inherits from null, but because logically, to me this is a dictionary/map. And I don't know any reason why you would ever extend or proxy a particular dictionary.

Thomas
  • 11,958
  • 1
  • 14
  • 23
  • Intresting !, Currently I try to bind functions directly in my `get` handler (see https://github.com/FranckFreiburger/module-invalidate/blob/master/index.js#L106), the drawback of my solution is that each access to a function returns a new binding. – Franck Freiburger Mar 04 '17 at 12:17
  • Note also that with your solution, dynamically-added functions on the target will not be taken in account. – Franck Freiburger Mar 04 '17 at 12:34
  • @FranckFreiburger, you could cache the bound functions, either by name in an object, or by src-function reference in an actual `Map`. – Thomas Mar 04 '17 at 13:52
  • @FranckFreiburger And I rarely have any dynamically-added functions on my objects; I build the object once and leave the interface that way. Everything else invalidates the hidden class of that object and messes with the optimizer in modern JS-engines. – Thomas Mar 04 '17 at 13:58
  • Ok but when you write this: `module.exports.foo = function() {}` `module.exports.bar = function() {}` you are modifying the object and it is legitimate – Franck Freiburger Mar 04 '17 at 14:04
  • @FranckFreiburger, yes it is legitimate *(and the code in my answer will deal with that properly)* but you don't extend an object later on, like `setTimeout(function(){ module.exports.asdf = someNewFunction; }, 10000);` or `var foo = require(whatever); foo.myUtility = anotherFunction;` This is considered a bad practice since PrototypeJS showed how bad you can mess up with extending foreign objects. – Thomas Mar 04 '17 at 14:12
  • I fully understand your point of view but I have to deal js code that is not mine. Have a look at my project: https://github.com/FranckFreiburger/module-invalidate – Franck Freiburger Mar 04 '17 at 14:19
  • 1
    @FranckFreiburger I've updated the answer regarding the mentioned caching. And I'll dive deeper into your repo; but later. – Thomas Mar 04 '17 at 14:26