From 1347b4a9d9d7c1b69d65c42339ea7d4d4e4c02f5 Mon Sep 17 00:00:00 2001 From: Ian Sutherland Date: Tue, 15 Aug 2017 03:21:49 -0600 Subject: [PATCH] Added unit tests for creating an element with a ref in a constructor. Only set ReactCurrentOwner.current in dev mode when the component has no constructor. (#10025) nhunzaker: I've added an additional line to the ref test that sets the NODE_ENV invironment variable. This allows the test to pass. --- .../reconciler/ReactCompositeComponent.js | 2 +- .../stack/reconciler/__tests__/refs-test.js | 76 +++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js b/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js index ec90a21068dc..36e3744a59f5 100644 --- a/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js +++ b/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js @@ -372,7 +372,7 @@ var ReactCompositeComponent = { publicContext, updateQueue, ) { - if (__DEV__) { + if (__DEV__ && !doConstruct) { ReactCurrentOwner.current = this; try { return this._constructComponentWithoutOwner( diff --git a/src/renderers/shared/stack/reconciler/__tests__/refs-test.js b/src/renderers/shared/stack/reconciler/__tests__/refs-test.js index b7b835bfc757..6db15dfb7b4f 100644 --- a/src/renderers/shared/stack/reconciler/__tests__/refs-test.js +++ b/src/renderers/shared/stack/reconciler/__tests__/refs-test.js @@ -13,6 +13,7 @@ var React = require('React'); var ReactTestUtils = require('ReactTestUtils'); +var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags'); var reactComponentExpect = require('reactComponentExpect'); @@ -283,3 +284,78 @@ describe('ref swapping', () => { __DEV__ = originalDev; }); }); + +describe('creating element with ref in constructor', () => { + class RefTest extends React.Component { + constructor(props) { + super(props); + this.p =

Hello!

; + } + + render() { + return
{this.p}
; + } + } + + var devErrorMessage = + 'addComponentAsRefTo(...): Only a ReactOwner can have refs. You might ' + + "be adding a ref to a component that was not created inside a component's " + + '`render` method, or you have multiple copies of React loaded ' + + '(details: https://fb.me/react-refs-must-have-owner).'; + + var prodErrorMessage = + 'Minified React error #119; visit ' + + 'http://facebook.github.io/react/docs/error-decoder.html?invariant=119 for the full message ' + + 'or use the non-minified dev environment for full errors and additional helpful warnings.'; + + var fiberDevErrorMessage = + 'Element ref was specified as a string (p) but no owner was ' + + 'set. You may have multiple copies of React loaded. ' + + '(details: https://fb.me/react-refs-must-have-owner).'; + + var fiberProdErrorMessage = + 'Minified React error #149; visit ' + + 'http://facebook.github.io/react/docs/error-decoder.html?invariant=149&args[]=p ' + + 'for the full message or use the non-minified dev environment for full errors and additional ' + + 'helpful warnings.'; + + it('throws an error when __DEV__ = true', () => { + ReactTestUtils = require('ReactTestUtils'); + + var originalDev = __DEV__; + __DEV__ = true; + + try { + expect(function() { + ReactTestUtils.renderIntoDocument(); + }).toThrowError( + ReactDOMFeatureFlags.useFiber ? fiberDevErrorMessage : devErrorMessage, + ); + } finally { + __DEV__ = originalDev; + } + }); + + it('throws an error when __DEV__ = false', () => { + ReactTestUtils = require('ReactTestUtils'); + + var originalDev = __DEV__; + var originalEnv = process.env.NODE_ENV; + + __DEV__ = false; + process.env.NODE_ENV = 'production'; + + try { + expect(function() { + ReactTestUtils.renderIntoDocument(); + }).toThrowError( + ReactDOMFeatureFlags.useFiber + ? fiberProdErrorMessage + : prodErrorMessage, + ); + } finally { + __DEV__ = originalDev; + process.env.NODE_ENV = originalEnv; + } + }); +});