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

❗️React does not recognize the 'x' prop warning when using styled-components v6 #388

Open
martimalek opened this issue Oct 5, 2023 · 4 comments
Labels
bug Something isn't working conversation needed Issues that may need to go to a broader conversation

Comments

@martimalek
Copy link
Contributor

martimalek commented Oct 5, 2023

  • @freenow/wave version: 1.34.1

Relevant code

styled-components v6 does not filter out props automatically anymore, so they are passed to the component underneath and React warns about it.

image

Check the v6 release notes of styled-components

What was expected to happen?

The props should not be passed to the component underneath. To resolve this we can either use transient props (with $ prefix) or destructure them in each component so they aren't passed down.

Reproduction

https://codesandbox.io/s/wave-with-styled-components-v6-62tmpr

@martimalek martimalek added bug Something isn't working conversation needed Issues that may need to go to a broader conversation labels Oct 5, 2023
@nlopin nlopin changed the title React does not recognize the 'x' prop warning when using styled-components v6 ❗️React does not recognize the 'x' prop warning when using styled-components v6 Oct 5, 2023
@martimalek
Copy link
Contributor Author

martimalek commented Feb 15, 2024

I don't think using transient props or destructuring them in each component are realistic options for us, at least in the short term, using transient props would break our existing APIs and to destructure the props we'd most likely need to create some kind of wrapper for the styled-system compose function.

I'd instead try to document what can be done at the project level to filter those props (and maybe offer a helper from Wave that has to be opted-in at the project level).

The solution that I'm thinking of would be to filter the invalid props passing a shouldForwardProp function to the StyleSheetManager, we can use @emotion/is-prop-valid to get the filtering logic

Something like

import { ReactNode } from 'react'

import isPropValid from '@emotion/is-prop-valid'
import { StyleSheetManager, type IStyleSheetContext } from 'styled-components'

/**
 * Filters all props that are not valid as html attributes, for more information check out:
 * https://styled-components.com/docs/faqs#shouldforwardprop-is-no-longer-provided-by-default
 */
const shouldForwardProp: IStyleSheetContext['shouldForwardProp'] = (propName: string, target) => {
    if (typeof target === 'string') {
        // For HTML elements, forward the prop if it is a valid HTML attribute
        return isPropValid(propName)
    }
    // For other elements, forward all props
    return true
}

export const GlobalStyleSheetManager = ({ children }: { children: ReactNode }) => {
    return <StyleSheetManager shouldForwardProp={shouldForwardProp}>{children}</StyleSheetManager>
}

@nlopin
Copy link
Member

nlopin commented Feb 18, 2024

Can we wrap our components internally with it? If we can, it looks like we'll have to do this individually for each exported component, which is not the best, but better than relying on people reading the documentation. Let's discuss it on our next call

@martimalek
Copy link
Contributor Author

For certain components it may look cleaner to use the withConfig helper instead, we need to try it, e.g.

styled(Box).withConfig({
  shouldForwardProps: () => {}
})``

@martimalek
Copy link
Contributor Author

As a first step we'll add to the "Get started" section in our docs an explanation on how to get rid of the warnings by using the StyleSheetManager wrapper

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working conversation needed Issues that may need to go to a broader conversation
Development

No branches or pull requests

2 participants