Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate to Styled Components v5 breaks snapshots #276

Open
shraddha-2407 opened this issue Nov 11, 2019 · 37 comments
Open

Migrate to Styled Components v5 breaks snapshots #276

shraddha-2407 opened this issue Nov 11, 2019 · 37 comments

Comments

@shraddha-2407
Copy link

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
Copy link

which versions specifically are you using?

@shraddha-2407
Copy link
Author

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
Copy link

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

I'm having quite different problems (see #285) with my setup, also on SC v5 and jest-styled-components v7 beta.

@gilbarbara
Copy link

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
Copy link

jure commented Jan 14, 2020

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
Copy link

iippis commented Jan 15, 2020

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
Copy link

@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:

it('referring to other components', () => {
const Link = styled.a`
display: flex;
align-items: center;
padding: 5px 10px;
background: papayawhip;
color: palevioletred;
`;
const Icon = styled.svg`
transition: fill 0.25s;
width: 48px;
height: 48px;
${Link}:hover & {
fill: rebeccapurple;
}
`;
const Label = styled.span`
display: flex;
align-items: center;
line-height: 1.2;
&::before {
content: '◀';
margin: 0 10px;
}
`;
const Container = styled.div`
color: black;
`;
const TextWithConditionalFormatting = styled.span`
${Container} & {
color: yellow;
background-color: ${props => (props.error ? 'red' : 'green')};
}
`;
const component = (
<Link href="#">
<Icon />
<Label>Hovering my parent changes my style!</Label>
<TextWithConditionalFormatting>I should be green</TextWithConditionalFormatting>
<TextWithConditionalFormatting error>I should be red</TextWithConditionalFormatting>
</Link>
);
expect(renderer.create(component).toJSON()).toMatchSnapshot('react-test-renderer');
expect(mount(component)).toMatchSnapshot('mount');
expect(render(component).container.firstChild).toMatchSnapshot('react-testing-library');
});

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

.sc-fzXfLU .c4 {
color: yellow;
background-color: green;
}
.sc-fzXfLU .c5 {
color: yellow;
background-color: red;
}

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
Copy link

jure commented Jan 22, 2020

@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
Copy link

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
Copy link

stevenocchipinti commented Feb 19, 2020

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
Copy link

zorfling commented Mar 4, 2020

Looks like this broke in #249.

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

@zorfling
Copy link

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

As @probablyup mentioned on styled-components/styled-components#2980 (comment) 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
Copy link

imdadul commented Mar 17, 2020

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

Any idea?

caasi pushed a commit to GSS-FED/vital-ui-kit-react that referenced this issue Mar 19, 2020
Style Components v5 will break class orders so we stay at v4 for now.

See:
styled-components/jest-styled-components#276
@ceari
Copy link

ceari commented Apr 24, 2020

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.

@hannadrehman
Copy link

hannadrehman commented May 3, 2020

is this fixed?. i am still facing this issue

@jamime
Copy link

jamime commented May 5, 2020

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
Copy link

hamatoyogi commented May 11, 2020

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
Copy link

mksztv commented Jul 8, 2020

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
Copy link

kerosan commented Jul 17, 2020

any updates?🤔

@castamir
Copy link

@kerosan a workaround was merged recently, no new release was created tho, see #320

@zorfling
Copy link

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

@tomdev10
Copy link

tomdev10 commented Nov 5, 2020

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
Copy link

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
Copy link
Contributor

robert-hurst-cmd commented Dec 13, 2020

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
Copy link

castamir commented Dec 16, 2020

@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
Copy link
Contributor

@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
Copy link

Daavidaviid commented Apr 21, 2021

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
Copy link

y0n3r commented May 22, 2021

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
Copy link

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

@stonebk
Copy link

stonebk commented Jul 9, 2021

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
Copy link

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

@remy90
Copy link

remy90 commented Dec 2, 2021

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

@abhishekbhan
Copy link

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

@TonyTroeff
Copy link

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
Copy link

KidEma commented Jun 22, 2022

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

@mmakarin
Copy link

mmakarin commented Apr 3, 2023

Same issue

@fcabreraupgrade
Copy link

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:

const atRules = getAtRules(ast, filter);

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests