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

v6: using styled(Component) results in looks like an unknown prop is being sent through to the DOM #4071

Closed
zackheil opened this issue Jul 2, 2023 · 10 comments

Comments

@zackheil
Copy link

zackheil commented Jul 2, 2023

I have noticed that some of my custom components that I attempt to style later with styled(MyComponent) are resulting in looks like an unknown prop is being sent through to the DOM messages. As far as I understand from reading the migration guide, that feature is still supported, but wanted to still know if this is a bug or I'm missing something. It seems to be different than #4058.

Take this simple demonstration component for example, I know that passing this message prop will result in it appearing in the DOM:

const Example = styled.h1<{ message: string }>`
  color: ${({ message }) => (message.includes('!') ? 'blue' : 'unset')};
`;

// usage:
<Example message="Hello!">Hello!</Example>

And that is exactly what I see:

<h1 message="Hello!" class="sc-dlfmnT cbdAIS">Hello!</h1>

But the behavior of wrapping an existing component in a styled() statement doesn't cause the props to display in the DOM:

const Example = ({ message, className }) => {
  return (
    <div className={className}>
      <h1>{message}</h1>
    </div>
  );
};
const StyledExample = styled(Example)`
  color: blue;
`;

// usage:
<StyledExample message="Example!" />
<div class="sc-aXZVg jxPTNQ"><h1>Example!</h1></div>

But now I get the following:

styled-components: it looks like an unknown prop "message" 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.) 

I saw the shouldForwardProp changes in the migration guide and I tried it just for fun, though I know it's meant for filtering props out, and got the following:

const Example = ({ message, className }) => {
  return (
    <div className={className}>
      <h1>{message}</h1>
    </div>
  );
};
const StyledExample = styled(Example).withConfig({
  shouldForwardProp: (prop) => 'message' !== prop,
})`
  color: blue;
`;

// usage:
<StyledExample message="Example!" />
<div class="sc-aXZVg jxPTNQ"><h1></h1></div>

though it did get rid of the warning

Reproduction

Here are 2 repros: v5 and v6. Both with examples in the App.tsx

v5: https://stackblitz.com/edit/vite-react-ts-keymr5?file=src%2FApp.tsx
v6: https://stackblitz.com/edit/vite-react-ts-vscvhm?file=src%2FApp.tsx

VPKSoft added a commit to VPKSoft/PasswordKeeper that referenced this issue Jul 4, 2023
@woodreamz
Copy link
Contributor

Same as #4049. I also created a really simple codesandbox similar to your example.

We are waiting for an answer but the warning seems to not be accurate.

@lucasgdb
Copy link

lucasgdb commented Jul 7, 2023

same issue here, using mui v5.

export const RCPasswordField = styled(({ InputProps, ...rest }) => <TextField {...rest} InputProps={InputProps} />)``

image

tried transient props, is-prop-valid from @emotion, withConfig and nothing solved. still having this issue.

@pankajpatel
Copy link

I faced the same issue and managed to work around it with the use of StyleSheetManager.

Here's what my provider looks like

  <StyleSheetManager
    enableVendorPrefixes
    shouldForwardProp={(propName, elementToBeRendered) => {
      return typeof elementToBeRendered === 'string' ? isPropValid(propName) : true;
    }}
    {...props}
  >
    <ThemeProvider theme={theme}>
      {...}
    </ThemeProvider>
  </StyleSheetManager>

Works well so far; still need to check the wide usage in production

himeno61 pushed a commit to himeno61/todo-app that referenced this issue Jul 10, 2023
All of them were result in recent changes in styled-components: styled-components/styled-components#4071
@jonasmarco
Copy link

jonasmarco commented Jul 11, 2023

I faced the same issue and managed to work around it with the use of StyleSheetManager.

Here's what my provider looks like

  <StyleSheetManager
    enableVendorPrefixes
    shouldForwardProp={(propName, elementToBeRendered) => {
      return typeof elementToBeRendered === 'string' ? isPropValid(propName) : true;
    }}
    {...props}
  >
    <ThemeProvider theme={theme}>
      {...}
    </ThemeProvider>
  </StyleSheetManager>

Works well so far; still need to check the wide usage in production

Hello, can you provide the complete code? For example, where does "isPropValid" come from?

I have this code below:

registry.tsx

'use client'

import React, { useState } from 'react'
import { useServerInsertedHTML } from 'next/navigation'
import {
  ServerStyleSheet,
  StyleSheetManager,
  ThemeProvider
} from 'styled-components'
import theme from '@/styles/theme'
import GlobalStyles from '@/styles/global'

export default function StyledComponentsRegistry({
  children
}: {
  children: React.ReactNode
}) {
  // Only create stylesheet once with lazy initial state
  // x-ref: https://reactjs.org/docs/hooks-reference.html#lazy-initial-state
  const [styledComponentsStyleSheet] = useState(() => new ServerStyleSheet())

  useServerInsertedHTML(() => {
    const styles = styledComponentsStyleSheet.getStyleElement()
    styledComponentsStyleSheet.instance.clearTag()
    return <>{styles}</>
  })

  if (typeof window !== 'undefined') return <>{children}</>

  return (
    <StyleSheetManager
      enableVendorPrefixes
      // shouldForwardProp={(propName, elementToBeRendered) => {
      //   return typeof elementToBeRendered === 'string' ? isPropValid(propName) : true;
      // }}
      sheet={styledComponentsStyleSheet.instance}
    >
      <ThemeProvider theme={theme}>
        {children}
        <GlobalStyles />
      </ThemeProvider>
    </StyleSheetManager>
  )
}

layout.tsx

export default function RootLayout({ children }: PropsWithChildren) {
  return (
    <html lang="pt-BR">
        <body>
          <StyledComponentsRegistry>{children}</StyledComponentsRegistry>
        </body>
    </html>
  )
}

@arsinclair
Copy link

I faced the same issue and managed to work around it with the use of StyleSheetManager.

Here's what my provider looks like

  <StyleSheetManager
    enableVendorPrefixes
    shouldForwardProp={(propName, elementToBeRendered) => {
      return typeof elementToBeRendered === 'string' ? isPropValid(propName) : true;
    }}
    {...props}
  >
    <ThemeProvider theme={theme}>
      {...}
    </ThemeProvider>
  </StyleSheetManager>

Works well so far; still need to check the wide usage in production

Does this have any performance implications? Is this check shouldForwardProp happening in the build-time or in the runtime?

@pankajpatel
Copy link

pankajpatel commented Jul 11, 2023

@arsinclair The shouldForwardProp check is happening on runtime, so I assume there will be some performance implications.

Maybe memorization can help.

Though I tried digging in the old version of styled components to see how it was handled, and it is at runtime

@pankajpatel
Copy link

@jonasmarco

Hello, can you provide the complete code? For example, where does "isPropValid" come from?

isPropValid is coming from is-prop-valid package as suggested by the SC migration guide.

@woodreamz
Copy link
Contributor

This should be fixed by this PR #4084. The warning will only be displayed on standard HTML elements and not when styling a React component.

@zackheil
Copy link
Author

This is fixed on my end. Thanks @woodreamz!

@iphyman
Copy link

iphyman commented Sep 22, 2023

Was having same issue and wanted to omit some other attributes that are been forwarded to the DOM, below code was helpful.

Packages comes from standard styled-components install

"use client";

import React, { useState } from "react";
import { useServerInsertedHTML } from "next/navigation";
import { ServerStyleSheet, StyleSheetManager } from "styled-components";
import isPropValid from "@emotion/is-prop-valid";

export default function StyledComponentsRegistry({
  children,
}: {
  children: React.ReactNode;
}) {
  // Only create stylesheet once with lazy initial state
  // x-ref: https://reactjs.org/docs/hooks-reference.html#lazy-initial-state
  const [styledComponentsStyleSheet] = useState(() => new ServerStyleSheet());

  useServerInsertedHTML(() => {
    const styles = styledComponentsStyleSheet.getStyleElement();
    styledComponentsStyleSheet.instance.clearTag();
    return <>{styles}</>;
  });

  if (typeof window !== "undefined") return <>{children}</>;

  return (
    <StyleSheetManager
      sheet={styledComponentsStyleSheet.instance}
      shouldForwardProp={(propName, elementToBeRendered) => {
        return typeof elementToBeRendered === "string"
          ? isPropValid(propName) && !["height", "width"].includes(propName)
          : true;
      }}
    >
      {children}
    </StyleSheetManager>
  );
}

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

7 participants