babel-plugin-styled-components icon indicating copy to clipboard operation
babel-plugin-styled-components copied to clipboard

Add support for styled-as-a-hoc

Open sylvain-hamel opened this issue 7 years ago • 14 comments

We have a HOC that calls to styled. We use that HOC a lot and because our components don't use styled directly, it seems like the plug-in is not able to generate class names based on the component name.

This code breaks the plug-in:

import * as styled1  from 'styled-components';
const styled = styled1.default;
const MySuperComponent = styled(({children, className}) =>
    (<div className={className}>{children}</div>))`color: red`;

I suppose that our HOC somehow creates the same conditions that the plug-in does not support.

Is this fixable? It there anything we can do in our HOC to give the plug-in it a hint?

sylvain-hamel avatar Nov 29 '17 22:11 sylvain-hamel

Hmm, I think we could support this. Mind adding a failing test case based on the other tests? That'd be super helpful, thanks!

mxstbr avatar Nov 30 '17 09:11 mxstbr

I think we can just target styled, css, injectGlobal, and keyframes regardless of where those variables are defined... Because otherwise the detection code would get ridiculous.

I think that'd be a fair assumption to make. Wdyt, @mxstbr?

Edit: @sylvain-hamel btw HOCs for styling are not best practice, and there's some more caveats that I can think of that could hinder the plugin from working (now or in the near future). Can you post some small code snippet of that entire pattern, please, if that's possible that is? :smile:

kitten avatar Nov 30 '17 12:11 kitten

Because otherwise the detection code would get ridiculous.

I don't understand, what does the detection have to do anything? We already check that there's an import from styled-components and then we check if there's a styled() function call? It should only be a matter of adding .withConfig at the end of the styled function call no matter what's inside it, I don't think this has anything to do with detection.

mxstbr avatar Nov 30 '17 12:11 mxstbr

@mxstbr I mean, code like @sylvain-hamel's breaks because he's presumably importing a local file instead of styled-components. So maybe we should get rid of the import/require-based detection.

kitten avatar Nov 30 '17 12:11 kitten

Sorry? He's importing from styled-components though? I don't 100% understand what you're saying.

mxstbr avatar Nov 30 '17 12:11 mxstbr

Oh you mean that this breaks the detection?

import * as styled1  from 'styled-components';
const styled = styled1.default;

Possibly. @sylvain-hamel can you try making that a normal import styled from 'styled-components' import?

mxstbr avatar Nov 30 '17 12:11 mxstbr

Hi all, I'll send a test case shortly.

sylvain-hamel avatar Nov 30 '17 21:11 sylvain-hamel

Are tests supposed to run on Windows? They passed on my Mac Book but 17 of them fail on my windows computer.

sylvain-hamel avatar Dec 06 '17 02:12 sylvain-hamel

Ok, so here is the code.

This is the HOC:

// turns `styled` into a composable hoc
const composeStyledComponent = (value, ...rest) => 
    BaseComponent => styled(BaseComponent)(value, ...rest);

Code that uses the HOC:

const enhance = composeStyledComponent`color:red`;
const Component = ({className}) => <div className={className}>hello</div>;
const EnhancedComponent = enhance(Component);

And then some JSX somewhere

<EnhancedComponent/>

sylvain-hamel avatar Dec 06 '17 03:12 sylvain-hamel

I'd like an update on this issue if possible. Is the code I provided clear? Can the plug-in be modified to support that use case? Thanks

sylvain-hamel avatar Jan 05 '18 21:01 sylvain-hamel

I faced the same problem

import styled from 'styled-component'

const withStyled = (component) => styled(component)``

And then when I use that hoc in some other files, I always get the same class name withStyled and some hash.

alehatsman avatar Sep 23 '18 10:09 alehatsman

We do the hash calculation based on the location of the call to styled, so that makes sense if all your styled calls are in the exact same file. On Sun, Sep 23, 2018 at 5:35 AM Aleh Atsman [email protected] wrote:

I faced the same problem

import styled from 'styled-component' const withStyled = (component) => styled(component)``

And then when I use that hoc in some other files, I always get the same class name withStyled and some hash.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/styled-components/babel-plugin-styled-components/issues/108#issuecomment-423806760, or mute the thread https://github.com/notifications/unsubscribe-auth/AAiy1liVbKBcCcI6d9sg9RTLKixbilyCks5ud2PYgaJpZM4QvwPZ .

quantizor avatar Sep 23 '18 14:09 quantizor

@probablyup yeah I understood that. Any ideas how to fix that? Quite ofther i want to be able to use style interpolation like

// button.js
import styled from 'styled-components'
import withStyled from '...'
import Icon from '...'

const StyledButton = styled.button`
// ....
`

const Button = ({ text, ...props }) => (
  <StyledButton {...props}>
    <Icon />
    { text }
  </StyledButton>
)

// and here is what i want to do, because later probably 
// i would need to add styles related to positioning. 
export default withStyled(Button)
import styled from 'styled-components'
import Button from 'components/Button'

const StyledCard = styled.div`
  display: flex
  ${Button} {
    margin-top: 10px;
  }
`

const Card = () => (
  <StyledCard>
   <Button />
  </StyledCard>
)

So what i see here is quite similar to all macro problems, now that thing doesn't compose, and the only way is to pass styles as arguments, or to create a wrapper, quite sad.

Any ideas?

alehatsman avatar Sep 23 '18 16:09 alehatsman

Running into what I think is the same issue, but there's another distressing element to it - it works on macOS, and fails on Linux (CI).

import styled from 'styled-components';

export const styledSomething = Component => styled(Component)`
  display: flex;
`;

I didn't write any of this HoC code, and to be honest it seems unnecessary here, but why would it work on one platform and not the other? I'm curious about this one.

tabrindle avatar Feb 26 '19 17:02 tabrindle