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

Audit: Prefixing Flexbox is currently considered an error #2022

Closed
zouhir opened this issue Apr 17, 2017 · 10 comments
Closed

Audit: Prefixing Flexbox is currently considered an error #2022

zouhir opened this issue Apr 17, 2017 · 10 comments
Assignees

Comments

@zouhir
Copy link

zouhir commented Apr 17, 2017

My CSS currently has Flexbox and I did lose points because of Prefixing.

The error I got was:

Using modern CSS features
Avoids old CSS flexbox
The 2009 spec of Flexbox is deprecated and is 2.3x slower than the latest spec

The feedback details in lighthouse found the error in this selector:

.row {
    display: -ms-flexbox;
}

white in my CSS, it actually has prefixed and non-prefixed version of display property as the following:

.row {
    display: -ms-flexbox;
    display: flex;
    -ms-flex-align: start;
    align-items: flex-start;
    -ms-flex-wrap: wrap;
    flex-wrap: wrap;
}

I think having that prefix will make my website more progressively enhanced for browsers that only support older slow specs and I believe modern ones should be using the non prefixed, modern faster version.

I'll raise it as a bug for now, keen to learn more about that issue and don't mind looking into it once I get the right knowledge on how LH should deal with Flexbox and maybe other modern CSS (I did signup for Google's OSS agreement btw)

screenshot
screen shot 2017-04-17 at 16 36 34

@hexalys
Copy link

hexalys commented Apr 17, 2017

Was just going to file this bug. So I'll comment here. I was made aware prefixed flexbox was discouraged.

This shouldn't be the case for reasons stated at browserslist/browserslist#98 and autoprefixer/autoprefixer.github.io#9

More broadly. I don't think it's wise for tools or search engine audits to suggest that any prefixes for CSS layout should be removed until usage reached 0.0x%. border-radius and box-shadow can be an exception, since those are cosmetic only (and now in the near completely obsolete category). However, adding layout related prefixes can simply insure that sites aren't completely unusable in older browsers still being used at large, even if usage is in low digits.

@brendankenny
Copy link
Member

brendankenny commented Apr 17, 2017

see #1710 for some recent related discussion, though it sidesteps the larger point from this issue.

(in the meantime, FWIW, while flagging this with an x to alert that old flexbox is being used is debatable, you aren't losing points to it. Nothing in the "Best Practices" section affects your score)

@ebidel
Copy link
Contributor

ebidel commented Apr 17, 2017

Just to be clear, Lighthouse is not a tool for "search engine audits".

One thing we could do is make the audit smarter: detect if the user is also using modern css flexbox. Then, warn if they are using the old without the new. In theory, the "Unused CSS rules" audit picks up the rest. However, it's only for rules atm and we're disabling it in the next release.


One thing that we want to achieve with LH and the best practices is to raise developer awareness. The 2009 CSS flex box has negative impact on performance, so it's still worth flagging. Another example is that it's too easy to include much more CSS than one actually needs. Tools like browserlist, while excellent (I use it all the time!), can exacerbate that problem.

Whether or not you can remove a flagged feature remains the discretion of the developer. Most of our audits (mutation events, document.write, appcache, websql, appcache, ...) fall into that category. Every site, market, and user base has different requirements. As @brendankenny mentioned, that's one reason all of those audits warn but don't score.

The preamble does a good job of setting the stage:

screen shot 2017-04-17 at 9 19 52 am

@NekR
Copy link

NekR commented Apr 17, 2017

As @brendankenny mentioned, that's one reason all of those audits warn but don't score.

They are red and look as an error though. Can it be stated more clearly that it's just a warning?

@hexalys
Copy link

hexalys commented Apr 17, 2017

@ebidel
I understand this doesn't affect the score. I merely referred to 'search engine audits' because some of those recommendations come from Google, and many people will perceive it as such. /aside

The old prefix does not have a significant negative impact on performance worth breaking sites for millions. I personally mitigate the small perf impact by only providing a single engine's prefix at a time, server side...

Sites working in 3-5 year old browsers is not luxury. In my blunt opinion, making a site work only in the last 2 browser versions, is the act of a privileged dev who is too lazy to make their site cross browser compatible...

And I am not even concerned with users in that part of the world myself. I have used the old flexbox prefixes in the past 3-4 years simply because 1-5 million users in the US still required it. Including clients of mine in the entertainment industry. It's an act of being responsible. Especially when a visit can cost $7 a click.

@ebidel
Copy link
Contributor

ebidel commented Apr 18, 2017

I don't think anyone is disagreeing that fallback is important.

What did you think of my proposal in #2022 (comment)? We really want to check is if a site is using an old version of flexbox (specifically the slow 2009 version) without trying the new.

The old prefix does not have a significant negative impact on performance worth breaking sites for millions.

Depends on the site. The opposite could be true. Given a large site with a significant amount of css, they could be hurting users by unconditionally shipping too much code to millions of users.

Something that we're still interested in doing is supplementing the report with RUM data and/or real analytics information. IOW, LH makes a suggestion backed by general data, but site owners are given more context to know whether to ignore the recommendation based on their data.

@hexalys
Copy link

hexalys commented Apr 18, 2017

We really want to check is if a site is using an old version of flexbox (specifically the slow 2009 version) without trying the new.

I don't think you would find much of this if at all. Common flexbox issues have more to do with prefix order than not having the new. @miketaylr or @karlcow would know better than me, if that was the case.
i.e. You'll find cases without flex or in the wrong order, but unlikely cases without -webkit-flex.

Given a large site with a significant amount of css, they could be hurting users by unconditionally shipping too much code to millions of users.

I do relate to the size factor as well. But I really feel strongly that there are more potentially penalizing CSS bloat factor to beat up first, before squeezing flexbox into unnecessary incompatibility territory. e.g. The audit says nothing about my prefixed box-shadow(s) or prefixed linear-gradient(s), which are technically less relevant or important than the old flexbox prefix, and more favorable for an earlier deprecation.

Removing those over time makes sense, but adequate prioritization along usage factors is important.
And yes the more context the better. Perhaps, you could factor the overall size of the CSS as a signal. Personally, as long as my CSS is kept under 16k gzipped I am not going to worry too much about removing useful prefixes, and always favor backward compat over saving a couple kbs of css.


Also @zouhir, know your prefixing shown here is inconsistent. It seems to rely on the last 2 browser version rule as well. The problem with that is you are including -ms-flexbox for IE10 released in 2012, while ommiting the -webkit-flex prefix for Safari 7 and 8: both much newer browsers released after IE11 and less than 3 years old. That's a good example showing how the last 2 browser rule can fail at consistent compatibility.

@karlcow
Copy link

karlcow commented Apr 19, 2017

As @hexalys said most of the issues we noticed related to flexbox and prefixes are patterns like:

missing standard property

Mozilla/Microsoft had to implement aliases.
~20% of mobile sites were breaking in Japan/China.

.foo {
   display: -webkit-flex;
   }

mistakenly thinking -moz-box is a thing in the Web context.

.foo {
   -moz-box-flex: 1;
   box-flex: 1;
   display: -moz-box;
   display: box;
   display: flex;
   flex: 1;
}

Order of flex properties

Happens very often. This would be a good test for audits: Warning on the order of properties.

.foo {
   display: flex;
   display: -webkit-flex;
   }

@zouhir
Copy link
Author

zouhir commented Apr 19, 2017

noted @hexalys and @karlcow - my prefixing rule was { browsers: 'last 2 versions' } thanks for the information and I'm going to improve it across multiple projects.


@ebidel I think your idea at #2022 commentd is doable and will give smarter insights about Flexbox audits which is more used everyday.

they way @karlcow broke it down on here sounds really cool to me! but is it an overkill if we want to tackle this issue any further than just checking of the new flex specs property does or does not exists with the old one?

@paulirish
Copy link
Member

This is hard to get right. We've moved this out of the default run of Lighthouse while we reevaluate.

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

8 participants