RFC: Do not allow CSS shorthands in makeStyles() calls

@layershifter

Summary

This RFC proposes to forbid usage of CSS shorthands (background, border, margin, padding, etc.) in makeStyles() calls. Only CSS longhands (backgroundColor, marginLeft, paddingLeft, etc.) will be allowed for usage. This will solve the problem with expanding CSS shorthands, see microsoft/fluentui#19402.

Why expand shorthands?

/* 💡 makeStyles() generates hashed classes, but it's not important in this example */
.a {
  background-color: red;
}
.b {
  background-color: green;
}
.c {
  color: yellow;
}

<!-- Case 1: ❌ Wrong usage -->
<div class="a b c">Hello world!</div>
<!-- Case 2: ✅ Correct usage -->
<div class="a c">Hello world!</div>
<div class="b c">Hello world!</div>

• In "Case 1" both classes are applied to an element: it's wrong as result is not deterministic and depends on classes order in CSS definitions (i.e. insertion order)
• In "Case 2" only single classname per CSS property is applied, result will be deterministic

🗃 CodeSandbox demo

This problem is solved by mergeClasses() function: it de-duplicates classes based on property name.

// ⚠ Simplified example
function App() {
  //                     👇 skips "a", returns only "b c"
  return <div className={mergeClasses('a', 'b', 'c')}>Hello world</div>;
}

To summarize: only non colliding properties should be applied to DOM elements with Atomic CSS. This works well for longhands, but there are cases when longhands and shorthands are combined:

makeStyles({
  root: { backgroundColor: 'red', background: 'green' },
});

👆 In this example the problem is the same: both classes will be applied, result depends on insertion order. In the same time it's annoying for customers use longhands, for example:

makeStyles({
  shorthands: {
    padding: '10px 15px 5px',
    border: '1px solid red',
  },
  longhands: {
    paddingTop: '10px',
    paddingRight: '15px',
    paddingBottom: '5px',
    paddingLeft: '15px',
    borderTopWidth: '1px',
    borderTopStyle: 'solid',
    borderTopColor: 'red',
    // ...
    // 9 more properties for other side i.e. "borderLeft*", "borderBottom*", "borderRight*"
  },
});

To allow use shorthands and longhands simultaneously makeStyles() currently uses inline-style-expand-shorthand (the same library is used in FluentUI Northstar, Fela* & Styletron** plugins).

• * Fela is Atomic CSS-in-JS
• ** Styletron has styletron-engine-atomic to work with Atomic CSS

However there are numerous issues with it (see microsoft/fluentui#19402), but the main problem is it's unclear what to do with CSS variables:

// Input
const input = { padding: 'var(--foo)' };
// Output
const output = {
  /* is it safe to expand it at all? */
  paddingTop: 'var(--foo)',
  paddingRight: 'var(--foo)',
  paddingBottom: 'var(--foo)',
  paddingLeft: 'var(--foo)',
};

• ✅ it works fine when --foo is atomic value, for example 4px
• ❌ it does not work when --foo is compound value, for example 4px 8px

🔗 CodeSandbox demo

Requirements

Stay with deterministic Atomic CSS

It's important for scaling our library to stay with deterministic Atomic classes. For example, we can extract common styles to a stylesheet that will be used across all products.

Unsupported styles should fail at build, not runtime

There were multiple options covered in the original issue (see microsoft/fluentui#19402), but during offline discussion we agreed to go with build time checks instead runtime and rely on TypeScript typings.

The motivation is simple: no one sees console noise, in big applications it usually 100+ development warnings only on initial load.

Keep CSS properties

• Keep interop with any CSS var on any CSS property
• Keep all style properties to spec with CSS properties

This means that custom syntax is a discarded solution, for example:

// ⚠ discarded solution, used only for an example
makeStyles({
  rootA: {
    // 1️⃣ no custom properties
    paddingX: 'var(--foo)',
  },
  rootB: {
    // 2️⃣ no custom values
    padding: { left: 'var(--foo)', right: 'var(--foo)' },
    margin: { all: 'var(--foo)' },
  },
  rootC: {
    // 2️⃣ no custom values
    padding: ['var(--foo)', 'var(--foo)'],
  },
});

Detailed Design or Proposal

Ban CSS shorthands in typings

The first part of proposal is to ban CSS shorthands in typings: no shorthand properties = no need to expand them = no issues ✅

Proposed to forbid usage of all CSS shorthands (list of CSS shorthands exported from MDN) except vendor prefixed (i.e. -ms, -webkit).

makeStyles({
  // 🚨 TypeScript will throw
  rootA: { padding: '4px' },
  // ✅ TypeScript will pass
  rootB: { paddingLeft: '4px', paddingRight: '4px', paddingBottom: '4px', paddingTop: '4px' },
});

Create macro functions for expansion

As it will be annoying and redundant to use CSS longhands in every case we will have special functions for expansion.

import { shorthands } from '@fluentui/react-make-styles';

makeStyles({
  rootA: { paddingLeft: '4px', paddingRight: '4px', paddingBottom: '4px', paddingTop: '4px' },
  rootB: {
    ...shorthands.padding('4px'),
    ...shorthands.padding('4px', '8px'),
    ...shorthands.padding('4px', '8px', '16px', 0),
  },
});

shorthands.padding('4px') from the example above will expand it two four properties:

shallowEqual(
  { paddingLeft: '4px', paddingRight: '4px', paddingBottom: '4px', paddingTop: '4px' },
  shorthands.padding('4px'),
); // returns "true"

Each macro will implement arguments order & expand based on CSS spec.

For initial implementation following shorthands will be implemented:

• border
• borderLeft
• borderBottom
• borderRight
• borderTop
• borderColor
• borderStyle
• borderRadius
• borderWidth
• margin
• padding

Additional functions can be implemented based on customer requests.

• These functions will not have significant impact on runtime version of makeStyles() as they are very lightweight.
• These functions will not have any impact when Babel plugin/Webpack loader are used as they will be evaluated during build.

Pros and Cons

• 👍 No CSS shorthands => no problems
• 👍 No new syntax
• 👎 Non obvious TypeScript errors in some cases
  • Might be solved with ESLint rules that will provide relevant information
• 👎 We have to explain this to users of makeStyles in our documentation

Discarded Solutions

NA

Open Issues

NA