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

No placeholder classes generated #49

Closed
guioconnor opened this issue Jul 17, 2017 · 12 comments
Closed

No placeholder classes generated #49

guioconnor opened this issue Jul 17, 2017 · 12 comments

Comments

@guioconnor
Copy link

While using the library, it seems all the classes on the component are replaced by an empty string, those created by StyledComponents as well as any other.

image

Package.json

    "enzyme-to-json": "^1.5.1",
    "jest-styled-components": "^4.0.2",

Test file

import 'jest-styled-components';
import toJson from 'enzyme-to-json';

...

    const container = shallow(<MyComponent></MyComponent>);
    expect(toJson(container)).toMatchSnapshot();
@MicheleBertoli
Copy link
Member

Hello @guioconnor, the new version of this packages strips out all the class names that don't have any style rules attached to them.
In your case, that's ok for the non-styled-components classes, but I would expect the styled-components ones to be replaced by a placeholder.
Any chances you could provide a minimal non-working example? That would help a lot.
Thanks!

@guioconnor
Copy link
Author

guioconnor commented Jul 19, 2017

Thanks @MicheleBertoli.

At a glance I find it counter productive to remove the classes for non styled components. The snapshot above is a good example of how difficult to read a component snapshot becomes without its classes. Dropdown, header and body become generic <div>s and unintended changes on them may become more difficult to notice.

I'm curious to what was the rationale for removing them?

As for the styled components, I'll take an educated guess here and say that the classes were stripped as a consequence of the style code not being added to the snapshot. IE, the plugin hasn't detected there is style associated with it.

I don't have readily available code I can share but I think this may be consequence of what we are trying to do. We are building a library of common components as a separate package and the <Button> above is the first migrated component.

I guess the styles may be being lost in in the import for snapshot purposes, though it works in the actual application.

ruyadorno added a commit to ruyadorno/jest-styled-components that referenced this issue Jul 20, 2017
ruyadorno added a commit to ruyadorno/jest-styled-components that referenced this issue Jul 20, 2017
@rsouthgate
Copy link

Tried this with the minimal example in the readme on 4.0.3:

import 'jest-styled-components';
import styled from 'styled-components';
import renderer from 'react-test-renderer';

describe('StyledComponentTests', () => {
    it('works', () => {
        const Button = styled.button`
            color: red;
        `
        const tree = renderer.create(<Button />).toJSON()
        expect(tree).toMatchSnapshot()
        expect(tree).toHaveStyleRule('color', 'red')
    });
});

snapshot gets classname stripped

exports[`StyledComponentTests works 1`] = `
<button
  className=""
/>
`;

and test fails on second assertion:

Property not found: "color"

@MicheleBertoli
Copy link
Member

@rsouthgate I can't repro the issue with v4.0.3.

When I run the code above, I get:

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

<button
  className="c0"
/>
`;

@MicheleBertoli
Copy link
Member

@guioconnor thanks to @ruyadorno, the package now preserves the custom class names.

The problem is now solved, but to get the most out of this package I'd suggest you to use it for testing single components (e.g. Button) rather than the entire tree.
The snapshots will be less noisy and more useful.

@guioconnor
Copy link
Author

Thanks @MicheleBertoli. Awesome.

I couldn't yet solve the other issue, also mentioned by @rsouthgate, at least I'm glad to see I'm not alone. I'm going to poke the problem more and report back on it with my findings.

@MicheleBertoli
Copy link
Member

Sure! Please open a new issue as soon as you have a non-working example.
Thank you very much!

@rsouthgate
Copy link

@MicheleBertoli I stripped it out of the project I was working on into a minimal dependency setup and yes, it works as you showed. The other project uses Lerna - so I'm thinking the dependency hoisting might be breaking something... I'll investigate and update here if I figure out what is causing the issue. @guioconnor Don't suppose you are using lerna where you are seeing the same issue?

@MicheleBertoli
Copy link
Member

This is interesting, thanks @rsouthgate.
If you have a way to repro the issue consistently with Lerna, I'd really love to check it out.

@rsouthgate
Copy link

@MicheleBertoli I can't tell why it's happening but I can tell how it's happening. In the lerna structure the root package.json has a devDependency of styled-components: ^2.1.1 (and jest-styled-components: ^4.0.3). Within the package with the test that produces this error there is a package.json with a dependency of styled-components: ^2.1.1. If I remove the dependency from the package's package.json (and remove styled-components from the package's node_modules - leaving the very same version in the top level node_modules) then everything works!

I'm mystified as to why though!

@rsouthgate
Copy link

@MicheleBertoli Just to follow up... if I explicitly bootstrap lerna with --hoist (so styled-components gets symlinked to the root version) the problem is fixed. The only implication for me is that current lerna hoisting only works with npm4 but for now that's good enough for me. I imagine this is a problem with something lerna is doing rather than anything either jest-styled-components or styled-components is doing, but thought I should follwo up here in case anyone else come across it.

@guioconnor
Copy link
Author

I haven't dug into the problem yet because I'm not currently working on the package that has this issue but my project is also Lerna-based, so whatever the problem is, seems to be related to the use of Lerna.

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

3 participants