What the will-change issue might be? #10831

@leedstyh We don't use the willChange property much: https://github.com/mui-org/material-ui/search?utf8=%E2%9C%93&q=willchange&type=. Can you provide a isolated reproduction example? The warning could come from a third party library.

Not appears every time, so I'll close this now. Will ping you back here if I have more details. Thanks @oliviertassinari

Hi @oliviertassinari

I've just encountered this warning.

Thanks for your efforts in keeping MUI performant!

I see that the MDN documentation has this prominent note:

Important: will-change is intended to be used as a last resort, in order to try to deal with existing performance problems. It should not be used to anticipate performance problems.

And further down provides a lengthy list of advice against using this property, including warning against premature optimisation.

Given these warnings, should this property be removed?

@avdd What makes you think it's coming from Material-UI?

This:

https://github.com/mui-org/material-ui/blob/5eee984cbe4f39f793573354ff170757f37d65cc/packages/material-ui/src/Fade/Fade.js#L66

I commented it out and the warning stopped.

Budget limit is the document surface area multiplied by 3

@avdd Why does it cover 3 times the surface area?

@oliviertassinari I think that's just firefox reporting how much memory it's allocating for this optimisation.

My point is, MDN suggests that will-change is an expensive optimisation and should only be used as a "last resort". I was wondering if it's appropriate for MUI to use this expensive optimisation at all. Do you think that it should be the app developer's decision instead?

Again, please check why you are covering 3 times the surface area. A single component shouldn't cover more than 1 time the surface area.

@oliviertassinari that number is a firefox implementation detail and nothing to do with my code:

https://dxr.mozilla.org/mozilla-central/source/layout/painting/nsDisplayList.cpp#2052

@oliviertassinari you weren't even quoting me! 🙂

Fortunately, your good design means I can somewhat easily turn this property off for my application by overriding the style for Backdrop.

But my point is, from my reading of MDN and other sources, that I probably shouldn't have to do that.

Important: will-change is intended to be used as a last resort, in order to try to deal with existing performance problems. It should not be used to anticipate performance problems. ... Don't apply will-change to elements to perform premature optimization.

So, is this a premature optimization? Was there an existing performance problem that this property solves? If not, then it would be fair to interpret that MDN is explicitly warning against precisely this usage, which is why firefox is generating the warning.

And not only MDN.

https://www.w3.org/TR/css-will-change-1/#using

Using will-change directly in a stylesheet implies that the targeted elements are always a few moments away from changing. This is usually not what you actually mean; instead, will-change should usually be flipped on and off via scripting before and after the change occurs

https://dev.opera.com/articles/css-will-change-property/

You see, the browser does already try to optimize for everything as much as it can (remember opacity and 3D transforms?), so explicitly telling it to do that doesn’t really change anything or help in any way. As a matter of fact, doing this has the capacity to do a lot of harm, because some of the stronger optimizations that are likely to be tied to will-change end up using a lot of a machine’s resources, and when overused like this can cause the page to slow down or even crash.

In other words, putting the browser on guard for changes that may or may not occur is a bad idea, and will do more harm that good. Don’t do it.

https://www.sitepoint.com/introduction-css-will-change-property/

Always remember to remove the will-change property when you’re finished using it. As I mentioned above, browser optimizations are a costly process and so when used poorly they can have adverse effects.

My interpretation is that, sans a thorough performance test suite with a specific hardware matrix (as the roadmap says, "we can’t optimize something we can’t measure"), a library has no place using will-change at all.

Again because of your good design, if there is an actual performance problem, an application developer can easily add this property in a targeted optimisation.

Finally, of course, thanks again for your fine work! 👍

@avdd Alright. Do you want to remove the usage of this property? Right now, it only makes sense for the LinearProgress component. The other usage occurrences are opinionated.

@oliviertassinari I also found a problem with will-change. My problem appears on the MacBook (on the PC all is well). I use the Dialog component. Inside it uses the Fade component with will-change: opacity. If the Dialog contains a lot of content the Paper (used by Dialog inside) should to scroll. A scrolling works well in all tested browsers: the Safary, Chrome, Firefox. But only the Safary and Firefox show a scrollbar at moment of scrolling. I think it's a bug of the Chrome of course, but I found a solution by replaced will-change: opacity on will-change: auto. I also know the Chromebook has the same problem in the Chrome.

@MrEfrem If you want to address it, we would more than happy https://github.com/mui-org/material-ui/issues/10831#issuecomment-419715529 :)

@oliviertassinari sorry, I didn't understand what you mean, I just noticed still one problem from using this rule.

Can I work on this?

https://github.com/mui-org/material-ui/blob/118d339113626669817b1ac3fcfc9bd72441124c/packages/material-ui/src/LinearProgress/LinearProgress.js#L81

https://github.com/mui-org/material-ui/blob/118d339113626669817b1ac3fcfc9bd72441124c/packages/material-ui/src/LinearProgress/LinearProgress.js#L86

https://github.com/mui-org/material-ui/blob/118d339113626669817b1ac3fcfc9bd72441124c/packages/material-ui/src/LinearProgress/LinearProgress.js#L97

https://github.com/mui-org/material-ui/blob/118d339113626669817b1ac3fcfc9bd72441124c/packages/material-ui/src/Fade/Fade.js#L66

Do I need to remove this block of code only? Or is it effected else where as well @oliviertassinari ?

@oliviertassinari Is this just removing willChange from the JSS inside LinearProgress?

@joshwooding No, the opposite. The idea is to only use the will change when we 100% know we gonna need it.

@oliviertassinari Ah, okay. I'm not very familiar with willChange. So i'll leave it for someone else

@issuehuntfest has funded $40.00 to this issue. See it on IssueHunt