Consider stopping auto-adding px to number style values (except for a small whitelist)

[mgol]

Do you want to request a feature or report a bug?

A removal of a feature, in a sense.

What is the current behavior?

React automatically adds the px suffix for numerical values passed to the style prop. As some CSS properties accept unitless values, React maintains a blacklist of properties that shouldn't get px auto-appended.

The problem is that this solution doesn't scale. It requires us to add more & more properties to the list as CSS specs expand and recently the list grows faster; Flexbox & Grid added quite a few of them. What's more confusing, some of those props would work both with & without the px suffix and that changes the meaning (lineHeight is suffering from that).

Although I'm a React newbie I'm quite familiar with this issue due to being a member of the jQuery Core team. jQuery has the same logic as React here and we keep having to add to the list. We've actually exposed the list at jQuery.cssNumber so that people don't always have to wait for us to add support for a property and do a release.

That's why we decided that in jQuery 4 we'll drop the auto-prefixing blacklist and turn to a whitelist that lists only a few most common properties to which we want to auto-append px (mostly because they're extremely common and we don't want to break the world too much); we plan to not expand that list unless we missed something really common. You can see the current plan in my PR: jquery/jquery#4055. In particular, see the proposed whitelist in a (visualized) regexp in:
https://github.com/jquery/jquery/blob/03e9dba3882868e1ee79f1fb0504326da925644f/src/css/isAutoPx.js.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than React. Paste the link to your JSFiddle (https://jsfiddle.net/Luktwrdm/) or CodeSandbox (https://codesandbox.io/s/new) example below:

What is the expected behavior?

I propose that React could do the same thing jQuery is planning to and switch the ever-expanding blacklist of CSS props that shouldn't have the px suffix applied to a small whitelist that should have the suffix applied.

This topic has been initially described in #13550.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

All browses & OSs. I don't know how old this logic is in React.

[philipp-spiess]

This is an awesome proposal. Thanks for writing this up!

Looking at the regex you created here I have the feeling that managing the whitelist is not considerable easier. If we would expand the regex into an object (which would be faster to test against in production I assume), we'd not really save a lot - if anything at all.

I think dropping number values for CSS properties would also not be a great solution since it's very handy to use it and it would break probably every React app on the planet 🙂 (well, some people use CSS-in-JS I heard).

Here's an alternative suggestion: What if we always add px to numbers and add dev-only warnings for our unit-less CSS property list that this is probably an issue?

This would be a lot less magic going on under the hood. We could add the warning in 16.x and keep the current behavior for a while. And at some point we can remove the whole blacklist from the production build.

[jquense]

What if we always add px to numbers and add dev-only warnings for our unit-less CSS property list that this is probably an issue?

Isn't this what we do already? The issue here is that will actively break a whole lot of properties right

[jquense]

I do think as an intermediate solution applying px to only a subset of properties is a good option. It means we aren't going to break most apps, and we don't have to worry about the endless arms race of adding properties.

[philipp-spiess]

Isn't this what we do already? The issue here is that will actively break a whole lot of properties right

Currently we do use the whitelist in production. For example here I use animationIterationCount and it will be added as 2 without a px suffix. You can also find the string animationIterationCount in https://unpkg.com/react-dom@16.4.0/cjs/react-dom.production.min.js. There's also no warning from what I can see.

I agree that we should not break React apps because of that but maybe we can add a warning and people slowly migrate away from this and one day (React 20, lol) we might be able to drop that whitelist and save precious bytes.

[jquense]

We whitelist properties without px suffixes, e.g. unitless ones, and otherwise always add the px to numbers. I'm not sure i understand the suggesting for the warning tho, what would we warn about?

[philipp-spiess]

My idea is to warn when using numbers with unitless CSS properties and encourage strings instead. This way we might eventually get rid of them and all remaining numbers can safely have a px suffix.

Not sure if the plan is too ambitious, though. 🙂

[mgol]

@philipp-spiess

Looking at the regex you created here I have the feeling that managing the whitelist is not considerable easier. If we would expand the regex into an object (which would be faster to test against in production I assume), we'd not really save a lot - if anything at all.

The idea is that this whitelist wouldn't really be maintained; it would basically be frozen. I went through all CSS 2.1 props and deduced which ones might be much more popular than the other ones. I only mentioned increasing the whitelist in the case if just after the release people report we missed a very popular CSS prop. I doubt it'll happen, though.

In the ideal world, we wouldn't have this auto-px-appending behavior at all. I'm not sure how feasible would that be to achieve in React but in jQuery we decided it's not due to all the legacy code around. jQuery is used in wildly different ways, often in WordPress projects without any build process at all, sometimes even without version control, code pasted in CMS fields. Many old unmaintained plugins are often included, etc. We don't have a codemod mechanism like React and even if we had many people wouldn't have a way to use it. That's why we want to keep a small whitelist to limit the damage.

React could follow the same strategy or it could aim at a more ambitious goal - to get rid of auto-appending px completely.

Here's an alternative suggestion: What if we always add px to numbers and add dev-only warnings for our unit-less CSS property list that this is probably an issue?

That wouldn't be intuitive IMO. Consider how surprising it'd be for people that the following:

<div style={{lineHeight: 2}} />

actually sets 2px and that they need to quote the number to get it without the suffix.

[jquense]

yeah I think the auto px append is a bit of a relic, it probably was inspired by jQuery (tho we'd have check with the old timers on that one...@zpao). The interesting thing here is that px is special if you want to write something in terms of rem or ems you already need to specify a string, i think moving toward a space where numbers mean unitless is the best option, if we were going to do that.

@mgol if you are interested in making this happen, seeing how much code we could remove by removing the whitelist (in dev) would be a good first step, might give us a better idea of the potential worth. @gaearon could give us a sense if such a change is at all feasible in the FB context as well.

[mgol]

I could take a stab at it if it's not totally out of the question. I'll wait for further feedback for now. :)

[philipp-spiess]

The idea is that this whitelist wouldn't really be maintained; it would basically be frozen.

I'm not sure. I think if we continue to support both auto-px and unit-less numbers, we will run into problems with new CSS properties where we have to make adjustments because users will expect it to work.

My idea with auto-px for everything is probably biased because I use it a lot 🙂. We should definitely look at the broader audience here and keep future units into account as well (👍 for @jquense's comment on rem or ems being different to px and @mgol's lineHeight example). But it's also important that we have a plan on how we could migrate (if we can do that at all).

Either way, getting rid of one of those patterns will make React less magical which I really like about this idea.

seeing how much code we could remove by removing the whitelist (in dev) would be a good first step

More than I thought (~1%:)

[mgol]

I merged my PR to jQuery that mostly removes the px auto-appending logic. We kept a small finite whitelist of CSS props that still get the behavior but no new properties will be caught. See jquery/jquery#4055.

The whitelist is a regex defined [in the isAutoPx.js modulehttps://github.com/jquery/jquery/blob/438b1a3e8a52d3e4efd8aba45498477038849c97/src/css/isAutoPx.js).