Merge pull request styled-components#828 from styled-components/fix/s…

…et-native-props-sfc Prevent ref prop from being passed to SFC
bhargav-kitchen · May 24, 2017 · 1e86cf9 · 1e86cf9
2 parents 647f230 + 148f45b
commit 1e86cf9
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -27,6 +27,7 @@ All notable changes to this project will be documented in this file. If a contri
 - Update StyledNativeComponent to match StyledComponent implementation.
 - Fix Theme context for StyledComponent for IE <10. (see [#807](https://github.com/styled-components/styled-components/pull/807))
 - Restore `setNativeProps` in StyledNativeComponent, thanks to [@MatthieuLemoine](https://github.com/MatthieuLemoine). (see [#764](https://github.com/styled-components/styled-components/pull/764))
+- Fix `ref` being passed to Stateless Functional Components in StyledNativeComponent. (see [#828](https://github.com/styled-components/styled-components/pull/828))
 - Add `displayName` to `componentId` when both are present (see [#821](https://github.com/styled-components/styled-components/pull/821))
 
 ## [v1.4.6] - 2017-05-02

diff --git a/src/models/StyledNativeComponent.js b/src/models/StyledNativeComponent.js
@@ -18,7 +18,7 @@ export default (constructWithOptions: Function) => {
     static styledComponentId: string
     static attrs: Object
     static inlineStyle: Object
-    root: Object
+    root: ?Object
 
     attrs = {}
     state = {
@@ -97,7 +97,19 @@ export default (constructWithOptions: Function) => {
     }
 
     setNativeProps(nativeProps: Object) {
-      this.root.setNativeProps(nativeProps)
+      if (this.root !== undefined) {
+        // $FlowFixMe
+        this.root.setNativeProps(nativeProps)
+      } else {
+        const { displayName } = this.constructor
+
+        // eslint-disable-next-line no-console
+        console.warn(
+          'setNativeProps was called on a Styled Component wrapping a stateless functional component. ' +
+          'In this case no ref will be stored, and instead an innerRef prop will be passed on.\n' +
+          `Check whether the stateless functional component is passing on innerRef as a ref in ${displayName}.`,
+        )
+      }
     }
 
     onRef = (node: any) => {
@@ -120,7 +132,14 @@ export default (constructWithOptions: Function) => {
         style: [generatedStyles, style],
       }
 
-      if (!isStyledComponent(target)) {
+      if (
+        !isStyledComponent(target) &&
+        (
+          // NOTE: We can't pass a ref to a stateless functional component
+          typeof target !== 'function' ||
+          (target.prototype && 'isReactComponent' in target.prototype)
+        )
+      ) {
         propsForElement.ref = this.onRef
         delete propsForElement.innerRef
       } else {

diff --git a/src/native/test/native.test.js b/src/native/test/native.test.js
@@ -164,10 +164,12 @@ describe('native', () => {
 
       const wrapper = mount(<Comp innerRef={ref} />)
       const view = wrapper.find('View').first()
+      const comp = wrapper.find(Comp).first()
 
       // $FlowFixMe
       expect(ref).toHaveBeenCalledWith(view.node)
       expect(view.prop('innerRef')).toBeFalsy()
+      expect(comp.node.root).toBeTruthy()
     })
 
     class InnerComponent extends React.Component {
@@ -182,10 +184,12 @@ describe('native', () => {
 
       const wrapper = mount(<OuterComponent innerRef={ref} />)
       const innerComponent = wrapper.find(InnerComponent).first()
+      const outerComponent = wrapper.find(OuterComponent).first()
 
       // $FlowFixMe
       expect(ref).toHaveBeenCalledWith(innerComponent.node)
       expect(innerComponent.prop('innerRef')).toBeFalsy()
+      expect(outerComponent.node.root).toBeTruthy()
     })
 
     it('should pass the innerRef to the wrapped styled component', () => {
@@ -196,9 +200,25 @@ describe('native', () => {
       const wrapper = mount(<OuterComponent innerRef={ref} />)
       const view = wrapper.find('View').first()
       const innerComponent = wrapper.find(InnerComponent).first()
+      const outerComponent = wrapper.find(OuterComponent).first()
 
       // $FlowFixMe
       expect(ref).toHaveBeenCalledWith(view.node)
+      expect(outerComponent.node.root).toBeTruthy()
+    })
+
+    it('should pass innerRef instead of ref to a wrapped stateless functional component', () => {
+      const InnerComponent = () => null
+      const OuterComponent = styled(InnerComponent)``
+      // NOTE: A ref should always be passed, so we don't need to (setNativeProps feature)
+
+      const wrapper = mount(<OuterComponent />)
+      const outerComponent = wrapper.find(OuterComponent).first()
+      const innerComponent = wrapper.find(InnerComponent).first()
+
+      expect(innerComponent.prop('ref')).toBeFalsy()
+      expect(innerComponent.prop('innerRef')).toBeTruthy()
+      expect(outerComponent.node.root).toBeFalsy()
     })
   })
 })