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

Remove 2009 flex box spec #63

Closed
tkh44 opened this issue Oct 11, 2017 · 15 comments
Closed

Remove 2009 flex box spec #63

tkh44 opened this issue Oct 11, 2017 · 15 comments

Comments

@tkh44
Copy link

tkh44 commented Oct 11, 2017

The spec is ancient and the prefixes are hurting performance for any browser that is ie11 and above.

New syntax support is: Chrome 21+, Safari 6.1+, Firefox 22+, Opera (& Opera Mobile) 12.1+, IE 11+, and Blackberry 10+.

(https://css-tricks.com/old-flexbox-and-new-flexbox/)

Summary: Old is 2.3x slower than new.

https://developers.google.com/web/updates/2013/10/Flexbox-layout-isn-t-slow
https://developers.google.com/web/tools/lighthouse/audits/old-flexbox

Regardless of how modern browsers are using the older properties, we are sending down unnecessary properties when using SSR.

pasted_image_at_2017_10_10_05_57_pm_360

I think we should remove this for the benefit of all end users. I'm willing to help with the removal if you are open to the idea of removing the old spec.

@tkh44
Copy link
Author

tkh44 commented Oct 11, 2017

cc @mxstbr @giuseppeg

@thysultan
Copy link
Owner

I'm open to removing the non IE10+ variants if caniuse stats paint a good picture for < Chrome 21 and < Safari 6.1 which is mostly out of concern for varying android versions that could be using older versions of Chrome as defaults.

The spec is ancient and the prefixes are hurting performance for any browser that is ie11 and above.

Does this still apply here? Since it ships the new syntax last, the browser should be using that instead right?

@mxstbr
Copy link

mxstbr commented Oct 11, 2017

styled-components supports all browsers React supports, which according to the docs means:

React supports all popular browsers, including Internet Explorer 9 and above.

I'd be against removing anything that breaks applications in browsers that React supports. /cc @gaearon

@giuseppeg
Copy link

giuseppeg commented Oct 11, 2017

As much as I would like to see legacy browsers die, I have to second @mxstbr's comment.
Styled-jsx users have the option to disable Stylis' auto prefixing feature and use autoprefixer via plugins.

@thysultan would it be possible to abstract the auto prefix logic into a separate plugin? In that case @tkh44 could write his own version / fork.

@mxstbr
Copy link

mxstbr commented Oct 11, 2017

Maybe it'd be possible to write a plugin that disables that part of the autoprefixing or something?

@thysultan
Copy link
Owner

thysultan commented Oct 11, 2017

@giuseppeg You can already do this now by switching off vendor prefixing, but you would have to implement all vendor prefixing by yourself with a plugin.

That said the inverse is also possible similar to what @mxstbr mentions, essentially a shouldComponentUpdate for vendor prefixes. At the moment it is a 100% in or out config stylis.set({prefix: false}), but nothing stops it from also taking in a function that returns true/false for individual prefixes.

@tkh44
Copy link
Author

tkh44 commented Oct 11, 2017

The argument about supporting whatever React supports seems a bit short-sighted. It seems like we are punishing our users for Facebook's business requirements.

A callback seems like a great compromise for now.

@mxstbr
Copy link

mxstbr commented Oct 11, 2017

It seems like we are punishing our users for Facebook's business requirements.

If Facebook sees significant enough usage of IE9/IE10 to be relevant to their business goals (and I wouldn't doubt their metrics) then it's also significant enough for any other business that builds for a global audience. (so like almost any other internet-based business) Why should a library punish its users to not be able to claim those x% of their business?

I don't see why we should deviate from their browser support. As a React user I want the promised browser support, no matter which libraries I use on top of that.* (even though I personally can't wait to get rid of Internet Explorer)

*Unless they explicitly state that they don't care about old browsers, that's a different story.

@thysultan
Copy link
Owner

thysultan commented Oct 11, 2017

The weird IE10 prefix should not pose any problem in regards to this issue. In relation to the old 2009 spec, the only issue i had was with supporting older android phones(4.2-4.3) that ship with a browser[1] that uses this old spec.

[1] https://caniuse.com/#search=flexbox

@tkh44
Copy link
Author

tkh44 commented Oct 11, 2017

You're right @mxstbr.

I'll concede on that point @thysultan. I wonder if we could design this in a way that end users (devs) can configure this instyled-components, emotion, styled-jsx, etc via some option.

@thysultan
Copy link
Owner

@tkh44 I imagine the shouldPrefix function that you can plug could look like this

var should = []

function shouldPrefix (name) {
	return should.includes(name)
}

stylis.use({prefix: shouldPrefix})

Which should allow you to expose what should be prefixed.

@giuseppeg
Copy link

For whatever reason it seems that UC Browser for Android has 7.6% market share and it supports the old spec yet I still think that many companies care more about IE than this :) You guys decide - I would only ask to release a major version in case we end up removing prefixes for old flexbox.

@tkh44
Copy link
Author

tkh44 commented Oct 11, 2017

@thysultan That would be perfect.

@mxstbr
Copy link

mxstbr commented Oct 12, 2017

I did not realise UC Browser only supports the old spec! That browser is massive in Asia.

I'm happy if it's a configuration thing where each library can decide on their own 😊

@thysultan
Copy link
Owner

released 3.4.0, which includes the option to dynamically control what properties to prefix.

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

4 participants