Workday/canvas-kit

Stencil class name order should not depend on call order

Closed this issue ยท 0 comments

๐Ÿ› Bug Report

Compat mode works by triggering Emotion's style merging where the CSS class name is important.

The createModifiers function takes in modifiers and returns class names. However, it lists those class names in the order of keys that you pass to the function rather than the order you've defined the modifiers. This effects stencils as well since they call createModifiers internally. For example:

const buttonStencil = createStencil({
  base: {},
  modifiers: {
    size: {
      large: {}, // 'cnvs-button--size-large'
    },
    position: {
      start: {} // 'cnvs-button--position-start'
    }
  }
})

buttonStencil({ position: 'start', size: 'large' })

The expected order of class names is:

  • cnvs-button
  • cnvs-button--size-large
  • cnvs-button--position-start

We expect this order, because that's the INSERTION order of the classes into the StyleSheetList and therefore is the order of style merging in static mode.
Instead, the modifier function cared the order you called the modifiers from the stencil, which was position then size. In buttonStencil({ position, size }). So the actual order of class names was:

  • cnvs-button
  • cnvs-button--position-start
  • cnvs-button--size-large

In most cases, this probably won't matter because modifiers tend to define different style properties. It does matter on TertiaryButton because both isThemeable and inverse modifiers define the [systemIconStencil.vars.color] variable. isThemeable was winning over inverse ONLY in compat mode because the class names were listed out of order. Static mode went by insertion order, but compat mode went in a different order. The fix is to make sure modifiers are always applied in insertion order so compat mode merges the exact same way static mode does.

Link to repl or repo (highly encouraged)

https://stackblitz.com/edit/style-merge-order-issue-7mmtqh?file=src%2FApp.tsx

Error Output

image