Skip to content

use of Object.defineProperty breaks ie8 #133

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

Closed
mattydoincode opened this issue Oct 1, 2015 · 11 comments · Fixed by #135
Closed

use of Object.defineProperty breaks ie8 #133

mattydoincode opened this issue Oct 1, 2015 · 11 comments · Fixed by #135

Comments

@mattydoincode
Copy link
Contributor

Hi all,

I understand completely that react-redux may not intend to support ie8 and that's totally fine. However, I'm curious if I could be given a little more insight into how the createProvider method works, especially regarding the call to _createClass so that I might find a workaround. As mentioned here calling Object.defineProperty breaks ie8. Perhaps there is a way to write some of the contents of createProvider in a more es6 heavy way? This might allow me to use my own ie8 compatible babel settings to avoid use of defineProperty. Any and all help is greatly appreciated!

@mattydoincode
Copy link
Contributor Author

And i now see that you guys have written this with es6 and that i was simply reading the deployed compiled files

@gaearon
Copy link
Contributor

gaearon commented Oct 1, 2015

This might allow me to use my own ie8 compatible babel settings to avoid use of defineProperty.

How can we do that?
We don't actually use Object.defineProperty—it's just what Babel uses for class definitions.
Can we avoid it with different Babel settings?

I'm curious if I could be given a little more insight into how the createProvider method works

Just a method that returns a class, see the source.

@mattydoincode
Copy link
Contributor Author

Ya i was reading the source like an idiot of my local copy of the files. It's quite easy to follow thanks.

here is our babel loader: babel-loader?loose=es6.classes,es6.properties.computed&stage=1&optional=spec.protoToAssign&externalHelpers=true

I believe that es6.classes and es6.properties.computed both protect against object.defineProperty.

We're a company that sells software to police forces and hence has to support ie8 and it's the absolute worst :(.

@mattydoincode
Copy link
Contributor Author

Actually i believe the specific babel loose mode for this case is es6.modules as explained here.

@gaearon
Copy link
Contributor

gaearon commented Oct 1, 2015

Can you confirm that compiling React Redux with loose: 'all' fixes the issue?
I don't mind compiling in this mode.

@gaearon gaearon reopened this Oct 1, 2015
@mattydoincode
Copy link
Contributor Author

yup! checkin now.

@mattydoincode
Copy link
Contributor Author

Going to work more on this today. The .babelrc is already using loose mode all, the real kicker is uses of static class properties. Going to make a PR, but finding that a bunch of other libraries in the redux ecosystem also use similar stuff, so having trouble getting a full integration test on ie8. Will update as I find out more!

@mattydoincode
Copy link
Contributor Author

I've forked and cloned the repo locally, have modified the code to protect against Object.defineProperty, but when attempting to test it using npm link am running into this issue. Essentially because react-redux has a (dev?) dependency on react 0.14-rc1, but my application still depends on react 0.13, both are loaded in. So my application is getting With React 0.14 and later versions, you no longer need to wrap <Provider> child into a function. I'm not sure how the build process of react-redux combined with npm link would end up including a second react version and i'm a little out of my element so I hope my test methodology isn't dumb. I might just drop in the shimmed compiled file for now...

@gaearon
Copy link
Contributor

gaearon commented Oct 5, 2015

Out in 3.1.0.

@xcatliu
Copy link

xcatliu commented Dec 23, 2015

use of Object.defineProperty breaks ie8 again.
#227

@gaearon
Copy link
Contributor

gaearon commented Dec 23, 2015

Fixed.

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

Successfully merging a pull request may close this issue.

3 participants