styled-components / babel-plugin-styled-components

Improve the debugging experience and add server-side rendering support to styled-components
MIT License
1.07k stars 141 forks source link

Add support for styled-as-a-hoc #108

Open sylvain-hamel opened 6 years ago

sylvain-hamel commented 6 years ago

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?

mxstbr commented 6 years ago

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

kitten commented 6 years ago

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:

mxstbr commented 6 years ago

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.

kitten commented 6 years ago

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

mxstbr commented 6 years ago

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

mxstbr commented 6 years ago

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?

sylvain-hamel commented 6 years ago

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

sylvain-hamel commented 6 years ago

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

sylvain-hamel commented 6 years ago

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 commented 6 years ago

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

alehatsman commented 6 years ago

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.

quantizor commented 6 years ago

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 notifications@github.com 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 .

alehatsman commented 6 years ago

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

tabrindle commented 5 years ago

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.