[docs] Improve documentation to cover shouldForwardProp

[jkjustjoshing]

I've been having issues with the following code:

const Box = styled('div')({
  background: ({ color }) => color
})
...
<Box color='red' />

The attribute color="red" gets added to the <div> in the DOM. Nothing in the documentation told me how to fix this. Some fixes on Stackoverflow or Spectrum.chat look like this:

const Box = styled(({ color, ...props }) => <div {...props} />)({
  background: ({ color }) => color
})

but this is messy, and removes the Typescript typings that Box has when doing styled('div').

I dug around in the source code of styled(), and it turns out there is a filterProps property I can use:

const Box = styled('div')({
  filterProps: ['color'],
  background: ({ color }) => color
})

This works great! Exactly what I need. However, a few problems:

1. As I mentioned, nowhere in the documentation can I find reference to this, much less how to use it.
2. Typescript doesn't like it -

const Box = styled('div')<any, { color: string }>({
  filterProps: ['color'], // error
  background: ({ color }) => color
})
/*
Type 'string[]' is not assignable to type 'string | number | JSSFontface | JSSFontface[] | CreateCSSProperties<{ color: string; }> | ((props: { color: string; }) => JSSFontface | JSSFontface[]) | ... 755 more ... | ((props: { ...; }) => VectorEffectProperty)'.
  Type 'string[]' is not assignable to type 'JSSFontface[]'.
    Type 'string' has no properties in common with type 'JSSFontface'
*/

3. It would probably make more sense adding it to the options object as a 2nd parameter:

const Box = styled('div')<any, { color: string }>({
  background: ({ color }) => color
}, {
  filterProps: ['color']
})

Implementing this change would be very simple:

1. Move this code block above the useStyles() call, and change it to this:

if (stylesOptions.filterProps) {
  filterProps = stylesOptions.filterProps;
  delete stylesOptions.filterProps;
}

2. In the Typescript type file on this line, make this change:

-   options?: WithStylesOptions<Theme>
+   options?: WithStylesOptions<Theme> & { filterProps?: string[] }

3. Throw together some quick documentation!

Since this field isn't documented, you could argue that moving it isn't a breaking change, since this isn't part of the public interface. However, it's probably better to:

1. Update the Typescript, add the option to the options object, but also allow it to still be specified in the style block
2. Add a console.warn() deprecation message in the console when NODE_ENV !== 'production'
3. Release this as a minor version bump
4. Remove the ability to specify in the style block in the next major version bump.

• The issue is present in the latest release.
• I have searched the issues of this repository and believe that this is not a duplicate.

[jdkula]

Since it appears this issue has not yet been claimed, I would love to take a stab at it! I've been using helper functions to implement the same functionality before realizing it was an undocumented part of styled.

It seems like adding similar functionality to withStyles would be similarly useful, too.

[oliviertassinari]

@jdkula We have been withholding all investment on the styling solution since v4 came out. One likely path is that we will dump it all. Until we know the direction we take, let's hold (applies for all package: styles issues). Thanks for the interest :).

[jkjustjoshing]

@oliviertassinari can you elaborate on what you mean by "dump it all", or point me to a discussion on the topic? That sounds like you're considering removing JSS entirely from the project - we're trying to decide between JSS and CSS Modules for my next project, and what MaterialUI uses is a non-zero part of that discussion.
EDIT - this is probably what you're referring to

[oliviertassinari]

We can close once our new experimentalStyled API gain more usage and is more battle-tested. The shouldForwardProp API should solve the issue.

[oliviertassinari]

An update, this issue is being resolved in v5 thanks to #22342 and the new @material-ui/styled-engine package.

We are normalizing with the emotion's shouldForwardProp API: https://emotion.sh/docs/styled#customizing-prop-forwarding.

import * as React from "react";
import { experimentalStyled as styled } from "@material-ui/core/styles";

interface BoxProps {
  foo: string;
}

const Box = styled("div", {
  shouldForwardProp: (prop) => prop !== "foo"
})(({ foo }: BoxProps) => ({
  backgroundColor: foo
}));

export default function UnstyledSlider() {
  return <Box foo="#000" style={{ width: 30, height: 30 }} />;
}

https://codesandbox.io/s/unstyledslider-material-demo-forked-rdp6p?file=/demo.tsx

The @material-ui/styled-engine-sc package allows using the same API with styled-components.

https://github.com/mui-org/material-ui/blob/f3d581b57f418b352e1ab9dbbd3bc96463744ef6/packages/material-ui-styled-engine-sc/src/index.js#L3-L7

[oliviertassinari]

I have repurposed the issue to cover v5 (instead of v4). We can:

1. Document the shouldForwardProp option: https://next.material-ui.com/guides/styled-engine/
2. Consider transient prop with the $ prefix [pigment-css] Add support for transient props with the styled API #25925

[mnajdova]

Should we consider this solved, now that we have https://next.material-ui.com/customization/styled/?