Custom shouldForwardProp is being preserved now when using `.withCo…

…mponent` (#1668)

* Forward shouldForwardProp through withComponent

- Fixes a bug that prevented `shouldForwardProp` from forwarding to components created via `withComponent`; Fixes #1394
- Compacts `createStyled`'s options argument

* Fixes an issue with label duplication

- Fixes duplicate labels when composing components.
- Allows for correctly setting the label for the new composed component.

* Refactor how withComponent handles shouldForwardProp

* Fix flow errors

Co-authored-by: Seth Benjamin <animecyc@Seths-MacBook-Pro.local>
Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>

.changeset/rotten-onions-hide.md

@@ -0,0 +1,5 @@
+---
+'@emotion/styled': minor
+---
+
+Custom `shouldForwardProp` is being preserved now when using `.withComponent`. Also, when passing an additional `shouldForwardProp` in `.withComponent`'s options (like this: `SomeComponent.withComponent('span', { shouldForwardProp })`) it's being composed with the potentially existing `shouldForwardProp`.

packages/styled/__tests__/styled.js

@@ -640,6 +640,7 @@ describe('styled', () => {
     const tree = renderer.create(<OneMoreComponent />).toJSON()
     expect(tree).toMatchSnapshot()
   })
+
   test('theming', () => {
     const Div = styled.div`
       color: ${props => props.theme.primary};

packages/styled/src/base.js

@@ -3,6 +3,7 @@ import * as React from 'react'
 import type { ElementType } from 'react'
 import {
   getDefaultShouldForwardProp,
+  composeShouldForwardProps,
   type StyledOptions,
   type CreateStyled,
   type PrivateStyledComponent
@@ -26,27 +27,18 @@ let createStyled: CreateStyled = (tag: any, options?: StyledOptions) => {
       )
     }
   }
+  const isReal = tag.__emotion_real === tag
+  const baseTag = (isReal && tag.__emotion_base) || tag
+
   let identifierName
-  let shouldForwardProp
   let targetClassName
   if (options !== undefined) {
     identifierName = options.label
     targetClassName = options.target
-    shouldForwardProp =
-      tag.__emotion_forwardProp && options.shouldForwardProp
-        ? propName =>
-            tag.__emotion_forwardProp(propName) &&
-            // $FlowFixMe
-            options.shouldForwardProp(propName)
-        : options.shouldForwardProp
   }
-  const isReal = tag.__emotion_real === tag
-  const baseTag = (isReal && tag.__emotion_base) || tag
 
-  if (typeof shouldForwardProp !== 'function' && isReal) {
-    shouldForwardProp = tag.__emotion_forwardProp
-  }
-  let defaultShouldForwardProp =
+  const shouldForwardProp = composeShouldForwardProps(tag, options, isReal)
+  const defaultShouldForwardProp =
     shouldForwardProp || getDefaultShouldForwardProp(baseTag)
   const shouldUseAs = !defaultShouldForwardProp('as')
 
@@ -196,12 +188,12 @@ let createStyled: CreateStyled = (tag: any, options?: StyledOptions) => {
       nextTag: ElementType,
       nextOptions?: StyledOptions
     ) => {
-      return createStyled(
-        nextTag,
-        nextOptions !== undefined
-          ? { ...(options || {}), ...nextOptions }
-          : options
-      )(...styles)
+      return createStyled(nextTag, {
+        ...options,
+        // $FlowFixMe
+        ...nextOptions,
+        shouldForwardProp: composeShouldForwardProps(Styled, nextOptions, true)
+      })(...styles)
     }
 
     return Styled

packages/styled/src/utils.js

@@ -38,6 +38,29 @@ export const getDefaultShouldForwardProp = (tag: ElementType) =>
     ? testOmitPropsOnStringTag
     : testOmitPropsOnComponent
 
+export const composeShouldForwardProps = (
+  tag: PrivateStyledComponent<any>,
+  options: StyledOptions | void,
+  isReal: boolean
+) => {
+  let shouldForwardProp
+  if (options) {
+    const optionsShouldForwardProp = options.shouldForwardProp
+    shouldForwardProp =
+      tag.__emotion_forwardProp && optionsShouldForwardProp
+        ? (propName: string) =>
+            tag.__emotion_forwardProp(propName) &&
+            optionsShouldForwardProp(propName)
+        : optionsShouldForwardProp
+  }
+
+  if (typeof shouldForwardProp !== 'function' && isReal) {
+    shouldForwardProp = tag.__emotion_forwardProp
+  }
+
+  return shouldForwardProp
+}
+
 export type CreateStyledComponent = <Props>(
   ...args: Interpolations
 ) => StyledComponent<Props>

packages/styled/test/__snapshots__/prop-filtering.test.js.snap

@@ -1,5 +1,12 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`composes shouldForwardProp on composed styled components 1`] = `
+<div
+  className="emotion-0 emotion-1"
+  xyz={true}
+/>
+`;
+
 exports[`custom shouldForwardProp works 1`] = `
 .emotion-0,
 .emotion-0 * {
@@ -124,3 +131,61 @@ Array [
   />,
 ]
 `;
+
+exports[`withComponent inherits explicit shouldForwardProp 1`] = `
+.emotion-0 {
+  color: hotpink;
+}
+
+<span
+  className="emotion-0 emotion-1"
+  foo={true}
+/>
+`;
+
+exports[`withComponent inherits explicit shouldForwardProp from flattened component 1`] = `
+.emotion-0 {
+  color: hotpink;
+  background-color: blue;
+}
+
+<span
+  className="emotion-0 emotion-1"
+  foo={true}
+/>
+`;
+
+exports[`withComponent should accept shouldForwardProp 1`] = `
+.emotion-0 {
+  color: hotpink;
+}
+
+<span
+  className="emotion-0 emotion-1"
+  xyz={true}
+/>
+`;
+
+exports[`withComponent should compose shouldForwardProp 1`] = `
+.emotion-0 {
+  color: hotpink;
+}
+
+<span
+  className="emotion-0 emotion-1"
+  qwe={true}
+  xyz={true}
+/>
+`;
+
+exports[`withComponent should compose shouldForwardProp with a flattened component 1`] = `
+.emotion-0 {
+  color: hotpink;
+}
+
+<span
+  className="emotion-0 emotion-1"
+  qwe={true}
+  xyz={true}
+/>
+`;

packages/styled/test/prop-filtering.test.js

@@ -6,17 +6,16 @@ import styled from '@emotion/styled'
 
 test('composes shouldForwardProp on composed styled components', () => {
   const StyledDiv = styled('div', {
-    shouldForwardProp: prop => prop === 'forwardMe'
+    shouldForwardProp: prop => prop !== 'foo'
   })()
 
   const ComposedDiv = styled(StyledDiv, {
-    shouldForwardProp: () => true
+    shouldForwardProp: prop => prop !== 'bar'
   })()
 
-  const tree = renderer.create(<ComposedDiv filterMe forwardMe />).toJSON()
+  const tree = renderer.create(<ComposedDiv foo bar xyz />).toJSON()
 
-  expect(tree.props.filterMe).toBeUndefined()
-  expect(tree.props.forwardMe).toBeDefined()
+  expect(tree).toMatchSnapshot()
 })
 
 test('custom shouldForwardProp works', () => {
@@ -470,3 +469,68 @@ test('prop filtering on composed styled components that are string tags', () =>
 
   expect(tree).toMatchSnapshot()
 })
+
+test('withComponent inherits explicit shouldForwardProp', () => {
+  const SomeComponent = styled('div', {
+    shouldForwardProp: prop => prop === 'foo'
+  })`
+    color: hotpink;
+  `
+  const AnotherComponent = SomeComponent.withComponent('span')
+  const tree = renderer.create(<AnotherComponent foo bar />).toJSON()
+  expect(tree).toMatchSnapshot()
+})
+
+test('withComponent inherits explicit shouldForwardProp from flattened component', () => {
+  const SomeComponent = styled('div', {
+    shouldForwardProp: prop => prop === 'foo'
+  })`
+    color: hotpink;
+  `
+  const AnotherComponent = styled(SomeComponent)`
+    background-color: blue;
+  `
+  const YetAnotherComponent = AnotherComponent.withComponent('span')
+  const tree = renderer.create(<YetAnotherComponent foo bar />).toJSON()
+  expect(tree).toMatchSnapshot()
+})
+
+test('withComponent should accept shouldForwardProp', () => {
+  const SomeComponent = styled('div')`
+    color: hotpink;
+  `
+  const AnotherComponent = SomeComponent.withComponent('span', {
+    shouldForwardProp: prop => prop === 'xyz'
+  })
+  const tree = renderer.create(<AnotherComponent qwe xyz />).toJSON()
+  expect(tree).toMatchSnapshot()
+})
+
+test('withComponent should compose shouldForwardProp', () => {
+  const SomeComponent = styled('div', {
+    shouldForwardProp: prop => prop !== 'foo'
+  })`
+    color: hotpink;
+  `
+  const AnotherComponent = SomeComponent.withComponent('span', {
+    shouldForwardProp: prop => prop !== 'bar'
+  })
+  const tree = renderer.create(<AnotherComponent foo bar qwe xyz />).toJSON()
+  expect(tree).toMatchSnapshot()
+})
+
+test('withComponent should compose shouldForwardProp with a flattened component', () => {
+  const SomeComponent = styled('div', {
+    shouldForwardProp: prop => prop !== 'foo'
+  })`
+    color: hotpink;
+  `
+  const AnotherComponent = styled(SomeComponent)`
+    background-color: blue;
+  `
+  const YetAnotherComponent = SomeComponent.withComponent('span', {
+    shouldForwardProp: prop => prop !== 'bar'
+  })
+  const tree = renderer.create(<YetAnotherComponent foo bar qwe xyz />).toJSON()
+  expect(tree).toMatchSnapshot()
+})