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

it looks like an unknown prop is being sent through to the DOM, which will likely trigger a React console error. #4049

Closed
vadimcoder opened this issue Jun 22, 2023 · 45 comments · Fixed by #4084

Comments

@vadimcoder
Copy link

vadimcoder commented Jun 22, 2023

Screen Shot 2023-06-22 at 6 22 45 PM

Hi everyone! Thanks for the incredible tool, guys!
Could you please tell what to do if the prop is really needed for the underline react Button component?

@KilianShen
Copy link

KilianShen commented Jun 28, 2023

I encountered the same problem, how did you solve it?
image

@vadimcoder
Copy link
Author

I encountered the same problem, how did you solve it?

Hi bro, I downgraded to the 5 version instead of the 6 beta. But you can solve it also with .attrs:

Screenshot 2023-06-28 at 11 37 54 AM

https://styled-components.com/docs/basics#attaching-additional-props

@KilianShen
Copy link

I encountered the same problem, how did you solve it?

Hi bro, I downgraded to the 5 version instead of the 6 beta. But you can solve it also with .attrs:

Screenshot 2023-06-28 at 11 37 54 AM https://styled-components.com/docs/basics#attaching-additional-props

Sorry to disturb again, I still have warning information after I modify it, and even copy the ATTRE example on the official website, there will be the same problem.
image
image

@vadimcoder
Copy link
Author

@KilianShen I don't know man, I hope someone else will answer your question.

@vadimcoder vadimcoder reopened this Jun 28, 2023
@woodreamz
Copy link
Contributor

woodreamz commented Jun 28, 2023

6.0.1 seems to fix the warning on transient props ($prefix) but the warning is still displayed for other props.

I just created a codesanbox to reproduce the issue: codesanbox.

I get a warning on the property isWarning but it seems wrong to me because the prop is forwarded to the component Title which is a valid prop consumed by the component.

image

I am downgrading to v5 until we understand this warning.

@wadehammes
Copy link

wadehammes commented Jun 28, 2023

@woodreamz @vadimcoder https://styled-components.com/docs/faqs#shouldforwardprop-is-no-longer-provided-by-default

Essentially, isWarning is not a valid prop, so unless you use the above, you will see the warning OR update everything to use transient ($) props.

@woodreamz
Copy link
Contributor

woodreamz commented Jun 28, 2023

@woodreamz @vadimcoder https://styled-components.com/docs/faqs#shouldforwardprop-is-no-longer-provided-by-default

Essentially, isWarning is not a valid prop, so unless you use the above, you will see the warning OR update everything to use transient ($) props.

I cannot use transient props in this case because isWarning is a real property. Why are you saying isWarning is not a valid prop?

I read the migration, I understand the shouldForwardProp is no longer provided by default. For me it makes sense to have the warning on styled HTML elements but not on styled React components. Everybody migrating to v6 will have hundreds of warnings..

Does it mean, we have to add something like that just to hide the warning?

<StyleSheetManager shouldForwardProp={(name) => !name.startsWith('$')}>
            {/* other providers or your application's JSX */}
</StyleSheetManager>

UPDATE: By the way, my codesanbox is really similar to the transient props doc. This example generates a warning... Seems to be an issue with the condition to display the warning.

@wadehammes
Copy link

wadehammes commented Jun 28, 2023

isWarning is not a valid, HTML attribute (prop). You should use a library like @emotion/is-valid-prop which will check for props (attributes) being valid html attrs/props, pass them through, and removes all invalid ones.

import isValidProp from "@emotion/is-valid-prop";

<StyleSheetManager shouldForwardProp={isValidProp}>
            {/* other providers or your application's JSX */}
</StyleSheetManager>

I read the migration, I understand the shouldForwardProp is no longer provided by default. For me it makes sense to have the warning on styled HTML elements but not on styled React components. Everybody migrating to v6 will have hundreds of warnings..

yes, unfortunately, I had 1000s of these errors in our codebase because we use lots of props for config since our site is heavily configurable via Contentful. Had to use the above to revert to v5 functionality but still enabling us to update to v6, and then we will update to use transient props over time.

@woodreamz
Copy link
Contributor

@wadehammes Not sure you read my example. I know isWarning is not a valid HTML attribute, it is a prop of Title, I cannot use a transient props for that.

The warning is just wrong in this case, isWarning is not going to be on the DOM. But let's wait for a maintainer. For now, we will stay on v5.

Thanks for trying to help me!

@wadehammes
Copy link

wadehammes commented Jun 28, 2023

@wadehammes Not sure you read my example. I know isWarning is not a valid HTML attribute, it is a prop of Title, I cannot use a transient props for that.

The warning is just wrong in this case, isWarning is not going to be on the DOM. But let's wait for a maintainer. For now, we will stay on v5.

Thanks for trying to help me!

Sure you can, just do:

const Title = ({className, $isWarning = false}) => { if ($isWarning) { /* stuff here */ } }

const StyledTitle = styled(Title)``;

<StyledTitle $isWarning></StyledTitle>

If you were using Typescript as well, just for this example, you can type the props with the $ as well:

const TItle = ({className, $isWarning = false}: {className: string, $isWarning: boolean}) = {}

The warning is just wrong in this case, isWarning is not going to be on the DOM. But let's wait for a maintainer. For now, we will stay on v5.

The prop IS in the DOM though, StyledTitle creates a DOM element that has isWarning on it, therefore you need to use transient prop for it.

@woodreamz
Copy link
Contributor

@wadehammes No, you can't do that. It's the purpose of transient props, styled-components will consume $isWarning and not forward it to Title.

@wadehammes
Copy link

wadehammes commented Jun 28, 2023

const Title = ({className, $isWarning = false}) => { if ($isWarning) { /* stuff here */ } }

const StyledTitle = styled(Title)``;

<StyledTitle $isWarning>

wow, I'll admit I am wrong here. Just created this in code sandbox and I see now what is happening. It's throwing away $isWarning before it gets passed down so the Title return is wrong, but no console error. isWarning has a console error but also works properly.

@woodreamz sorry for the back and forth dude! hopefully they get it sorted for you

@woodreamz
Copy link
Contributor

@wadehammes No problem, I appreciate you tried to help on that!

@apolyanov
Copy link

+1 here, I am trying to style the FontAwesome react component but it says that the icon prop of the FontAwesome react component is not valid, which isn't true at all. And I can't use transient property since it's consumed by the FontAwesome react component. I will stay on v6 though since this is a warning that displays only during development. I hope we get this sorted out soon.

wolfpilot added a commit to wolfpilot/wolfpilot.github.io that referenced this issue Jun 29, 2023
@mkascel
Copy link

mkascel commented Jun 30, 2023

I too am quite confused by the docs suggestion of using the <StyleSheetManager shouldForwardProp={isPropValid}>, the docs seem to suggest that this will restore v5 behavior, but it seems to be filtering all of my props from being sent through the whole React component hierarchy, rather than just preventing them from being passed to the DOM.

@MrUltimate
Copy link

I'm also running into this issues. I'm still using the pages direction in NextJS 12 and can't seem to find a way to make it work with that either. Using the <StyleSheetManager /> means I'll have to force migrate to the app directory which breaks framer-motion support for <AnimatePresence />

Does anyone have any workouts? I think for now I will also downgrade back to v5

@antonycms
Copy link

This behavior was added in version 6.0.0, this is not a bug, you can read changes from version 6.0.0 here

@apolyanov
Copy link

@antonycms I think the warning itself is not correct. Let's take my use case for example. I am using the FontAwesome react component. As I mentioned the component takes icon as a property. They do what they do with it and it never gets to the html DOM. However since I am trying to style it with styled-components this icon prop passes through it. It does get consumed by the FontAwesome react component and it never gets to the DOM as styled-components think it does.

@apolyanov
Copy link

@probablyup I really want to start contributing and look into such issues, but I am having headaches setting up the environment locally. Can I get some assist from you?

@antonycms
Copy link

@apolyanov take a look at this breaking changes thread, it should solve your problem (it solved mine).

@owaisahmed5300
Copy link

Styled Component 6.0.1

In react base component you will need to add StyleSheetManager and set it's shouldForwardProp to isPropValid

  <React.StrictMode>
      <StyleSheetManager shouldForwardProp={isPropValid} disableVendorPrefixes={false}>
          <ThemeProvider theme={ThemeConfig}>
              <GlobalStyles />
              <Topbar />
              <App />
          </ThemeProvider>
      </StyleSheetManager>
  </React.StrictMode>,

You can import isPropValid from @emotion/is-prop-valid. This is included when you've installed the styled component.

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

You can learn more about that at their official docs: https://styled-components.com/docs/api#stylesheetmanager

@psecret
Copy link

psecret commented Jul 3, 2023

Styled Component 6.0.1

In react base component you will need to add StyleSheetManager and set it's shouldForwardProp to isPropValid

  <React.StrictMode>
      <StyleSheetManager shouldForwardProp={isPropValid} disableVendorPrefixes={false}>
          <ThemeProvider theme={ThemeConfig}>
              <GlobalStyles />
              <Topbar />
              <App />
          </ThemeProvider>
      </StyleSheetManager>
  </React.StrictMode>,

You can import isPropValid from @emotion/is-prop-valid. This is included when you've installed the styled component.

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

You can learn more about that at their official docs: https://styled-components.com/docs/api#stylesheetmanager

disableVendorPrefixes (v5, removed in v6)

@vadimcoder
Copy link
Author

Would rather shouldForwardProp={() => true}

@BiancaArtola
Copy link

any solution to this? I am facing the same issue

@quantizor
Copy link
Contributor

Do what the migration guide and/or dev-time warning suggests.

@woodreamz
Copy link
Contributor

woodreamz commented Jul 7, 2023

Do what the migration guide and/or dev-time warning suggests.

@probablyup Could you tell me what's the purpose of this warning? I am not sure to understand, for me it seems maybe useful if I style an HTML element (styled.div, styled.a...) because all the props passed could be passed to the DOM. Even that, React is already taking care of that by logging an error:

image

But when styling a React component, the warning seems completely wrong for me... Passing properties to a React component is one of the more basic thing you can do. Displaying a warning for that seems to go against React concepts. It seems, every people using styled-components > 6 will need to implement the workaround to bring back v5 behaviour.

In my comment, I provided a really basic example which is the same as the transient doc.

That being said, thank you for the work.

@quantizor
Copy link
Contributor

quantizor commented Jul 7, 2023

Well, it's meant as a warning. You don't have to heed it and if you know what you're doing and the underlying component intercepts the prop then all is well.

It's kind of a damned if you do, damned if you don't situation. We previously got a number of issues opened asking why props used for styling were going through to the DOM and triggering the React-side messaging.

Transient props were specifically added to fix the props-as-styling issue, but it's unclear if that pattern has become common yet or not.

@quantizor
Copy link
Contributor

If people are using transient props consistently, maybe this warning doesn't need to exist anymore.

@quantizor quantizor reopened this Jul 7, 2023
@apolyanov
Copy link

I can't seem to set up the local environment to work, because I really wanted to try and implement a fix for this and start contributing because if the prop is being consumed by a React component the seem wrong. In my opinion this should only show if a html element is being directly styled.

@quantizor
Copy link
Contributor

I can't seem to set up the local environment to work, because I really wanted to try and implement a fix for this and start contributing because if the prop is being consumed by a React component the seem wrong. In my opinion this should only show if a html element is being directly styled.

Ahh that's a good idea.

@yelsayed
Copy link

yelsayed commented Jul 7, 2023

If people are using transient props consistently, maybe this warning doesn't need to exist anymore.

@probablyup I don't think this is the issue. I'm using transient props consistently across my app. The problem is that I'm wrapping another component that expect other props. Those props need to be passed through, if you filter them out the wrapped component throws an error.

const StyledHelloWorld = styled(HelloWorld)<{ $styledProp }>`
  // Some style
`
const Component = () => {
  return <StyledHelloWorld actualProp={"hello world"} $styledProp={true} />
}

In this case HelloWorld expects actualProp and will not pass it to the DOM, ie it will consume it. But styled-components throws an error.

To fix this I just downgraded to v5. But I'd love to go back to v6 if someone is able to fix this.

@apolyanov
Copy link

I can't seem to set up the local environment to work, because I really wanted to try and implement a fix for this and start contributing because if the prop is being consumed by a React component the seem wrong. In my opinion this should only show if a html element is being directly styled.

Ahh that's a good idea.

Do you think this can be implemented, also is it possible for you to show me how you have set up styled-components locally?

@quantizor
Copy link
Contributor

quantizor commented Jul 7, 2023 via email

@apolyanov
Copy link

@probablyup I am on windows and installed yarn. Perhaps we can continue the conversation on another platform so we don't bloat the issue here. I am on discord - 20slimfingers

@vadimcoder
Copy link
Author

vadimcoder commented Jul 8, 2023

Hi @probablyup

We previously got a number of issues opened asking why props used for styling were going through to the DOM and triggering the React-side messaging.

OK the team solved those people's problem, but now the people who really know how to use styled-components properly (transient props) should suffer.

@yo8568
Copy link

yo8568 commented Jul 9, 2023

Hi ALL,

I also met the same warning when I installed styled-components v6.0.0-rc with next.js 13.
But I upgraded styled-components to v6.0.3, this warning would not be occurred again.

Hope my information can help someone question. 🙋‍♂️

@lfuelling
Copy link

The suggestion with isPropValid just breaks my application, while shouldForwardProp={() => true} removes the warnings while not breaking anything.

This warning appears to me for props that are never passed down to DOM elements, it seems to be very warn-happy.

@woodreamz
Copy link
Contributor

@probablyup @apolyanov I created a branch to try to only display the warning when styling HTML element, I published a draft PR #4084. For now, the only way I found to detect a HTML element is to check if the type is string (see diff:

      if (
        !shouldForwardProp &&
        process.env.NODE_ENV === 'development' &&
        !isPropValid(key) &&
        !seenUnknownProps.has(key) &&
        // Only warn on HTML Element.
        typeof elementToBeCreated === 'string'
      ) {
        seenUnknownProps.add(key);
        console.warn(
          `styled-components: it looks like an unknown prop "${key}" is being sent through to the DOM, which will likely trigger a React console error. If you would like automatic filtering of unknown props, you can opt-into that behavior via \`<StyleSheetManager shouldForwardProp={...}>\` (connect an API like \`@emotion/is-prop-valid\`) or consider using transient props (\`$\` prefix for automatic filtering.)`
        );
      }
    }
  }

But I am trying to find a more accurate condition, any idea?

@quantizor
Copy link
Contributor

We probably want to check it against the list from domElements.ts and if it's not part of that set then ignore it

@vadimcoder
Copy link
Author

Guys, @woodreamz @probablyup
It worked in the past. Could we just have the old code (responsible for that) back? If possible.

@apolyanov
Copy link

@vadimcoder with the PR that @woodreamz wants to merge this will be fixed and also still show error if we try and pass unknown props to HTML tags. It's a good to have warning when displayed correctly.

@belinde
Copy link

belinde commented Jul 20, 2023

Hi ALL,

I also met the same warning when I installed styled-components v6.0.0-rc with next.js 13. But I upgraded styled-components to v6.0.3, this warning would not be occurred again.

Hope my information can help someone question. raising_hand_man

Same for me. Jumped stright from 6.0.2 to 6.0.4 and the wanrings disappeared with no changes in the codebase.

@Fato07
Copy link

Fato07 commented Jul 27, 2023

If you are sure that all of your styled-components are using transient props and you're still getting the warning, then the issue might be with a third-party library or component that is not using transient props. If that's the case, you may want to consider using with a prop filtering function, as suggested in the warning message.

Here's how you can set that up:

import { StyleSheetManager } from 'styled-components';
import isValidProp from '@emotion/is-prop-valid';

<StyleSheetManager shouldForwardProp={propName => isValidProp(propName)}>
    <YourComponent />
</StyleSheetManager>

@douglasrcjames
Copy link

If you are sure that all of your styled-components are using transient props and you're still getting the warning, then the issue might be with a third-party library or component that is not using transient props. If that's the case, you may want to consider using with a prop filtering function, as suggested in the warning message.

Here's how you can set that up:

import { StyleSheetManager } from 'styled-components';
import isValidProp from '@emotion/is-prop-valid';

<StyleSheetManager shouldForwardProp={propName => isValidProp(propName)}>
    <YourComponent />
</StyleSheetManager>

Attempted this, but still getting the warning for a 3rd party datetime picker. For this lib I cant set the props to be transient since it wouldnt be captured.

@kyuhyunhan
Copy link

guys, don't waste your time on it anymore. As some of here already mentioned above, there was a kinda fix (or rafactor) on it.

https://github.com/styled-components/styled-components/releases/tag/v6.0.4

You can just try upgrade the version first, (and if it's not work, hope you find a better way😅)

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

Successfully merging a pull request may close this issue.