wave icon indicating copy to clipboard operation
wave copied to clipboard

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

Open martimalek opened this issue 2 years ago • 4 comments

  • @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 avatar Oct 05 '23 09:10 martimalek

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>
}

martimalek avatar Feb 15 '24 10:02 martimalek

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

nlopin avatar Feb 18 '24 22:02 nlopin

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 avatar Feb 22 '24 10:02 martimalek

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

martimalek avatar Feb 22 '24 10:02 martimalek

Closing for now, as we have the solution documented in the Documentation Website. Resolved by #433.

arturmiglio avatar Aug 28 '24 10:08 arturmiglio