styled-components / jest-styled-components

🔧 💅 Jest utilities for Styled Components
MIT License
1.59k stars 145 forks source link

Migrate to Styled Components v5 breaks snapshots #276

Open shraddha-2407 opened 4 years ago

shraddha-2407 commented 4 years ago

Hi, thanks for a great library :)

I've updated to Styled Components v5, and while everything seems to be fine, the snapshots have broken. I have also upgraded to beta version of jest-styled-components -> v7.

I don't want to update any snapshots until I really understand what the changes are.

I am including an image below — a lot of the changes seem to be to class names, and also random insertion of empty media queries but there is also removed styles which appear further down, see attached. Anyone know why this is happening? Any advice would be much appreciated!

Screen Shot 2019-11-11 at 13 26 04 Screen Shot 2019-11-11 at 13 30 58
rafalmaciejewski commented 4 years ago

which versions specifically are you using?

shraddha-2407 commented 4 years ago

System: OS: macOS Mojave 10.14.6 CPU: (4) x64 Intel(R) Core(TM) i5-6360U CPU @ 2.00GHz Memory: 82.24 MB / 16.00 GB Shell: 3.2.57 - /bin/bash Binaries: Node: 10.3.0 - ~/.nvm/versions/node/v10.3.0/bin/node Yarn: 1.9.4 - ~/.nvm/versions/node/v10.3.0/bin/yarn npm: 6.1.0 - ~/.nvm/versions/node/v10.3.0/bin/npm Watchman: 4.9.0 - /usr/local/bin/watchman npmPackages: babel-plugin-styled-components: ^1.8.0 => 1.10.0 styled-components: ^5.0.0-rc.1 => 5.0.0-rc.1

tinynumbers commented 4 years ago

@shraddha-2407 what test approach are you using? Enzyme shallow rendering? React-testing-library? Something else?

I'm having quite different problems (see https://github.com/styled-components/jest-styled-components/issues/285) with my setup, also on SC v5 and jest-styled-components v7 beta.

gilbarbara commented 4 years ago

I'm having a similar problem but only for some of the snapshots. About a third of the tests kept working but the rest are falling as it renders sc-*** classes instead.

styled-components@5.0.0 jest-styled-components@7.0.0

I'm not using the babel plugin...

System: OS: macOS 10.15.2 CPU: (12) x64 Intel(R) Core(TM) i9-8950HK CPU @ 2.90GHz Memory: 10.52 GB / 32.00 GB Shell: 5.0.11 - /usr/local/bin/bash Binaries: Node: 13.6.0 - /usr/local/bin/node Yarn: 1.21.1 - /usr/local/bin/yarn npm: 6.13.4 - /usr/local/lib/node_modules/.bin/npm Watchman: 4.9.0 - /usr/local/bin/watchman

jure commented 4 years ago

I am also seeing lots of diffs in the snapshots, indicating that jest-styled-components doesn't do its class name magic in some cases, e.g.:

       <a
-        className="c3 c4"
+        className="sc-fzXfNK c3 c4"
         href="/"
         onClick={[Function]}
       >

Happy to help out if someone gives me some pointers!

iippis commented 4 years ago

On top of the classname change (which to me isn't that big of a problem) we also got some extra things popping up in the snapshots. Like with react-helmet testing suddenly these @media queries appeared in the snapshots before the actual element we are comparing to:

      @media (max-width:849px) {

      }

      @media (max-width:849px) {

      }

      @media (max-width:849px) {

      }

      @media (max-width:849px) {

      }

      @media (max-width:491px) {

      }

      @media (max-width:849px) {

      }

      @media (max-width:491px) {

      }

      @media (max-width:491px) {

      }

      @media (max-width:849px) {

      }

      @media (max-width:491px) {

      }

      @media (max-width:491px) {

      }

      @media (max-width:491px) {

      }
byronmejia commented 4 years ago

@jure - can you confirm if that specific scenario you've screen grabbed uses Referring To Other Components?

My suspicion is the current serialiser does not look at components that have not been rendered yet:

https://github.com/styled-components/jest-styled-components/blob/e9ef3e0363a12bd5dfcbaef80c72689d760838a2/test/styleSheetSerializer.spec.js#L200-L252

As you can see in the matching snapshot, it will use the hash over the reduced classNames:

https://github.com/styled-components/jest-styled-components/blob/e9ef3e0363a12bd5dfcbaef80c72689d760838a2/test/__snapshots__/styleSheetSerializer.spec.js.snap#L537-L545

This has made it difficult to migrate our source code from 4.2.x to 5.x due to these snapshots now taking the hash over the generated classNames which we have found to be non-deterministic in the past.

jure commented 4 years ago

@byronmejia no, unfortunately we don't use this way of referring to components.

For example, the component that now shows the hashes instead of the c0, c1 stuff is:

const common = css`
  color: ${th('colorPrimary')};
  font: ${th('fontInterface')};
  font-size: ${th('fontSizeBase')};
  font-weight: ${props => (props.active ? 'bold' : 'normal')};
  text-decoration: none;
  text-transform: none;
  transition: ${th('transitionDuration')} ${th('transitionTimingFunction')};

  &:hover,
  &:focus,
  &:active {
    background: none;
    color: ${th('colorPrimary')};
    text-decoration: underline;
  }

  ${override('ui.Action')};
`

const Button = styled(OriginalButton)`
  background: none;
  border: none;
  min-width: 0;
  padding: 0;

  ${common};
`

const Link = styled(OriginalLink)`
  ${common};
`

th and override are props.theme helpers. (OriginalLink and OriginalButton are simple styled.a and styled.button respectively)

I'll try to dig a bit deeper.

stevenocchipinti commented 4 years ago

I'm also experiencing this problem.

This is the simplest example I could come up with to reproduce the problem, seems to happen whenever the styled() syntax is used to override an existing styled component.

Here is an implementation to demonstrate:

import React from "react";
import styled from "styled-components";
import "jest-styled-components";

const Foo = styled.p`
  color: blue;
`;

const Bar = styled(Foo)`
  color: red;
`;

export default () => <Bar>test</Bar>;

A test with react-testing-library like this:

import React from "react";
import { render } from "@testing-library/react";

import Foo from "./Foo";

test("it works", () => {
  const { container } = render(<Foo />);
  expect(container.firstChild).toMatchSnapshot();
});

Generates a snapshot like this:

// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`it works 1`] = `
.c0 {
  color: blue;
  color: red;
}

<p
  class="sc-AxjAm c0"
>
  test
</p>
`;

I also tried with react-test-renderer and while the snapshot contains className instead of class, the auto-generated hash is still present so I don't think that makes any difference.

stevenocchipinti commented 4 years ago

Also, it looks like this scenario isn't covered by tests, this is the only test I saw that uses styled(): https://github.com/styled-components/jest-styled-components/blob/master/test/styleSheetSerializer.spec.js#L69

Although this one is fine because the the component being extended is not a styled-component.

I just realised the snapshots for "referring to other components" actually includes auto-generated hashes (like .sc-fzXfLU) as mentioned above by @byronmejia! (Sorry for doubling up 😅)

Are those hashed that supposed to be the snaphots? 🤔

zorfling commented 4 years ago

Looks like this broke in #249.

162 looks to be a fix to the same issue previously.

zorfling commented 4 years ago

@probablyup @MicheleBertoli @mxstbr - is the fix here to just clear out any unreferenced sc-#####?

As @probablyup mentioned on https://github.com/styled-components/styled-components/issues/2980#issuecomment-576991934 these are static classes for component selector targeting, so can we just strip them?

We're getting updated classes in every snapshot (I think, with a new component / order change in a top level barrel file) which is very noisy. eg

-                   className="c28 sc-LzLOJ c37"
+                   className="c28 sc-LzLOQ c37"

The problem seems to be that in the case that a component extends from a component that isn't itself used, masterSheet.names from the styled-components internals doesn't include it so therefore it doesn't get replaced.

Simplest repro:

it('breaks', () => {
  const Container = styled.div`
    background: red;
  `;

  const StyledContainer = styled(Container)`
    background: blue;
  `;

  const component = (
    <StyledContainer>Hi</StyledContainer>
  );

  expect(render(component).container.firstChild).toMatchSnapshot('react-testing-library');
});

returns:

exports[`breaks: react-testing-library 1`] = `
.c0 {
  background: red;
  background: blue;
}

<div
  class="sc-AxjAm c0"
>
  Hi
</div>
`

and with replaceHashes and replaceClasses commented out:

exports[`breaks: react-testing-library 1`] = `
.dZTZYu {
  background: red;
  background: blue;
}

<div
  class="sc-AxjAm sc-AxirZ dZTZYu"
>
  Hi
</div>
`

Thanks folks! Would really love to get this one fixed.

imdadul commented 4 years ago

In my case, after upgrading it's like this image

Any idea?

ceari commented 4 years ago

Same issues here with the empty media queries (https://github.com/styled-components/jest-styled-components/issues/276#issuecomment-574518539). What's worse is that the storyshots created under Windows differ from the storyshots created under Linux because of a different number of empty media queries written.

Update: We were able to resolve this issue by adding an explicit import to jest-styled-components as documented here: https://github.com/styled-components/jest-styled-components#global-installation

The empty media queries are no longer generated.

hannadrehman commented 4 years ago

is this fixed?. i am still facing this issue

jamime commented 4 years ago

I'm also seeing this with code like this

const StepSequenceItemStyle = styled.li`
  ${StyledIcon} {
    position: relative;
  }
- .c1 .c4 {
+ .c1 .sc-AxiKw {
I'm trying to update to the latest versions: Name Version
styled-components 5.1.0
jest-styled-components 7.0.2
The original snapshot (.c1 .c4) was generated with the following versions: Name Version
styled-components 4.4.1
jest-styled-components 6.3.3
hamatoyogi commented 4 years ago

Same issues here with the empty media queries (#276 (comment)). What's worse is that the storyshots created under Windows differ from the storyshots created under Linux because of a different number of empty media queries written.

Update: We were able to resolve this issue by adding an explicit import to jest-styled-components as documented here: https://github.com/styled-components/jest-styled-components#global-installation

The empty media queries are no longer generated.

This did not work for me. I still have empty media queries generated. I'm still having issues with weird snapshots as well. Is there an update on this issue?

Name Version
styled-components 5.1.0
jest-styled-components 7.0.2
mksztv commented 4 years ago

Faced with same issue when extending component. For code: export const Button = styled(BaseButton)<ButtonProps>'border: 1px solid blue;'; will be created snapshot with following classnames className="sc-fzqAbL c2". But if update to export const Button = styled.button<ButtonProps>'border: 1px solid blue;'; snapshot will be generated with following classname: className="c2"

kerosan commented 4 years ago

any updates?🤔

castamir commented 4 years ago

@kerosan a workaround was merged recently, no new release was created tho, see https://github.com/styled-components/jest-styled-components/pull/320

zorfling commented 3 years ago

This looks fixed for me in v7.0.3 - Thanks! 🎉

tomdev10 commented 3 years ago

This is still happening for me, with the exact versions above even after v7.0.3 upgrade -any ideas? 😣 only with the extension of another styled component though

castamir commented 3 years ago

Well, it also depends how you use styled. For example, we import from styled-components/macro and this package does not work properly with it.

robert-hurst-cmd commented 3 years ago

I'm still seeing sc- classes in my code with v7.0.3. In my case this is happening when styling child components.

Given the following layout

<ParentComponent>
  <ChildComponent/>
</ParentComponent>

And styling ChildComponent via ParentComponent as such

const BaseChildComponent = styled.dev``
const ChildComponent = styled(BaseChildComponent)``
const ParentComponent = styled.div`
  ${BaseChildComponent} {
    text: red;
  }
`

My snapshot has the following:

.c0 .sc-dlfnbm {
    text: red;
}

Edit: looks like this is to do with wrapping the child component Edit 2: This turned out to be because of css that referenced styled components that were not used in the final render. I've opened a new ticket here.

castamir commented 3 years ago

@robert-hurst-cmd could you please provide the entire file including imports? It would be ideal if you can provide a standalone repro steps

robert-hurst-cmd commented 3 years ago

@castamir I've done some more debugging and narrowed down the issue, but it's not necessarily related to this issue. I've created a new issue here.

Daavidaviid commented 3 years ago

Still seeing a lot of empty media queries with these versions:

"styled-components": "^5.2.3",
"babel-plugin-styled-components": "^1.12.0",
"jest-styled-components": "^7.0.4",
y0n3r commented 3 years ago

I'm also experiencing this issue after upgrading styled-components from version 5.0.1 to 5.3.0. There are supposed to be no breaking changes between the two versions, but alas, all of our snapshot tests are now failing.

arthurgmalheiros commented 3 years ago

Any updates guys? It seems like every release of styled-components this fix is getting lost :(

stonebk commented 3 years ago

jest-styled-components@7.0.5 was just released and it appears to resolve a lot of the issues we were seeing with inconsistent classnames -- unsure if it has any effect on the original reported issue.

jimmymorris commented 3 years ago

it appears that the empty media queries are still present with 7.0.5

remy90 commented 2 years ago

Still occuring on 7.0.8: Screenshot 2021-12-02 at 13 55 32 Screenshot 2021-12-02 at 13 55 04

abhishekbhan commented 2 years ago

I am still getting the empty media queries. Any updates? "styled-components": "^5.3.0" "jest-styled-components": "^7.0.8"

TonyTroeff commented 2 years ago

This is still not fixed in jest-styled-components@7.0.8. I can see the sc-* generated class names and they are causing some issues because we have to constantly update our snapshots.

KidEma commented 2 years ago

I'm having multiple empty media queries as well in jest-styled-components@7.0.8

mmakarin commented 1 year ago

Same issue

fcabreraupgrade commented 1 year ago

Hey everyone!

I found a workaround to this issue but given I'm not an expert on the codebase I would need some help from the community.

If I change the following line: https://github.com/styled-components/jest-styled-components/blob/e94ebd2782ac945d8fc732e11fa3059374e10ff2/src/styleSheetSerializer.js#L77

to be:

const atRules = getAtRules(ast, filter).filter(entry => entry.rules.length > 0);

Makes all the empty media queries go away (this fails with @supports rules as well).

Looks the way the serializer search for rules, in some cases generates empty rules, so adding the filter fixes the problem on the project I'm working on.

Hope we could fix the problem with this!