Monkey-patching native JavaScript constructors

Question

I've written a small utility for monkey-patching native JavaScript constructor functions. For example, you can use it to modify input arguments before returning an instance (this can be useful for unit tests).

There is much more detail about the utility on GitHub, so it may be useful to read through the readme on there.

I would be interested to know if I've hugely over-engineered this. There may be simpler techniques that I could take advantage of. Any comments are appreciated.

Here is the code:

var patch = (function () {
 /*jshint evil: true */
 "use strict";
 var global = new Function("return this;")(), // Get a reference to the global object
 fnProps = Object.getOwnPropertyNames(Function); // Get the own ("static") properties of the Function constructor
 return function (original, originalRef, patches) {
 var ref = global[originalRef] = original, // Maintain a reference to the original constructor as a new property on the global object
 args = [],
 newRef, // This will be the new patched constructor
 i;
 patches.called = patches.called || originalRef; // If we are not patching static calls just pass them through to the original function
 for (i = 0; i < original.length; i++) { // Match the arity of the original constructor
 args[i] = "a" + i; // Give the arguments a name (native constructors don't care, but user-defined ones will break otherwise)
 }
 if (patches.constructed) { // This string is evaluated to create the patched constructor body in the case that we are patching newed calls
 args.push("'use strict'; return (!!this ? " + patches.constructed + " : " + patches.called + ").apply(null, arguments);"); 
 } else { // This string is evaluated to create the patched constructor body in the case that we are only patching static calls
 args.push("'use strict'; return (!!this ? new (Function.prototype.bind.apply(" + originalRef + ", [{}].concat([].slice.call(arguments))))() : " + patches.called + ".apply(null, arguments));");
 }
 newRef = new (Function.prototype.bind.apply(Function, [{}].concat(args)))(); // Create a new function to wrap the patched constructor
 newRef.prototype = original.prototype; // Keep a reference to the original prototype to ensure instances of the patch appear as instances of the original
 newRef.prototype.constructor = newRef; // Ensure the constructor of patched instances is the patched constructor
 Object.getOwnPropertyNames(ref).forEach(function (property) { // Add any "static" properties of the original constructor to the patched one
 if (fnProps.indexOf(property) < 0) { // Don't include static properties of Function since the patched constructor will already have them
 newRef[property] = ref[property];
 }
 });
 return newRef; // Return the patched constructor
 };
}());

And here's an example usage (this is a real-world use case for working around a bug in date parsing in PhantomJS):

Date = patch(Date, "DateOriginal", {
 constructed: function () {
 var args = [].slice.call(arguments);
 if (typeof args[0] === "string" && /^\d{4}\/\d{2}$/.test(args[0])) {
 args[0] = args[0] + "/02"; // Make sure the argument has a 'day'
 }
 return new (Function.prototype.bind.apply(DateOriginal, [{}].concat(args)));
 }
});

There are some more examples, and details of the arguments to patch in the GitHub readme.

• Larry Battle: Why not use Object.create()? Also what JS engine or environment will your scripts be running on? If it's web, then it shouldn't work in IE 6-8 because they don't support Function.prototype.bind().
• James Allardice: @LarryBattle - This will only run in environments that do support Function#bind (and any other ES5 methods in there). Polyfills for bind won't work.
• Sukima: Curious what facilitated the need to do the Function / evals?

Answer 1

I don't see any good reason why you're using some of these hacks. It took me a while to understand what you were doing and I was pretty much forced to step through it in debugger. Going to review this line by line and point out simplifications and patterns as I see fit. I'll be batching together all my suggestions into a counter proposal in this jsbin , using the unit tests from your repo.

To start, it seems quite unneccessary and non standard to get the global scope your way (and I'm going to later argue you shouldn't need it)...

var global = new Function("return this;")();

Just do what every other package does and get scope at the start of your wrapper

(function(global) { })(this); //or use self w/e

I'm not a fan of your fnProps idea. I would prefer you store has = Object.prototype.hasOwnProperty; and filter out items on the Function prototype as follows:

var has = Object.prototype.hasOwnProperty();
/* .... */
//To avoid storing generic function properties (like length) on newRef
Object.getOwnPropertyNames(ref).forEach(function(key) {
 if(!has.call(Function, key)) newRef[key] = ref[key];
});

I'm biased here, but as a past Mootools developer I keep comparing your function with Class.refactor. Personally I would prefer you add a reference to the original function on newRef - maybe as some property _original or let the user handle storing the original. I'm not a fan of adding a global for the original function.

A question, why do you not pass this to your constructors? If you're wrapping some classes this seems like it may make usuage less intuitive.

Update (still at work and can't look into removing the new Function() approach yet but I think I found a great change... You can rewrite the extremely confusing bind.apply(...) like this (unit tests passing):

newRef = Function.apply(null, args);

I'm not a minifier but theres no good reason to do !!this ? x : y

Counter proposal:

I believe you should drop the arity requirement and drop the global - instead inform the user they should cache the original locally. I've rewritten the code with these considerations in mind. Here's a jsbin with these changes (note I haven't removed the global to respect your test cases):

var patch = (function (global) {
 /*jshint evil: true */
 "use strict";
 var has = Object.prototype.hasOwnProperty,
 slice = Array.prototype.slice,
 bind = Function.bind;
 return function (original, patches) {
 var args = [],
 newRef, // This will be the new patched constructor
 i;
 patches.called = patches.called || original; // If we are not patching static calls just pass them through to the original function
 if (patches.constructed) { // This string is evaluated to create the patched constructor body in the case that we are patching newed calls
 newRef = function(/*args*/) {
 return (this && this !== global ? patches.constructed : patches.called).apply(this, arguments);//note called with context
 };
 } else { // This string is evaluated to create the patched constructor body in the case that we are only patching static calls
 newRef = function(/*args*/) {
 return this && this !== global ? new (bind.apply(original, [].concat({}, slice.call(arguments))))
 : patches.called.apply(this, arguments);
 };
 }
 // Create a new function to wrap the patched constructor
 newRef.prototype = original.prototype; // Keep a reference to the original prototype to ensure instances of the patch appear as instances of the original
 newRef.prototype.constructor = newRef; // Ensure the constructor of patched instances is the patched constructor
 Object.getOwnPropertyNames(original).forEach(function (property) { // Add any "static" properties of the original constructor to the patched one
 if (!has.call(Function, property)) { // Don't include static properties of Function since the patched constructor will already have them
 newRef[property] = original[property];
 }
 });
 return newRef; // Return the patched constructor
 };
})(this);

Edit - missed an easy code size optimization:

newRef = function(/*args*/) {
 if(this && this !== global) {
 return patches.constructed ? patches.constructed.apply(this, arguments)
 : new (bind.apply(original, [].concat({}, slice.call(arguments))))// create the patched constructor body in the case that we are only patching static calls;
 } else {
 return patches.called.apply(this, arguments);
 }
};

• James Allardice: Thanks for answering. I don't remember the reasons for half of the code in the question (it was over a year ago). When I get a chance I'll look back over it and try to work out what I was doing and why.
• konijn: Good, I will get some action for my bounty ;)
• megawac: This was interesting! Also you can get around that native Function.prototype.bind requirement with eval if desired