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

Write docs for shouldForwardProp #655

Closed
Andarist opened this issue May 11, 2018 · 13 comments
Closed

Write docs for shouldForwardProp #655

Andarist opened this issue May 11, 2018 · 13 comments

Comments

@Andarist
Copy link
Member

Would be great to have this one documented, it might be used like this:

import isPropValid from '@emotion/is-prop-valid'

const StyledTextarea = styled(TextareaAutosize, { shouldForwardProp: isPropValid })({
  fontSize: '16px',
})

Another example can be found in this test case.

@stale
Copy link

stale bot commented Jul 24, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@tricoder42
Copy link

Hey @Andarist,
is it possible to set a global/default shouldForwardProp?

I'm using test prop to add a testing ID which I use in end-to-end tests and which is removed from production code.

I was thinking about using data-test prop instead, but that wouldn't work for components passing data-test down to child container:

const Button = ({ children, /*more props here */ data-test }) => {
  // some logic based on props

  return <StyledButton data-test={data-test}>{children}</StyledButton>
}

This doesn't work because data-test can't be a variable. dataTest isn't converted to data-test either and using spread operator just for one prop is a bit overkill :/

I would like to allow test prop in all styled components, but adding { shouldForwardProp: isTestProps } is again bit overkill. I think I'll use it as a workaround styled('div', withTestProps), but curious if I can override the default behavior.

@Andarist
Copy link
Member Author

U cant quite override the default behaviour, u could create a custom "proxy" styled factory that would just forward slightly modified arguments to the real one and use your proxy across your application.

I would encourage you to rethink if you need to pass data-test around as prop, seems like an anti-pattern to me (unless some specific use case needs it). Most commonly it should be just used as a simple marker, used plain in sight, not as a configurable prop.

@tricoder42
Copy link

I would like to keep using test prop to be honest. data-test was just a workaround because it's already valid prop (but invalid variable name in js).

I'll try custom proxy when it becomes more painful. Right now I have just one place in codebase where I want to use test prop on styled component. In this case I use good old className instead of styled component.

Thank you for clarification 👍

@eugene-beliaev
Copy link

eugene-beliaev commented Nov 12, 2018

Why don't make shouldForwardProp === '@emotion/is-prop-valid' by default? I often use props in css and it's really non-convenient to write this construction every time. I think it's rare case when you need to render invalid prop and it would be more convenient to sometimes write something like { shouldForwardProp: () => true } insead of to often write { shouldForwardProp: isPropValid }. How do you think guys? Or it's not possible due to performance degradation? @Andarist

@Andarist
Copy link
Member Author

It's a default for DOM tags - it's not for compose components though.

@lifeiscontent
Copy link
Contributor

lifeiscontent commented Dec 28, 2018

@Andarist what is the signature for shouldForwardProp in the case of something like styled.div({color: 'green'});

@Andarist
Copy link
Member Author

Not sure what do u mean, this example doesnt use showcase what props are being passed to that styled component.

@lifeiscontent
Copy link
Contributor

@Andarist I mean, how do I pass in shouldForwardProp to a component that is created with the styled.div signature.

@Andarist
Copy link
Member Author

you cant, you have to use factory syntax:

styled('div', { shouldForwardProp: prop => true })({ color: 'green' });

@EmaSuriano
Copy link

Hello guys,
I'm trying to create a wrapper of styled function with shouldForwardProp defined by me and only once in the entire project. I'm trying to do it with TS, but sadly I can't find the way to do it properly.

This is what I have:

import styled, { CreateStyled } from '@emotion/styled';
import isPropValid from '@emotion/is-prop-valid';

interface ITheme = {
  //myThemeStructure
}

const WHITE_LIST_PROPS = ['props', 'that', 'will', 'be', 'passed'];

const shouldForwardProp = (prop: string): boolean =>
  isPropValid(prop) || WHITE_LIST_PROPS.includes(prop);

const customStyled = (cmp: any) => styled(cmp, { shouldForwardProp });

export default customStyled as CreateStyled<ITheme>;

And then I'm receiving an error saying:

_shared_styled__WEBPACK_IMPORTED_MODULE_2__.default.div is not a function

This is happening when I'm writing styled.div which makes sense because in my wrapper I'm returning only a function. But then I don't know how to wrap the styled function from emotion ...

Could you please point me out what am I doing wrong? Also, if there is any way of doing this, I can add it to the docs. This could be helpful to other people also.

Thanks in advance!

@rathpc
Copy link

rathpc commented Jun 10, 2019

Hello guys,
I'm trying to create a wrapper of styled function with shouldForwardProp defined by me and only once in the entire project. I'm trying to do it with TS, but sadly I can't find the way to do it properly.

This is what I have:

import styled, { CreateStyled } from '@emotion/styled';
import isPropValid from '@emotion/is-prop-valid';

interface ITheme = {
  //myThemeStructure
}

const WHITE_LIST_PROPS = ['props', 'that', 'will', 'be', 'passed'];

const shouldForwardProp = (prop: string): boolean =>
  isPropValid(prop) || WHITE_LIST_PROPS.includes(prop);

const customStyled = (cmp: any) => styled(cmp, { shouldForwardProp });

export default customStyled as CreateStyled<ITheme>;

And then I'm receiving an error saying:

_shared_styled__WEBPACK_IMPORTED_MODULE_2__.default.div is not a function

This is happening when I'm writing styled.div which makes sense because in my wrapper I'm returning only a function. But then I don't know how to wrap the styled function from emotion ...

Could you please point me out what am I doing wrong? Also, if there is any way of doing this, I can add it to the docs. This could be helpful to other people also.

Thanks in advance!

Did you ever manage to get this working properly?

@teefouad
Copy link

teefouad commented Apr 7, 2021

If anyone is interested, I am using this:
(feel free to modify, improve, suggest ... whatever you want)

/* helpers.ts */

import { ReactElement, PropsWithChildren } from 'react';
import emotionStyled from '@emotion/styled';
import isPropValid from '@emotion/is-prop-valid';
import { css, SerializedStyles } from '@emotion/react';

type StyleGetter<T> = {
  (props: T): SerializedStyles;
};

type ProxiedStyled = {
  [key in keyof JSX.IntrinsicElements]: <T>(
    styleGetter?: StyleGetter<T>,
    whitelist?: string[],
  ) => (props: PropsWithChildren<T>) => ReactElement<T>;
};

export const styled = new Proxy({} as ProxiedStyled, {
  get(_, name: keyof JSX.IntrinsicElements) {
    return <T extends object>(
      styleGetter: StyleGetter<T> = () => css``,
      whitelist: (keyof T)[] = [],
    ) => emotionStyled(name, {
      shouldForwardProp: (prop: keyof T) => isPropValid(prop) || whitelist.includes(prop),
    })(styleGetter);
  },
});

then use it:

import { css } from '@emotion/react';
import { styled } from './helpers';

interface Props {
  textColor: string;
}

const Container = styled.div<Props>(({ textColor }) => css`
  position: relative;
  color: ${textColor};
`);

When you use <Container> in JSX, prop textColor will be automatically omitted and will not appear on the DOM element.
Unless you actually want it to appear, then you can whitelist it:

const Container = styled.div<Props>(({ textColor }) => css`
  position: relative;
  color: ${textColor};
`, ['textColor']);

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

8 participants