Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrading to React v15.2.0 causes my development build to crash IE9 #7803

Closed
ccoffey opened this issue Sep 24, 2016 · 20 comments
Closed

Upgrading to React v15.2.0 causes my development build to crash IE9 #7803

ccoffey opened this issue Sep 24, 2016 · 20 comments

Comments

@ccoffey
Copy link

ccoffey commented Sep 24, 2016

My application crashes IE9 when I upgrade to React v15.2.0! It works fine with v15.1.0 and all earlier versions.

My diff looks like this

package.json
 -    "react-addons-perf": "15.1.0",
 +    "react-addons-perf": "15.2.0",
 -    "react-addons-shallow-compare": "15.1.0",
 +    "react-addons-shallow-compare": "15.2.0",
 -    "react-addons-test-utils": "15.1.0",
 +    "react-addons-test-utils": "15.2.0",
 -    "react": "15.1.0",
 +    "react": "15.2.0",
 -    "react-addons-css-transition-group": "15.1.0",
 +    "react-addons-css-transition-group": "15.2.0",
 -    "react-addons-transition-group": "15.1.0",
 +    "react-addons-transition-group": "15.2.0",
 -    "react-dom": "15.1.0",
 +    "react-dom": "15.2.0",

Below is a GIF of the crash, I recorded this using BrowserStack (Win7, IE9). I start the recording with Google loaded and dev tools open, I then navigated to my apps URL. As you can see IE9 just crashes hard with nothing logged to console. All other browsers load my app just fine, this only seems to crash IE9!

crash

Did you guys drop support for IE9 in 15.2.0? If not, any ideas what might be causing this?

@aweary
Copy link
Contributor

aweary commented Sep 24, 2016

Support for IE9 was not dropped as far as I know, but unfortunately we can't really help without an idea of what code is causing the issue. Have you tried identifying which part of your application is causing the issue, and if so can you share some of the code that you believe is causing this?

@ccoffey
Copy link
Author

ccoffey commented Sep 24, 2016

I have also tried with the latest version of React (v15.3.2) and it still crashes.

Have you tried identifying which part of your application is causing the issue, and if so can you share some of the code that you believe is causing this?

I currently have no idea what is causing this. It boots in IE9 with React v15.1.0 and all earlier versions of React. It crashes IE9 hard with no error information (console logs, etc) when I use React v15.2.0 or any later version.

I was hoping you guys could point me in the right direction. What did you change in v15.2.0 that might interact badly with IE9?

@ccoffey
Copy link
Author

ccoffey commented Sep 25, 2016

I've managed to narrow this a little further. My application crashes IE9 if I use react & react-dom v15.2.0-rc.1.

My diff now looks like this

package.json
 -    "react": "15.1.0",
 +    "react": "15.2.0-rc.1",
 -    "react-dom": "15.1.0",
 +    "react-dom": "15.2.0-rc.1",

I get some warnings when I run npm install but I guess this is expected!

screen shot 2016-09-25 at 12 13 12 pm

Am I correct in saying that 1 of the 5 commits below must be the culprit? Taken from here

  1. Merge pull request #6553 from zpao/cleanuppkg
  2. Merge pull request #6338 from sebmarkbage/reactnative2
  3. Added docs for environment integration.
  4. Use the same changelog format as 15.0 post
  5. Merge pull request #6443 from Aweary/patch-1

If so then is it possible for me to narrow this further to a specific commit? Any help with this would be greatly appreciated!

@gaearon
Copy link
Collaborator

gaearon commented Sep 25, 2016

Does 15.3.1 have the same problem?

@ccoffey
Copy link
Author

ccoffey commented Sep 25, 2016

I paired my application back to a minimal example which reproduces this issue: https://github.com/ccoffey/react-ie9-crash

@ccoffey
Copy link
Author

ccoffey commented Sep 25, 2016

@gaearon I just tried 15.3.1 with my minimal example and it crashes too.

@gaearon
Copy link
Collaborator

gaearon commented Sep 25, 2016

That’s weird. I can reproduce this with your example (even without the iframe).

I noticed that UMD builds seem to work. If I replace require('react') and require('react-dom') with globals and include their scripts on the page, it doesn’t crash.

@gaearon
Copy link
Collaborator

gaearon commented Sep 25, 2016

For the reference, here is a minimal bundle causing the crash: https://gist.githubusercontent.com/gaearon/e1cae3b92c41bf6a284a09f7e69eb38b/raw/da3a7dbfc93499d7245035bbf0c9f39f32579f73/bundle.js.

I produced it by compiling

var React = require('react');
var ReactDOM = require('react-dom');
var node = document.createElement('div');
document.body.appendChild(node);
ReactDOM.render(React.createElement('div', {}, 'React IE9 Crash'), node);

with Webpack and your configuration.

@ccoffey
Copy link
Author

ccoffey commented Sep 25, 2016

@gaearon thank you for validating and reproducing this issue. What are the next steps? Do you guys prioritise these types of regressions?

@gaearon
Copy link
Collaborator

gaearon commented Sep 25, 2016

I’m not sure it’s a regression yet. As I said, UMD builds seem fine, so this is something I can only reproduce with Webpack build so far. Even more weirdly, replacing this line with var hostInst = inst; fixes it for me. Merely the act of calling that function (even if it is replaced with identity function) crashes IE9. This is so strange that I don’t think we are able to fix it in any way, assuming it’s some sort of script engine bug. There is just not enough information about why it happens to make it actionable. We could restructure that code but since UMD builds work fine, the code itself is not problematic, but some combination of the code and how Webpack bundles it triggers some issue in the script engine. So I’m afraid that, unless you can suggest a solution, I can only recommend using UMD builds (if they don’t have these problems). It is not easy to work around a script engine bug in a library, but if you have suggestions, I’ll be happy to listen.

@ccoffey
Copy link
Author

ccoffey commented Sep 25, 2016

@gaearon any idea why this only became a problem in 15.2.0-rc.1? Also how is this not affecting alot more people? Seems like this should affect everyone using webpack and React > 15.1.0?

@gaearon
Copy link
Collaborator

gaearon commented Sep 25, 2016

If it's a script engine bug it's really random and could occur due to any code change anywhere else in the bundle. We don't know what causes it so even if we know the exact commit, we won't have a way to fix it without a plausible theory about what happened.

I'm not sure why it doesn't affect more people. I haven't checked if there's anything else that could influence it. For example I don't know if WebpackDevServer injects any code into the page. Also I'm not sure if it happens with all Webpack versions.

@ccoffey
Copy link
Author

ccoffey commented Sep 25, 2016

@gaearon in case it helps to know, I just bumped
webpack from 1.12.2 to 1.13.2
webpack-dev-server from 1.12.0 to 1.16.1

and I am still seeing the same crash! I'm not convinced this is a random script engine bug but I would really love to be proven wrong!

@gaearon
Copy link
Collaborator

gaearon commented Sep 25, 2016

Does the workaround of using UMD builds help you in the meantime?

@bretthadley
Copy link

Hi,

I'm getting the same issue and weirdly if I include the webpack.optimize.UglifyJsPlugin() it works fine as well.

@ccoffey
Copy link
Author

ccoffey commented Sep 27, 2016

@gaearon I'm afraid I don't follow you. Can you please explain how I would configure "UMD builds" with the minimum example react-ie9-crash application that I posted?

@gaearon
Copy link
Collaborator

gaearon commented Sep 27, 2016

@ccoffey

You can configure Webpack with resolve.alias option to use UMD bundles (aka precompiled minified files) for React and ReactDOM:

// in your webpack config
resolve: {
  alias: {
    react: 'react/dist/react.min.js',
    'react-dom': 'react-dom/dist/react-dom.min.js',
  }
}

Then your code will use them instead of CommonJS modules.

However, as @bretthadley reported in the comment above, enabling UglifyJsPlugin seems enough to solve the issue, which is something you need to do in production anyway. This might explain why more people aren’t seeing this, as few people develop with IE:

var webpack = require('webpack');

// in your webpack config
plugins: [
  new webpack.optimize.UglifyJsPlugin()
]

@ccoffey
Copy link
Author

ccoffey commented Sep 27, 2016

@gaearon thank you for all of your help! I just verified that enabling the UglifyJsPlugin for our development builds does indeed "fix" this issue! @bretthadley thank you for pointing this out!

Funny enough, we already use the UglifyJsPlugin in our production builds so we would not have been blocked by this issue if we were not so good at testing in development 😄

Fear not tho, this experience will not change any of our good testing procedures!

@eoin
Copy link
Contributor

eoin commented Oct 19, 2016

git bisect indicates that this was introduced in e974383.

The change itself isn't problematic, but it does seem to have triggered an IE9 script engine bug of some sort, resulting in the crash. I've found that a small refactor of precacheChildNodes 'fixes' the issue, but I'm not entirely sure why. 😞

@ccoffey ccoffey changed the title Upgrading to React v15.2.0 causes my app to crash IE9 Upgrading to React v15.2.0 causes my development build to crash IE9 Oct 19, 2016
@gaearon
Copy link
Collaborator

gaearon commented Nov 10, 2016

Fixed in #8018.

@gaearon gaearon closed this as completed Nov 10, 2016
gaearon pushed a commit that referenced this issue Nov 10, 2016
This ‘fixes’ a bizarre IE9 script engine issue. #7803
gaearon pushed a commit that referenced this issue Jan 6, 2017
This ‘fixes’ a bizarre IE9 script engine issue. #7803
(cherry picked from commit 6ce8f1f)
acusti pushed a commit to brandcast/react that referenced this issue Mar 15, 2017
This ‘fixes’ a bizarre IE9 script engine issue. facebook#7803
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants