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

IE8 compatibility issues #1338

Closed
dmatteo opened this issue Sep 20, 2015 · 10 comments
Closed

IE8 compatibility issues #1338

dmatteo opened this issue Sep 20, 2015 · 10 comments

Comments

@dmatteo
Copy link
Contributor

dmatteo commented Sep 20, 2015

I can't make react-bootstrap work in IE8, even while taking all the necessary "precautions" (shims, shams and so forth).
React 0.13 works fine when loaded alone, but when I import anything from react-bootstrap things brake.

An exception is thrown in es5-shams and it seems to me is due to something trying to use Object.defineProperty, but I'm not entirely sure why this is happening.

Is someone else using IE8 without issues?

@AlexKVal
Copy link
Member

I know that this info won't help at all but just for clarity: It was decided to drop IE8 support #692

There are a lot of babel and babel-runtime code aside from just react codebase.
The error could be in any of those layers.

This project uses only one shim import 'es5-shim' here https://github.com/react-bootstrap/react-bootstrap/blob/master/test/index.js#L1 and only for tests to work.

@jquense
Copy link
Member

jquense commented Sep 20, 2015

I don't think that ie8 support was dropped, we just don't test for it.

like react it probably won't work in dev mode but the production mode strips out all the defineProperty calls. if ya can't point to where something is broken or want to PR a fix we'd most probably accept it

@dmatteo
Copy link
Contributor Author

dmatteo commented Sep 20, 2015

That is unfortunate, you probably should have mentioned this in http://react-bootstrap.github.io/getting-started.html#browser-support, where there is a specific mention to IE8.

I'm no big fan of supporting IE8 either, but we specifically chose to go with React + Bootstrap for a major refactor, mostly to have the IE8 compatibility and I was lead to think that react-bootstrap was supporting it too.
We're rewriting Webforms at Podio, which are used not by our customers, but by our customers' customers, and that's the reason why we can't drop IE8 support, while at Podio itself we only support the 2 most recent versions of the major browsers.

I will spend some time trying to pinpoint the issue and see if I can find a solution to PR, but it could be hard if you "stopped caring" about IE8.
I'll keep you posted.

@jquense
Copy link
Member

jquense commented Sep 20, 2015

Like I said, its not that we have stopped caring, its just that there currently isn't enough resources from maintainers to constantly test, and work on IE8. We do what we can to the extent we can to support it. we've choosen DOM libraries that work with IE8, we do our best to write compatible code as well, but since we don't have the resources to test thoroughly in an IE8 environment its hard to get it right 100%.

We still consider ie8 breakages to be bugs and will almost certainly make changes, and accept PR's that fix our oversights, and everyone is working to the extent that is feasibly possible by folks who do this in their spare time to maintain support. It is just an unfortunate (or fortunate depending) situation that maintainers here don't need to support ie8 so we don't get as active feedback on that as we would like.

The one caveat (as I mentioned) is that you must run the NODE_ENV === 'production' for ie8 code to have a chance to work since most of the dev warnings rely on es5 features like defineProperty this is mostly true for react itself as well.

@AlexKVal
Copy link
Member

I must apologize about not conveying that @jquense so clearly expressed at previous post #1338 (comment) about that we just have not enough resources for rigorous testing on IE8 and really would appreciate PRs with fixes like these:
#755
#881
or at least if you can pin-point the place where the problem exists, then we will have starting point we can work with 😉

p.s. I've created the issue about clarifying the real state about IE8 support in the documentation. #1339
Thank you @dmatteo to pointing it out.
🍒

@dmatteo
Copy link
Contributor Author

dmatteo commented Sep 20, 2015

Thx @AlexKVal and @jquense for your replies.
Please understand that I wasn't try to blame you for non completely supporting IE8 as I understand and appreciate the value of OSS and the free work contributors put into it.
For that matter, I wanted to contribute to this project for a while now, and I will hopefully start soon(ish) 😄

That said, I will try to pin point and possibly PR a fix for the problem and also, in the meantime, I will help you update the docs in regard to this.

@Timetheos
Copy link

@dmatteo, did you ever find the solution for your original post? I am having the same issue. I will also look into it more, but I may have to ditch react-bootstrap for now.

@Timetheos
Copy link

@dmatteo FYI: I haven't dived down into how functional it is, but in terms of just getting it to load, I tried various versions of react-bootstrap. The last version I was able to get to work was 0.24.5. Note that when I installed it, npm also loaded lodash@3.10.1 and babel-runtime@5.8.25 (core-js@1.1.4).

@dmatteo
Copy link
Contributor Author

dmatteo commented Sep 26, 2015

Hey @Timetheos, I haven't digged into it yet, except for the lodash-compat PR which was evident.
Unfortunately we had to ditch react-bootstrap in the project I'm working on for time costraints, as debugging RB was too time consuming for the basic Bootstrap functionalities we needed.

I still want to find the culprit so that we can have an IE8 Bootstrap 3 version available for people, but I don't know how soon :)

btw, babel is not the issue if you compile with loose: 'all', as we're using it now

@taion taion added the bug: IE label Oct 3, 2015
@taion
Copy link
Member

taion commented Nov 15, 2015

I'm going to close this for now as too broad of a meta-issue without anything directly actionable. Please open issues for specific actionable problems as they arise.

@taion taion closed this as completed Nov 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants