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

Bring compatibility with v4 #191

Closed
santino opened this issue Sep 16, 2018 · 42 comments
Closed

Bring compatibility with v4 #191

santino opened this issue Sep 16, 2018 · 42 comments

Comments

@santino
Copy link
Contributor

santino commented Sep 16, 2018

Hi @MicheleBertoli,
just a quick one to see what your plan is to add compatibility with styled-components v4.

Right now jest-styled-components cannot really be used with v4; the main issue I came across so far is that when using Enzyme Shallow Rendering this renders an element like the following:
<BaseStyledComponent theme={{...}} forwardedClass={{...}} forwardedRef={{...}} />

The getHTML function will print this when that component is passed:
<style data-styled="" data-styled-version="4.0.0-beta.5"> /* sc-component-id: sc-bdVaJa */ </style>
Which means no styles rules can be found.

Everything works normally when using Full DOM Rendering since that would render

<ForwardRef theme={{...}}>
  <BaseStyledComponent theme={{...}} forwardedClass={{...}} forwardedRef={{...}}>
    <label className="sc-bdVaJa bGCxuB" />
  </BaseStyledComponent>
</ForwardRef>

Giving then access to the actual styled component (in the example above the label).

@MicheleBertoli
Copy link
Member

MicheleBertoli commented Sep 17, 2018

Hello @santino, thanks for opening this issue.

The main priority for this repo is now making the library compatible with v4.
We changed the regex to match the new attribute in #181, but, as you already found out, and as I mentioned in #183, there are a few major issues with Enzyme.

I pushed a work-in-progress branch where I'm experimenting, and of course some help is greatly appreciated :)

@Ky6uk
Copy link

Ky6uk commented Oct 16, 2018

I see SC v4 has been released recently. Can we have an estimation of the release v4-compatible version of this library? I found my tests just failed on the latest stable version of jest-styled-components right now.

@MicheleBertoli
Copy link
Member

Hey @Ky6uk, thanks for your comment.
Here's the work-in-progress, would you be happy to help?

@Ky6uk
Copy link

Ky6uk commented Oct 16, 2018

@MicheleBertoli yes, I can check it on my codebase to find problems. Do we have this branch as a npm package? Maybe under @beta tag or something?

@MicheleBertoli
Copy link
Member

MicheleBertoli commented Oct 21, 2018

Thank you very much @Ky6uk, I just published v7.0.0-0 under next.
Please let me know how it goes, and feel free to contribute to this repo.

@Ky6uk
Copy link

Ky6uk commented Oct 23, 2018

I see .js files inside src have different format in v7 in difference to v6. For example I'm not using babel at all and jest throws an error about unexpected syntax near import statements. Is it planned to leave those sources as ES6-modules?

@dumbNickname
Copy link

Hi. I just upgraded styled-components and I am facing issues when using shallow rendering as well. Here, for reference a simple example on codesandbox It contains just a simple component, maybe it helps you to confirm that everything works as expected. Basically the test fails with information:

Unable to find node on an unmounted component.

I already googled that enzyme shallow rendering calls life cycle methods by default, so I just tried to disable those with { disableLifecycleMethods: true },. I did not check how jest-styled-components work so that was just a blind shot, anyway it does not help as then test throws:

No style rules found on passed Component

it looks like I need to understand how this lib works, so will try to help with fixing - but I might need some help :P Anything besides unit tests passing that I should be aware of?

@MicheleBertoli
Copy link
Member

Thanks for the heads up @Ky6uk, I pushed a new version on @next without import.

Hey @dumbNickname, thank you very much for your comment.
I confirm that shallow rendering is the most problematic with this upgrade (to the point that I was thinking about deprecating Enzyme).
I'm happy to help understanding the library but I guess the most important thing is making the tests green :)

@dumbNickname
Copy link

Hey. I already created a pull request. I am almost done with next commit. The test that is still not green is failing due to: styled-components/styled-components#2071. I will try to enable next disabled tests and make those green. Nevertheless will need a bit more time.

Please let me know when you make up your mind regarding enzyme. As there is a global option to disable calling life cycle methods on enzyme configuration, I would ask you to keep it supported and publish my changes as beta release if possible.

@MicheleBertoli
Copy link
Member

This is awesome! Thank you very much for working on this, @dumbNickname.
I replied on the PR, and I can't wait to merge it.

@CoenWarmer
Copy link

Been following this with interest :) it seems the PR is ready- could it be merged? Really want to upgrade to SC4 and this is holding us back ;)

@MicheleBertoli
Copy link
Member

Hello @CoenWarmer, thanks for your comment.
Which PR are you referring to?

In order to use the work-in-progress version you need to run:
yarn add jest-styled-components@next

@karolisgrinkevicius
Copy link

karolisgrinkevicius commented Nov 16, 2018

@MicheleBertoli even using jest-styled-components@next and themeGet helper from styled-components@4.1.1 it doesn't work as expected. Enzyme's version is 3.7.0.

Consider the following as an example:

const MyStyledComponent = styled.div`
  padding-left: 0;
`;

const OverriddenMyStyledComponent = styled(MyStyledComponent)`
  //👇🏻 1024px
  ${media.lg`
      padding-left: ${themeGet('space.3')}px; // 👈🏻 1024px
  `};
`;

const wrapper = mount(<OverriddenMyStyledComponent />);
expect(wrapper).toHaveStyleRule('padding-left', '16px', { // 16px is the value of space[3] from theme
  media: `screen and (min-width: 1024px)`,
});

Media helper:

export const media = Object.keys(breakpointsMap).reduce(
  (accumulator, label) => {
    accumulator[label] = (strings, ...args) =>
      css`
        ${mediaQuery(label)} {
          ${css(strings, ...args)};
        }
      `;
    return accumulator;
  },
  {},
);

I get the following assertion error from jest:

Expected
      "padding-left: 16px"
    Received:
      "padding-left: px"

Do you have any suggestions on this?

@MicheleBertoli
Copy link
Member

MicheleBertoli commented Nov 18, 2018

Thanks for your comment, @karolisgrinkevicius.
The problem you described seems unrelated to this issue, and it should be fixed by wrapping the component into a ThemeProvider.
I hope this helps.

@Ky6uk
Copy link

Ky6uk commented Nov 19, 2018

@MicheleBertoli unfortunately those helpers (mWT, sWT) doesn't work with SC 4 because context logic has been changed.

@MicheleBertoli
Copy link
Member

Thanks for your comment, @Ky6uk.
ThemeProvider seems to work (I updated my comment).
The main idea is that themeGet can't work without a theme.

@loonywizard
Copy link

Hi everyone!
Our team is trying to migrate to styled-components v4, but despite our code works, tests fail with the only one error: "Check the render method of ThemeProvider."

In my package.json

"jest-styled-components": "^7.0.0-2",
"styled-components": "^4.1.2",

It seems that we have common problem, has anyone found solution?

@csvan
Copy link

csvan commented Feb 19, 2019

@ruchern no problem! There is mounting pressure from react-redux to make Enzyme fully compatible with React 16 as well (version 6.0 of that lib also has major issues with Enzyme due to Context API), so hopefully there will be considerable progress the coming weeks.

@csvan
Copy link

csvan commented Feb 19, 2019

Another important issue to follow: enzymejs/enzyme#1973

@ruchernchong
Copy link

ruchernchong commented Feb 19, 2019

@csvan React Hooks is also a mounting pressure for enzyme to move.

@csvan
Copy link

csvan commented Feb 19, 2019

@ruchern not sure, hooks is still very new and not too many major libs depend on it to work. Would be nice to see support anyway of course

@cihati
Copy link

cihati commented Feb 19, 2019

Getting warning " > jest-styled-components@7.0.0-2" has incorrect peer dependency "styled-components@^2.0.0 || ^3.0.2"., FYI

@aMoniker
Copy link

aMoniker commented Mar 5, 2019

I'm getting the same thing as @cihati. I'd be happy to use the published @next version, but it looks like the peer dependency version range wasn't updated to include ^4.0.0. This breaks tools like license_finder which will choke on the unmet peer dependency.

Is it possible to quickly update the peer dep version range for v7.0.0-2 so it includes v4?

@rodrijuarez
Copy link

hey @csvan! enzymejs/enzyme#1960 and enzymejs/enzyme#1966 have been solved by now, are there any updates on this one?
Thanks a lot for all the good work in this library 😁

@beausmith
Copy link

Same issue using jest-styled-components@next:

warning " > jest-styled-components@7.0.0-2" has incorrect peer dependency "styled-components@^2.0.0 || ^3.0.2".

Repro steps:

$ yarn add --dev jest-styled-components@next
yarn add v1.15.2
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
warning " > jest-styled-components@7.0.0-2" has incorrect peer dependency "styled-components@^2.0.0 || ^3.0.2".
[4/4] 🔨  Building fresh packages...
success Saved lockfile.
success Saved 1 new dependency.
info Direct dependencies
└─ jest-styled-components@7.0.0-2
info All dependencies
└─ jest-styled-components@7.0.0-2
✨  Done in 9.22s.

@csvan
Copy link

csvan commented Apr 16, 2019

Hey @rodrijuarez , this repo is maintained my @MicheleBertoli, not me :) Yes, I noted that a lot of the most important issues and PRs have been marked as resolved in Enzyme recently. Right now the best thing to do is wait for their next minor release and see how much it solves.

@dance-dance-banana-frenzy
Copy link

Having issues with

➜   ✗ npm install --save-dev jest-styled-components 
npm WARN eslint-config-react-app@4.0.1 requires a peer of eslint-plugin-flowtype@2.x but none is installed. You must install peer dependencies yourself.
npm WARN jest-styled-components@6.3.1 requires a peer of styled-components@^2.0.0 || ^3.0.2 but none is installed. You must install peer dependencies yourself.

+ jest-styled-components@6.3.1
added 3 packages from 38 contributors in 27.448s

I see the master branch in the jest-styled-components repo shows the following in package.json

{
"devDependencies": {
  "styled-components": "^3.4.5"
},
"peerDependencies": {
    "styled-components": "^2.0.0 || ^3.0.2"
  }
}

I currently have styled-components@^4.2.0 in my own project.

This appears to be an old issue (#183 closed, 3 open pr #184 #191 #304 ). Is there a fix in place soon?

@ghost
Copy link

ghost commented Jun 10, 2019

Currently awaiting this to be updated, as all shallow mounting on testing our components fail. Wondered if there was any update to this? I noticed that Enzyme have done a release as of 6 days so there could be some updates that might be now applicable for this.

There is a PR that is currently open, and have tried to install it locally but this is proving difficult to check.

@quantizor
Copy link
Contributor

v6.3.3 loosens the peerDependencies, but the library already works with styled-components v4 👍

@KITSCorbinCon
Copy link

KITSCorbinCon commented Jun 19, 2019

We are still having issues with our shallow mounted tests, more specifically finding applied styles with toHaveStyleRule.

Here is a code sandbox which shows our test failing (have to click on open in editor) after upgrading to styled components version 4.3.0 (plus other dependencies).

Previously this test is working fine with styled components version 3.4.5. In the above sandbox if I change back to this version the exact same test is working fine.

@probablyup

@PierreCapo
Copy link

PierreCapo commented Jul 5, 2019

For those who got issues with mountWithTheme and shallowWithTheme this is how we ended up with styled-components: 4.3.2 and enzyme: 3.10.0

import { ThemeProvider } from "styled-components";
import { yourTheme } from "path-to-your-theme";

const getThemeProviderWrappingComponent = theme => ({ children }) => (
  <ThemeProvider theme={yourTheme}>{children}</ThemeProvider>
);

export const shallowWithTheme = (tree: React.Node, theme: Object = yourTheme) => {
  return shallow(tree, {
    wrappingComponent: getThemeProviderWrappingComponent(theme)
  })
    .dive()
    .dive();
};

export const mountWithTheme = (component: React.Node, theme: Object = yourTheme) => {
  const wrapper = mount(component, {
    wrappingComponent: getThemeProviderWrappingComponent(theme)
  });

  return wrapper;
};

@marlonmleite
Copy link

@MicheleBertoli why the toHaveStyleRule not work with shallow (enzyme)?

@KonstantinKudelko
Copy link

For those who got issues with mountWithTheme and shallowWithTheme this is how we ended up with styled-components: 4.3.2 and enzyme: 3.10.0

import { ThemeProvider } from "styled-components";
import { yourTheme } from "path-to-your-theme";

const getThemeProviderWrappingComponent = theme => ({ children }) => (
  <ThemeProvider theme={yourTheme}>{children}</ThemeProvider>
);

export const shallowWithTheme = (tree: React.Node, theme: Object = coreLibDefaultTheme) => {
  return shallow(tree, {
    wrappingComponent: getThemeProviderWrappingComponent(theme)
  })
    .dive()
    .dive();
};

export const mountWithTheme = (component: React.Node, theme: Object = yourTheme) => {
  const wrapper = mount(component, {
    wrappingComponent: getThemeProviderWrappingComponent(theme)
  });

  return wrapper;
};
import React from 'react';
import Adapter from 'enzyme-adapter-react-16';
import { configure, shallow, render, mount } from 'enzyme';
import { ThemeProvider } from 'styled-components';

import AppTheme from './App/Themes';

configure({ adapter: new Adapter() });

const getThemeProvider = () => ({ children }) => (
  <ThemeProvider theme={AppTheme}>{children}</ThemeProvider>
);

export const mountWithTheme = component =>
  mount(component, {
    wrappingComponent: getThemeProvider(),
  });

export const shallowWithTheme = component =>
  shallow(component, {
    wrappingComponent: getThemeProvider(),
    disableLifecycleMethods: true,
  })
    .dive()
    .dive();

mountWithTheme is not working for me =(

Error:
Error: Could not parse CSS stylesheet

@raphaelbs
Copy link

raphaelbs commented Aug 22, 2019

@MicheleBertoli why the toHaveStyleRule not work with shallow (enzyme)?

In our company we have a lot of tests written with shallow that uses toHaveStyleRule.

We are migrating to material v4 and getting this problem as well.

"@material-ui/core": "^4.3.1",
"@material-ui/icons": "^4.2.1",
"styled-components": "^4.3.2",
"jest": "^23.0.0",
"jest-styled-components": "^6.3.3",
"enzyme": "^3.10.0",
"enzyme-to-json": "^3.4.0",

@ShayanBits
Copy link

ShayanBits commented Aug 23, 2019

I am also facing the same issue.

// Text.js Where I use a prop in a styled component to choose between rtl and ltr

import React from 'react';
import styled from 'styled-components';

export const Text = ({isA = 'span', isRtl, children, className, ...restProps}) => {
    const Tag = isA;
    return (
        <Tag className={className} {...restProps}>{children}</Tag>
    );
};

export const StyledText = styled(Text)`
  direction: ${({isRtl}) => (isRtl ? "rtl" : "ltr")}
`;

// Text.test.js where I want to check that my StyledText Component has a style of direction : ltr

    describe('<StyledText/>', () => {
        it('should have', () => {
            // when
            const wrapper = shallow(<StyledText isRtl={true}>{sampleText}</StyledText>);
            // then
            // console.log(wrapper.debug());
            expect(wrapper).toHaveStyleRule('direction', 'ltr');
        });
    });

// Error message when tests run:

 FAIL  src/__tests__/Text.test.js
  ● <Text /> › <StyledText/> › should have

    No style rules found on passed Component

      68 |             // then
      69 |            
    > 70 |             expect(wrapper).toHaveStyleRule('direction', 'ltr');
         |                             ^
      71 |         });
      72 |     });
      73 | });

@raphaelbs
Copy link

I've added a log in utils.js file.

image

Apparently styled-components is not generating the html with classes.
Idk, could be related to SC_DISABLE_SPEEDY.

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

No branches or pull requests