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

A way to be able to turn off autoprefixer #2530

Closed
pristas-peter opened this issue Dec 15, 2015 · 9 comments · Fixed by #3009
Closed

A way to be able to turn off autoprefixer #2530

pristas-peter opened this issue Dec 15, 2015 · 9 comments · Fixed by #3009
Labels
discussion external dependency Blocked by external dependency, we can’t do anything about it new feature New feature or request

Comments

@pristas-peter
Copy link

Hi, we are using material-ui components wrapped in Radium. Since Radium does auto-prefixing by decorating the render method, which is also server-side rendering capable, there is no need for material-ui components to do the same thing. Currently there is no way to turn off this feature in material-ui, what we do is that we comment out methods in styles/auto-prefix.js do to nothing, but could there be implemented a better way, through theme manager or something similar?

@alitaheri alitaheri added new feature New feature or request discussion external dependency Blocked by external dependency, we can’t do anything about it labels Dec 15, 2015
@alitaheri
Copy link
Member

There is a discussion going on in #684. However, what you ask is something different. @oliviertassinari Can this be even implemented?

@oliviertassinari
Copy link
Member

what we do is that we comment out methods in styles/auto-prefix.js

That's not ideal, radium is probably not autoprefixing every react element (futher down the tree).

We are using material-ui components wrapped in Radium

What's the use case ?

@pristas-peter
Copy link
Author

That's not ideal, radium is probably not autoprefixing every react element (futher down the tree).

I think it does, but I'm not sure about this... I've patched import function of every material-ui component to be wrapped in Radium...

What's the use case ?

Radium is capable of passing userAgent through context, not global variable like material-ui. Due to racing condition with global variable, we cannot use your prefixing solution...

@pristas-peter
Copy link
Author

Or just implement feature to be able to pass userAgent within context...

@oliviertassinari
Copy link
Member

Or just implement feature to be able to pass userAgent within context

I couldn't agree more. We need this feature 👍

@oliviertassinari
Copy link
Member

we are using material-ui components wrapped in Radium

Have you seens any performances issues when doing so?

Regarding our current style approach, that's far from being ideal. We are going to have to improve it for various reason:

  • the autoprefix is not flexible enought (more or less easy to fix)
  • the style is recomputed at each render while it doesn't change. This has probably very bad effect on performances
  • We need responsive components

radium is probably not autoprefixing every react element (futher down the tree).

I think it does, but I'm not sure about this...

Wow, I had a look at the source code of Radium: https://github.com/FormidableLabs/radium/blob/master/src/resolve-styles.js.
Looks like you are right, radium is travelling the all tree. That seems preaty scarying. I have no idea what are the performances implication about this 😱.

@pristas-peter
Copy link
Author

Well there is boolean variable, which checks if element was already visited, so it happens once per render. There was a lot of talk about performance in Radium's github, the results were not that bad... We are using simple animations, they never drop below 60fps. I think Radium's plugin architecture allows you to write plugin which would use immutable for checking if style prop has changed.
Currently they've also implemented SSR media queries a feature which was requested for a long time and it's working great.

@oliviertassinari
Copy link
Member

@pristas-peter Thanks for your feedback! We are seriously considering the Radium option for this project.

@oliviertassinari oliviertassinari added this to the 0.14.2 Release milestone Jan 6, 2016
@pristas-peter
Copy link
Author

Feel free to ask anymore questions... Personally I would be very satisfied with that solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion external dependency Blocked by external dependency, we can’t do anything about it new feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants