From c337440c1b13958b73bd8b6dba15d0e54015449e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Phil=20Pl=C3=BCckthun?= Date: Wed, 24 May 2017 23:19:17 +0100 Subject: [PATCH 1/2] Prevent ref prop from being passed to SFC Due to the setNativeProps feature, a ref or innerRef will always be passed. This causes a warning for stateless, functional components, since those are not allowed to accept refs. This changes the behaviour to pass innerRef instead and output a warning if setNativeProps is used nonetheless. --- src/models/StyledNativeComponent.js | 25 ++++++++++++++++++++++--- src/native/test/native.test.js | 20 ++++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/models/StyledNativeComponent.js b/src/models/StyledNativeComponent.js index 9259259bc..4f03f20be 100644 --- 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 index 77221bf01..d7ee1e180 100644 --- a/src/native/test/native.test.js +++ b/src/native/test/native.test.js @@ -164,10 +164,12 @@ describe('native', () => { const wrapper = mount() 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() 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() 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() + 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() }) }) }) From 7e6b463b822ebe9a2405bfdad2fe54c8628951b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Phil=20Pl=C3=BCckthun?= Date: Wed, 24 May 2017 23:23:32 +0100 Subject: [PATCH 2/2] Add CHANGELOG entry I've put it below the `setNativeProps` one, in case someone is searching for both. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5404ee550..32e545201 100644 --- 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