emotion-js / emotion

👩‍🎤 CSS-in-JS library designed for high performance style composition
https://emotion.sh/
MIT License
17.52k stars 1.11k forks source link

toHaveStyleRule in jest-emotion doesn't normalize whitespace #1988

Open mxmul opened 4 years ago

mxmul commented 4 years ago

Current behavior:

Emotion is stripping some whitespace from style rules, which makes it difficult to assert that a JS variable exists in emitted styles.

For example, if I have defined color token like this:

const RED = 'rgba(255, 0, 0, 1)';

and I use it in a styled component, the Jest matcher considers the color value to be different:

test('renders with correct styles', () => {
  const Div = styled('div')`
    color: ${RED};
  `;
  const tree = renderer.create(<Div />).toJSON();
  expect(tree).toHaveStyleRule('color', RED); // this fails
})

To reproduce:

  1. Clone https://github.com/mxmul/jest-emotion-repo
  2. $ yarn
  3. $ yarn test
$ jest
 FAIL  ./emotion.test.js
  ✕ renders with correct styles (18 ms)

  ● renders with correct styles

    Expected color to match:
      rgba(255, 0, 0, 1)
    Received:
      rgba(255,0,0,1)

      13 |   `;
      14 |   const tree = renderer.create(<Div />).toJSON();
    > 15 |   expect(tree).toHaveStyleRule('color', RED);
         |                ^
      16 | })
      17 |

      at Object.<anonymous> (emotion.test.js:15:16)

Expected behavior:

Whatever transformations Emotion is applying to CSS property values should also be applied to the values passed to toHaveStyleRule. It should treat rgb(255, 0, 0) and rgb(255,0,0) as equivalent.

Environment information:

Andarist commented 4 years ago

It would be great to fix this but I must say that this is not the top priority for us and I might not find the time to implement a fix for this myself.

mxmul commented 4 years ago

@Andarist are you open to a PR then? I could look into this myself

Andarist commented 4 years ago

Sure thing! PRs are always welcome. I would prefer a PR against the next branch though. Please propose a solution before jumping right into coding - I'll be able to tell you if the proposed solution solves potential edge cases etc.

RoystonS commented 4 years ago

Would a pure Stylis solution of serialize(compile(input), [stringify]) produce the desired effect? i.e. get stylis to parse the CSS, then spit it out again using its stringify middleware, which does the whitespace stripping?

Andarist commented 4 years ago

That was literally my first intuition as well but Stylis v4 doesnt normalize whitespace in parenthesis :s

RoystonS commented 4 years ago

Yikes, so it'll need to be that plus a little custom plugin that tidies up values inside decl StyleElements...

Andarist commented 4 years ago

Yeah, probably something like that - we could actually use a minifier plugin that would be used to optimize whitespace in our precomputed styles when using Babel plugin, so this could serve both cases.

Andarist commented 3 years ago

Please always try to share a repro case in a runnable form - either by providing a git repository to clone or a codesandbox. OSS maintainers usually can't afford the time to set up the repro, even if exact steps are given.

KitsonBroadhurst commented 3 years ago

Please always try to share a repro case in a runnable form - either by providing a git repository to clone or a codesandbox. OSS maintainers usually can't afford the time to set up the repro, even if exact steps are given.

I will investigate further and reply with a repo instead!