node-process does not function in IE9

[EdJ]

This module seems to have recently become a dependency somewhere in the webpack toolchain (specifically as a sub-dependency of node-libs-browser). My team noticed a couple of days ago that a few of our build agents were generating client-side code that was no longer functional in IE9. (this may also be true of IE8 or lower, but we do not support them so it's hard to tell!)

We have tracked the issue down to the lines I've changed in this PR. Unfortunately in old versions of IE attempting to call setTimeout using .call(null,...) would fail with an error (TypeError: Invalid calling object), and prevent any further JS execution on the page. You can find more information about the error here, although this article unfortunately does not directly cover this particular scenario (setTimeout and a null reference for this).

Hope this PR meets your contribution requirements, and if this solution isn't acceptable or you want to discuss further I'd be happy to see what we can come up with :)

[calvinmetcalf]

hey so quickly, it's more complicated then this because other versions of IE will through that same error if you simply rename setTimeout and try to use it. That being said you shouldn't actually be getting to this code path as a fix I pushed up earlier this week added the if guard to only use the cached timeout if the global one had been replaced. Is there replacement of settimeout going on in your code ?

[EdJ]

Yeah makes sense, I originally tried to do something like:

function overriddenSetTimeout(fn, time) { setTimeout(fn, time); }

and use that as the cached version but I was trying to do this PR very quickly too and obviously it fails the equality checks... Would maybe be worth looking into using Karma/etc. to do some x-browser testing on this? Will look further into this PR on Monday. We don't override setTimeout (as far as I'm aware) but there's a lot of library code in there that we're not using directly so could be interaction between that and your library.

[tsholmes]

Another option here would be to do something like:

return (function() {
  return cachedSetTimeout.call(this, fun, 0);
}());

In IE9 the this will be window (which is necessary), and everywhere else it will be some mix of window and null (but it doesn't matter).

[calvinmetcalf]

@tsholmes not necessarily, certain versions of webpack will eval code in debug mode which can lead to different global objects.

The bug seams to me that we are using the cached version of the timeouts in the first place as the most recent fix was supposed to use the cache version of setTimeout only in the case of somebody screwing around with it, am downloading an ie9 vm to try testing

[calvinmetcalf]

also when this is run in browserify/webpack it can be in strict mode (it's come up) so we can't guarantee that it would be window in IE9

[tsholmes]

luckily ie9 has a "feature" where strict mode doesn't stop this from being bound

(function(){
  'use strict';
  return this;
}())

still returns window in ie9. I'm not familiar with webpack (hitting this issue through browserify), so unsure what a good solution there would be.

[calvinmetcalf]

fun fact: it's only clearTimeout that has this problem in IE9,

setTimeout.call(null, function(){console.log('hello')}, 0) is fine, it's clearTimeout.call(null, token) that throws

[calvinmetcalf]

yeah I don't use webpack much either but we had an earlier issue there, basically webpack uses eval to execute the script when debuging and that does something to the scope which is why were using call in the first place instead of just cachedClearTimeout

[calvinmetcalf]

ok opened up a pull which should fix this, I've tested this manually but please test it yourself and if it works I can publish it

[calvinmetcalf]

ok should be fixed in 0.11.8

[EdJ]

Cool, I'll give this a check tomorrow at some point (sorry, busy weekend!). Good stuff on the quick turnaround!