toHaveStyle not properly matching compound statement

[denchen]

• jest-dom version: 2.1.0
• node version: 8.9.4
• npm version: 6.4.18
• dom-testing-library version: 3.11.1
• react-testing-library version: 5.2.0

Relevant code or config:

Copied & modified from your base CodeSandbox:

import React from "react";

const styles = {
  fontFamily: "monospace",
  textAlign: "center",
  transition: "opacity 0.2s ease-out, top 0.3s cubic-bezier(1.175, 0.885, 0.32, 1.275)"
};

export default ({ name }) => <h1 style={styles}>Hello {name}!</h1>;

Test:

test("react-testing-library works!", () => {
  const { container } = render(<Hello name="Jill" />);
  expect(container.firstChild).toHaveStyle(`font-family: monospace`);
  expect(container.firstChild).toHaveStyle(`text-align: center`);
  expect(container.firstChild).toHaveStyle(
    `transition: all 0.7s ease, width 1.0s cubic-bezier(3, 4, 5, 6);`
  );

What you did:

I was trying to test my transition style that's actually a compound statement.

What happened:

It seems toHaveStyle() will return true even if my assertion differs greatly. It does seem if I remove the compound statement in the test, it will properly fail:

  expect(container.firstChild).toHaveStyle(
    `transition: all 0.7s ease;`
  );

Reproduction:

https://codesandbox.io/s/14mlwzjjvj

Problem description:

I have two code paths (via props), one with transition, one without. I want to ensure the transition CSS is what it should be given the props input.

Suggested solution:

My one workaround for now is to put the transition in a separate class and include it based on my props, thus I can use toHaveClass().

[gnapse]

Have you tried specifying the css properties in the matcher without using a compound form?

toHaveStyles(`
  transition-delay: ...
  transition-duration: ...
  ...
`)

Not saying it's a definite solution, but it could be a workaround. Not that I'm not asking you to define your styles in this way, only the styles to use in the toHaveStyles matcher.

Let me know if that helps. I'm not sure how to support compound css properties. We're merely using the DOM APIs to query the styles, and I'm not sure if you can query a compound css property, which are probably only convenience syntactic ways to specify a bunch of related but independent core css properties.

Even if this workaround works, we can leave this ticket open to continue the discussion. Maybe there is something we can do. Don't know.

[denchen]

In my example CodeSandbox, I changed the assertion to be:

  expect(container.firstChild).toHaveStyle(`transition-duration: 0.4s`);

Except, the test will fail with:

- Expected

- transition-duration: 0.4s;
+ transition-duration: 0s;

Even though my compound transition has a 0.2sec and a 0.3sec duration.

As noted in my original post, my workaround was to just test for a class (eg. .animated) that has the transition. And since I'm using EmotionJS with the jest-emotion library, which will let me take snapshots that include all the CSS.

[gnapse]

Ok, thanks for your input. I'll leave this ticket open and dig more into why this is happening.

[smacpherson64]

This may be related to: https://github.com/jsakas/CSSStyleDeclaration/issues/46. jsdom uses cssstyle for styles I believe.

[0815Strohhut]

Some additional input which I think might be related:

I have a similar issue with "rgba" color values. The problem seems to be in "getStyleDeclaration" of the toHaveStyles matcher. When the styles are assigned to the created div, any styles that are not understood seem to get lost. That seems to be the case for "rgba" values (whereas "rgb" works) and probably also for the transitions mentioned above.

As a result, the toHaveStyles matches has no expected value and the "isSubset" matching will always return true as the empty set is a subset of any set.

Thus, I see two problems:

1. Some valid styles (like rgba) are lost
2. Invalid styles should not result in the matcher to always return true

While it might be possible to improve upon the second problem by at least checking that the expected object is non-empty, I have no idea how to adress the first point. Any input would be appreciated.

[CarlaTeo]

I believe my pull request might solve this: https://github.com/gnapse/jest-dom/pull/81

[ehannes]

Any news on this? This passes in my test suite:

  beforeEach(() => {
    renderResult = render(<p data-testid="paragraph">Test</p>);
    element = renderResult.getByTestId('paragraph');
  });

  it('passes whatever nonsense I test', () => {
    expect(element).toHaveStyle('whatever: nonsense');
  });

[gnapse]

Any news on this? This passes in my test suite:

  beforeEach(() => {
    renderResult = render(<p data-testid="paragraph">Test</p>);
    element = renderResult.getByTestId('paragraph');
  });

  it('passes whatever nonsense I test', () => {
    expect(element).toHaveStyle('whatever: nonsense');
  });

@ehannes this is not related to this ticket. This ticket is about the fact that if you declare a compound css style, and then check for it, you do not get the test to pass, because right now you'd have to test for each of parts of the compound independently.

Your issue however is related to what @CarlaTeo is fixing in #81. That PR is awaiting for some answers to code review. It was also claimed that that PR fixes this issue even though these are separate issues. I'm not sure that it does, as it does not have tests in it actually proving it.

I will be devoting some time to a few pending maintenance tasks on this library next week, and I'll add that PR to the list, to see if it's merged, and also to confirm or deny if it fixes this issue as well.

[ehannes]

Ah, sorry @gnapse. I followed to the pull request you mentioned and confused the issue and the PR :) I'll keep an eye out on that PR instead. Thanks for your time, this package is awesome!

[theseanco]

I am also having this issue attempting to verify a style of translate: 0% on a matched element.

in my case,

expect(getByTestId(container, 'slider-container')).toHaveStyle(`translate: 0%`)

will match, but so will

expect(getByTestId(container, 'slider-container')).toHaveStyle(`translte: 890%`)

[StudioSpindle]

@theseanco Is the typo intentional? (translte: 890%)

[StudioSpindle]

Quite a while since this error was raised.

Updated the CodeSandbox example to the latest version: https://codesandbox.io/s/react-testing-library-demo-gsxjr?fontsize=14&hidenavigation=1&theme=dark

The error still exists.

[StudioSpindle]

Just to be clear, with "compound statement" you mean be chaining multiple shorthand properties in one value?

A single shorthand property (Transition):

/* transition: <property> <duration> <timing-function> <delay>; */
.shorthand {
  font-family: monospace;
  text-align: center;
  transition: margin-right 5s ease-in-out 0.2s;
}

Multiple shorthand properties:

.shorthand-and-compound-example {
  transition: margin-right 5s ease-in-out 0.2s, opacity 5s ease-in-out 0.2s
}

Writing a single transition in longhand will work for toHaveStyle():

.longhand {
  transition-property: margin-right;
  transition-duration: 5s;
  transition-timing-function: ease-in;
  transition-delay: 0.2s;
}

However, chaining them will probably also not work:

.longhand-compound {
  transition-property: margin-right, background-color;
  transition-duration: 3s, 5s;
  transition-timing-function: ease-in, ease-out;
  transition-delay: 0.2s, 0.1s;
}

So it seems two (related) problems in toHaveStyle:

1. Shorthand functions not found 2. Multiple shorthand properties not recognised (a.k.a. the compound statement)

---

note: All shorthand properties: animation, background, border, border-bottom, border-color, border-left, border-radius, border-right, border-style, border-top, border-width, column-rule, columns, flex, flex-flow, font, grid, grid-area, grid-column, grid-row, grid-template, list-style, margin, offset, outline, overflow, padding, place-content, place-items, place-self, text-decoration, transition (Shorthand properties)

[gnapse]

@StudioSpindle thanks a lot for looking into this. This is, in my opinion, the biggest issue we've got in jest-dom right now.

You have correctly described the issues this matcher has right now. As for the cause, I think this is related to the fact that getComputedStyles does not return compound styles, and we use that to check the element styles. Whatever getComputedStyles returns, we use it to match it against the styles the custom matcher receives as argument. I'm not sure of an easy way to fix it.

Here's a note in getComputedStyles documentation that hints at this being a problem for us:

The returned CSSStyleDeclaration object contains active values for CSS property longhand names. For example, border-bottom-width instead of the border-width and border shorthand property names. It is safest to query values with only longhand names like font-size. Shorthand names like font will not work with most browsers.

Maybe we just need to make this clear in toHaveStyle's documentation and get on with it. But I'm open to suggestions to solve this so that it would be transparent to users of this lib.

[StudioSpindle]

If the longhand value returned from the getComputedStyles does include multiple shorthand properties, and the list of shorthand properties is exhaustive.

How about the following: What if we always compare against longhand. If the expected value is shorthand; we write a function (using the exhaustive list) that would convert it to the longhand equivalent?

[StudioSpindle]

By the way: There seems to be a strange result of the getComputedStyle in combination with the CSS cubic-bezier() function. When using shorthand, getComputedStyle does not return the correct value.

So that will stay an issue.

Fiddle: https://jsfiddle.net/Spindle/jyuvzgb5

[gnapse]

Indeed, in addition to what I said, there may be other issues. I can think of browser-specific issues around the implementation of getComputedStyles. In particular I can expect for jsdom to have its share of issues and quirks around it.

For example, take a look at their issues searching for getComputedStyles. In particular, take a look at some issues I found at a glance:

• https://github.com/jsdom/jsdom/issues/2588
• https://github.com/jsdom/jsdom/issues/2327

I wonder if it's worth the effort to try to make toHaveStyle work fully as expected in all situations. It would be nice if we could, but there are so many variables outside our control.

I'm leaving towards making a note around all this in the README, linking to this issue even, and warning about caveats with the use of this matcher. At least in the short term, even if we start an effort towards fixing all this.

[StudioSpindle]

Thanks for the info. Makes sense. Would it be an addition to return an error message in the console with this info if a jest-dom user would try to compare a shorthand css value and not getting a return (/match)? If there is anything I can help with let me know.

[gnapse]

Would it be an addition to return an error message in the console with this info if a jest-dom user would try to compare a shorthand css value and not getting a return

Actually yes! That would be a big plus in addition to the notice in the README. We can definitely strive to have a list of all short-hand and compound css property names in the code, and if the css that's being used in an assertion has some of these, and the assertion failed, we can try to give a better error message mentioning the caveats of this matcher.

[StudioSpindle]

@gnapse I've started working on a proposal of what we've discussed. Feel free to comment/give pointers any time. I will ping you once I have a working version.