defunctzombie / node-process

process information for node.js and browsers
MIT License
122 stars 62 forks source link

0.11.2 breaks IE11 compat #62

Closed leidegre closed 8 years ago

leidegre commented 8 years ago

I don't have the necessary environment right now to be able to repro this in a way that I can share. In a nutshell this is how I would presume you would repro this issue.

Environment Windows 7/IE 11

Some stuff that goes into the <head/> tag.

<meta http-equiv='X-UA-Compatible' content='IE=edge'>
<script src='https://cdnjs.cloudflare.com/ajax/libs/core-js/2.1.3/shim.min.js' type='text/javascript'></script>

Using webpack with the following config to inject bluebird as the default Promise implementation everywhere. Webpack entry config as follows:

entry: [
  'any-promise/register/bluebird',
  'expose?Promise!any-promise', // make bluebird the de-facto Promise everywhere
  ...
],
...

In this case I was using popsicle to fetch a resource (JSON document) and that's when the issue occurred. There is a relationship between any-promise and popsicle for that reason I think it might be necessary to use popsicle for a successful repro.

See comments on this commit for more information.

calvinmetcalf commented 8 years ago

ok I'm trying to reproduce it but unable to, can you post some compiled code that lets this happen, I'm currious if web pack is doing something unexpected. The only references I'm finding to similar errors involve people doing stuff like ({setTimeout:setTimeout}).setTimeout(fn, 0) or reflux/refluxjs#374 which I'm not 100% sure what they are referring to

leidegre commented 8 years ago

@calvinmetcalf I've extracted the faulting module from webpack bundle. You can find it here.

https://gist.github.com/leidegre/98c40c9718615a41776519f5632d3b41

Though, I don't see anything fishy. Maybe you do?

It's not impossible that this bug has been fixed in later versions of IE11 and that it's only a problem with specific builds. This is of course happening in a corporate environment and I have very limited access to these systems and everything works just fine using the older release so this is not a pressing issue in any way.

image

calvinmetcalf commented 8 years ago

sorry, I meant in context of the bundle so I could try running it to see if I could get the error to happen

leidegre commented 8 years ago

OK, here's an repro for you https://github.com/leidegre/node-process-ie11-bug

I've verified that the bug does occur with version 0.11.2 but not with 0.11.1.

image

image

Happy hunting!

calvinmetcalf commented 8 years ago

ok thanks

calvinmetcalf commented 8 years ago

remove devtool: 'eval', and it's fixed

calvinmetcalf commented 8 years ago

this fyi explains why we didn't see it more frequently that feature looks to be (based on the name) just used for dev tools, I'll take a look at web pack to see what exactly that option does, but it looks to be hacky involving eval, likey for source maps which, from what I remember, is probably not required any more

leidegre commented 8 years ago

OK, cool.

The purpose of the devtool: 'eval' was to be able to browse the source as it was compiled, no source maps back to original source. That was how I understood it but maybe it is more invasive. I thought it was the equivalent of do nothing.

Will take a closer look! Feel free to close the issue if you're done with it.

calvinmetcalf commented 8 years ago

there are actually a few options 'cheap-module-source-map' or 'source-map' are probably what you want to use

leidegre commented 8 years ago

@calvinmetcalf Actually no, I don't want source maps. Builds are quicker without them as well.

However, I don't understand why eval somehow changes the way IE11 treats the code but that remains to be explored.

ThomWright commented 8 years ago

I'm seeing this on Edge as well as IE11.

Running the following in the console reproduces the issue easily enough.

var myTimeout = setTimeout;

myTimeout(function() {}, 0)
calvinmetcalf commented 8 years ago

so the eval is only there in order to create source maps so thats not an issue.

so for eval screwing stuff up, basically there is something wonky and wrong in how eval and setTimeout come together in IE11.

setTimeout issue

IE11 has an extra/unneeded test to check what the 'this' value of setTImeout is and seems to throw if it is defined but not window, in other words ({setTimeout:setTimeout}).setTimeout(function (){console.log('hello')}) throws an error in IE11

eval

this is happening with eval because eval changes the context of where code is run which seems to be causing the context of the setTimeout to change causing it to throw the error

calvinmetcalf commented 8 years ago

@ThomWright just tested this

var myTimeout = setTimeout;

myTimeout(function() {}, 0)

does not cause an error

ThomWright commented 8 years ago

You're right, I just tried it again and it worked. Both times in Edge v20 (pretty ancient but that's what's in my VM). Very confusing! Maybe I was running it in a weird context without realising? :confused:

calvinmetcalf commented 8 years ago

yeah I think we may be able to avoid wierdness by switching over to strict mode

jhiesey commented 8 years ago

I am having the exact same issue, but using the most recent code that already has #64 applied.

I seem to be able to work around it by replacing cachedSetTimeout.call(null, ... with cachedSetTimeout.call(global, ... so that the actually correct object is passed for this.

calvinmetcalf commented 8 years ago

are you running it in web pack with the eval stuff ?

jhiesey commented 8 years ago

I am seeing this in my tests using zuul, which is browserify based.

calvinmetcalf commented 8 years ago

@jhiesey try turning of coverage, #70 should probably help

calvinmetcalf commented 8 years ago

though that being said I've been unable to reproduce this with zuul