From 6fb8133ed3aa6b23063375dd345c6e413b05f0fe Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Thu, 17 Nov 2022 01:15:57 +0100 Subject: [PATCH] Turn on string ref deprecation warning for everybody (not codemoddable) (#25383) ## Summary Alternate to https://github.com/facebook/react/pull/25334 without any prod runtime changes i.e. the proposed codemod in https://github.com/reactjs/rfcs/blob/createlement-rfc/text/0000-create-element-changes.md#deprecate-string-refs-and-remove-production-mode-_owner-field would not work. ## How did you test this change? - [x] CI - [x] `yarn test` with and without `warnAboutStringRefs` --- .../src/__tests__/ReactComponent-test.js | 29 +- .../__tests__/ReactComponentLifeCycle-test.js | 6 +- .../__tests__/ReactCompositeComponent-test.js | 63 ++-- .../__tests__/ReactDOMEventListener-test.js | 5 +- .../src/__tests__/ReactDOMInput-test.js | 21 +- ...eactDOMServerIntegrationAttributes-test.js | 2 +- .../ReactDOMServerIntegrationRefs-test.js | 22 +- .../src/__tests__/ReactIdentity-test.js | 5 +- .../ReactMultiChildReconcile-test.js | 10 +- .../ReactServerRenderingHydration-test.js | 8 +- .../src/__tests__/ReactTestUtils-test.js | 27 +- .../src/__tests__/ReactUpdates-test.js | 54 ++-- .../src/__tests__/refs-destruction-test.js | 62 ++-- packages/react-dom/src/__tests__/refs-test.js | 282 +++++++++++------- .../ResponderEventPlugin-test.internal.js | 69 +++-- .../src/ReactChildFiber.new.js | 15 +- .../src/ReactChildFiber.old.js | 15 +- .../ReactIncrementalSideEffects-test.js | 17 +- .../ReactTestRenderer-test.internal.js | 19 +- .../ReactCoffeeScriptClass-test.coffee | 18 +- .../__tests__/ReactContextValidator-test.js | 6 +- .../react/src/__tests__/ReactES6Class-test.js | 18 +- .../react/src/__tests__/ReactElement-test.js | 2 +- .../src/__tests__/ReactElementClone-test.js | 35 ++- .../src/__tests__/ReactElementJSX-test.js | 2 +- .../src/__tests__/ReactJSXElement-test.js | 5 +- .../src/__tests__/ReactStrictMode-test.js | 39 ++- .../__tests__/ReactTypeScriptClass-test.ts | 17 +- packages/shared/ReactFeatureFlags.js | 2 +- .../forks/ReactFeatureFlags.native-fb.js | 2 +- .../forks/ReactFeatureFlags.native-oss.js | 2 +- .../forks/ReactFeatureFlags.test-renderer.js | 2 +- .../ReactFeatureFlags.test-renderer.native.js | 2 +- .../ReactFeatureFlags.test-renderer.www.js | 2 +- .../shared/forks/ReactFeatureFlags.testing.js | 2 +- .../forks/ReactFeatureFlags.testing.www.js | 2 +- .../shared/forks/ReactFeatureFlags.www.js | 2 +- 37 files changed, 591 insertions(+), 300 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactComponent-test.js b/packages/react-dom/src/__tests__/ReactComponent-test.js index 9296c2d0ae..4df1b8dfc1 100644 --- a/packages/react-dom/src/__tests__/ReactComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactComponent-test.js @@ -12,6 +12,7 @@ let React; let ReactDOM; let ReactDOMServer; +let ReactFeatureFlags; let ReactTestUtils; describe('ReactComponent', () => { @@ -21,6 +22,7 @@ describe('ReactComponent', () => { React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); ReactTestUtils = require('react-dom/test-utils'); }); @@ -36,7 +38,7 @@ describe('ReactComponent', () => { }).toThrowError(/Target container is not a DOM element./); }); - it('should throw when supplying a ref outside of render method', () => { + it('should throw when supplying a string ref outside of render method', () => { let instance =
; expect(function() { instance = ReactTestUtils.renderIntoDocument(instance); @@ -102,7 +104,7 @@ describe('ReactComponent', () => { } }); - it('should support refs on owned components', () => { + it('should support string refs on owned components', () => { const innerObj = {}; const outerObj = {}; @@ -133,10 +135,29 @@ describe('ReactComponent', () => { } } - ReactTestUtils.renderIntoDocument(); + expect(() => { + ReactTestUtils.renderIntoDocument(); + }).toErrorDev( + ReactFeatureFlags.warnAboutStringRefs + ? [ + 'Warning: Component "div" contains the string ref "inner". ' + + 'Support for string refs will be removed in a future major release. ' + + 'We recommend using useRef() or createRef() instead. ' + + 'Learn more about using refs safely here: https://reactjs.org/link/strict-mode-string-ref\n' + + ' in div (at **)\n' + + ' in Wrapper (at **)\n' + + ' in Component (at **)', + 'Warning: Component "Component" contains the string ref "outer". ' + + 'Support for string refs will be removed in a future major release. ' + + 'We recommend using useRef() or createRef() instead. ' + + 'Learn more about using refs safely here: https://reactjs.org/link/strict-mode-string-ref\n' + + ' in Component (at **)', + ] + : [], + ); }); - it('should not have refs on unmounted components', () => { + it('should not have string refs on unmounted components', () => { class Parent extends React.Component { render() { return ( diff --git a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js index cc2588fe72..46b1fd9bb8 100644 --- a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js +++ b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js @@ -378,7 +378,7 @@ describe('ReactComponentLifeCycle', () => { } // you would *NEVER* do anything like this in real code! this.state.hasRenderCompleted = true; - return
I am the inner DIV
; + return
I am the inner DIV
; } componentWillUnmount() { @@ -477,7 +477,9 @@ describe('ReactComponentLifeCycle', () => { class Component extends React.Component { render() { return ( - {this.props.tooltipText}
}> + {this.props.tooltipText}}> {this.props.text} ); diff --git a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js index ed1e36f9ba..4510ebf001 100644 --- a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js @@ -72,6 +72,8 @@ describe('ReactCompositeComponent', () => { MorphingComponent = class extends React.Component { state = {activated: false}; + xRef = React.createRef(); + _toggleActivatedState = () => { this.setState({activated: !this.state.activated}); }; @@ -79,9 +81,9 @@ describe('ReactCompositeComponent', () => { render() { const toggleActivatedState = this._toggleActivatedState; return !this.state.activated ? ( - + ) : ( - + ); } }; @@ -91,14 +93,16 @@ describe('ReactCompositeComponent', () => { * reallocated again. */ ChildUpdates = class extends React.Component { + anchorRef = React.createRef(); + getAnchor = () => { - return this.refs.anch; + return this.anchorRef.current; }; render() { const className = this.props.anchorClassOn ? 'anchorClass' : ''; return this.props.renderAnchor ? ( - + ) : ( ); @@ -186,11 +190,11 @@ describe('ReactCompositeComponent', () => { it('should rewire refs when rendering to different child types', () => { const instance = ReactTestUtils.renderIntoDocument(); - expect(instance.refs.x.tagName).toBe('A'); + expect(instance.xRef.current.tagName).toBe('A'); instance._toggleActivatedState(); - expect(instance.refs.x.tagName).toBe('B'); + expect(instance.xRef.current.tagName).toBe('B'); instance._toggleActivatedState(); - expect(instance.refs.x.tagName).toBe('A'); + expect(instance.xRef.current.tagName).toBe('A'); }); it('should not cache old DOM nodes when switching constructors', () => { @@ -739,10 +743,13 @@ describe('ReactCompositeComponent', () => { } class Wrapper extends React.Component { + parentRef = React.createRef(); + childRef = React.createRef(); + render() { return ( - - + + ); } @@ -750,14 +757,14 @@ describe('ReactCompositeComponent', () => { const wrapper = ReactTestUtils.renderIntoDocument(); - expect(wrapper.refs.parent.state.flag).toEqual(true); - expect(wrapper.refs.child.context).toEqual({flag: true}); + expect(wrapper.parentRef.current.state.flag).toEqual(true); + expect(wrapper.childRef.current.context).toEqual({flag: true}); // We update while is still a static prop relative to this update - wrapper.refs.parent.setState({flag: false}); + wrapper.parentRef.current.setState({flag: false}); - expect(wrapper.refs.parent.state.flag).toEqual(false); - expect(wrapper.refs.child.context).toEqual({flag: false}); + expect(wrapper.parentRef.current.state.flag).toEqual(false); + expect(wrapper.childRef.current.context).toEqual({flag: false}); }); it('should pass context transitively', () => { @@ -1142,14 +1149,17 @@ describe('ReactCompositeComponent', () => { } class Component extends React.Component { + static0Ref = React.createRef(); + static1Ref = React.createRef(); + render() { if (this.props.flipped) { return (
- + B (ignored) - + A (ignored)
@@ -1157,10 +1167,10 @@ describe('ReactCompositeComponent', () => { } else { return (
- + A - + B
@@ -1171,14 +1181,14 @@ describe('ReactCompositeComponent', () => { const container = document.createElement('div'); const comp = ReactDOM.render(, container); - expect(ReactDOM.findDOMNode(comp.refs.static0).textContent).toBe('A'); - expect(ReactDOM.findDOMNode(comp.refs.static1).textContent).toBe('B'); + expect(ReactDOM.findDOMNode(comp.static0Ref.current).textContent).toBe('A'); + expect(ReactDOM.findDOMNode(comp.static1Ref.current).textContent).toBe('B'); // When flipping the order, the refs should update even though the actual // contents do not ReactDOM.render(, container); - expect(ReactDOM.findDOMNode(comp.refs.static0).textContent).toBe('B'); - expect(ReactDOM.findDOMNode(comp.refs.static1).textContent).toBe('A'); + expect(ReactDOM.findDOMNode(comp.static0Ref.current).textContent).toBe('B'); + expect(ReactDOM.findDOMNode(comp.static1Ref.current).textContent).toBe('A'); }); it('should allow access to findDOMNode in componentWillUnmount', () => { @@ -1453,10 +1463,11 @@ describe('ReactCompositeComponent', () => { this.state = { color: 'green', }; + this.appleRef = React.createRef(); } render() { - return ; + return ; } } @@ -1502,15 +1513,15 @@ describe('ReactCompositeComponent', () => { expect(renderCalls).toBe(2); // Re-render base on state - instance.refs.apple.cut(); + instance.appleRef.current.cut(); expect(renderCalls).toBe(3); // No re-render based on state - instance.refs.apple.cut(); + instance.appleRef.current.cut(); expect(renderCalls).toBe(3); // Re-render based on state again - instance.refs.apple.eatSlice(); + instance.appleRef.current.eatSlice(); expect(renderCalls).toBe(4); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js index 282197f17c..87bfec0c17 100644 --- a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js @@ -193,12 +193,13 @@ describe('ReactDOMEventListener', () => { const onMouseOut = event => mouseOut(event.target); class Wrapper extends React.Component { + innerRef = React.createRef(); getInner = () => { - return this.refs.inner; + return this.innerRef.current; }; render() { - const inner =
Inner
; + const inner =
Inner
; return (
diff --git a/packages/react-dom/src/__tests__/ReactDOMInput-test.js b/packages/react-dom/src/__tests__/ReactDOMInput-test.js index 7e1bc146d7..c7a167391f 100644 --- a/packages/react-dom/src/__tests__/ReactDOMInput-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMInput-test.js @@ -1071,22 +1071,31 @@ describe('ReactDOMInput', () => { it('should control radio buttons', () => { class RadioGroup extends React.Component { + aRef = React.createRef(); + bRef = React.createRef(); + cRef = React.createRef(); + render() { return (
A - + B
{ } const stub = ReactDOM.render(, container); - const aNode = stub.refs.a; - const bNode = stub.refs.b; - const cNode = stub.refs.c; + const aNode = stub.aRef.current; + const bNode = stub.bRef.current; + const cNode = stub.cRef.current; expect(aNode.checked).toBe(true); expect(bNode.checked).toBe(false); diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js index 014d741d99..42ba08e2a5 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js @@ -337,7 +337,7 @@ describe('ReactDOMServerIntegration', () => { itRenders('no ref attribute', async render => { class RefComponent extends React.Component { render() { - return
; + return
; } } const e = await render(); diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js index 2e67110c28..041dc9ebe7 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js @@ -14,6 +14,7 @@ const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegratio let React; let ReactDOM; let ReactDOMServer; +let ReactFeatureFlags; let ReactTestUtils; function initModules() { @@ -22,6 +23,7 @@ function initModules() { React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); ReactTestUtils = require('react-dom/test-utils'); // Make them available to the helpers. @@ -91,10 +93,22 @@ describe('ReactDOMServerIntegration', () => { root.innerHTML = markup; let component = null; resetModules(); - await asyncReactDOMRender( - (component = e)} />, - root, - true, + await expect(async () => { + await asyncReactDOMRender( + (component = e)} />, + root, + true, + ); + }).toErrorDev( + ReactFeatureFlags.warnAboutStringRefs + ? [ + 'Warning: Component "RefsComponent" contains the string ref "myDiv". ' + + 'Support for string refs will be removed in a future major release. ' + + 'We recommend using useRef() or createRef() instead. ' + + 'Learn more about using refs safely here: https://reactjs.org/link/strict-mode-string-ref\n' + + ' in RefsComponent (at **)', + ] + : [], ); expect(component.refs.myDiv).toBe(root.firstChild); }); diff --git a/packages/react-dom/src/__tests__/ReactIdentity-test.js b/packages/react-dom/src/__tests__/ReactIdentity-test.js index 6f13f5812b..06337ccf16 100644 --- a/packages/react-dom/src/__tests__/ReactIdentity-test.js +++ b/packages/react-dom/src/__tests__/ReactIdentity-test.js @@ -67,17 +67,18 @@ describe('ReactIdentity', () => { function renderAComponentWithKeyIntoContainer(key, container) { class Wrapper extends React.Component { + spanRef = React.createRef(); render() { return (
- +
); } } const instance = ReactDOM.render(, container); - const span = instance.refs.span; + const span = instance.spanRef.current; expect(span).not.toBe(null); } diff --git a/packages/react-dom/src/__tests__/ReactMultiChildReconcile-test.js b/packages/react-dom/src/__tests__/ReactMultiChildReconcile-test.js index c14b4b0b58..1943281eac 100644 --- a/packages/react-dom/src/__tests__/ReactMultiChildReconcile-test.js +++ b/packages/react-dom/src/__tests__/ReactMultiChildReconcile-test.js @@ -60,6 +60,8 @@ class StatusDisplay extends React.Component { * Displays friends statuses. */ class FriendsStatusDisplay extends React.Component { + displays = {}; + /** * Gets the order directly from each rendered child's `index` field. * Refs are not maintained in the rendered order, and neither is @@ -84,7 +86,7 @@ class FriendsStatusDisplay extends React.Component { const originalKeys = this.getOriginalKeys(); for (let i = 0; i < originalKeys.length; i++) { const key = originalKeys[i]; - res[key] = this.refs[key]; + res[key] = this.displays[key]; } return res; } @@ -104,7 +106,7 @@ class FriendsStatusDisplay extends React.Component { // We are only interested in children up to the current key. return; } - expect(this.refs[key]).toBeTruthy(); + expect(this.displays[key]).toBeTruthy(); } } @@ -116,7 +118,9 @@ class FriendsStatusDisplay extends React.Component { !status ? null : ( { + this.displays[key] = current; + }} contentKey={key} onFlush={this.verifyPreviousRefsResolved.bind(this, key)} status={status} diff --git a/packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js b/packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js index 8f7d48d791..029c5cc6e2 100644 --- a/packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js +++ b/packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js @@ -36,6 +36,8 @@ describe('ReactDOMServerHydration', () => { let numClicks = 0; class TestComponent extends React.Component { + spanRef = React.createRef(); + componentDidMount() { mountCount++; } @@ -46,7 +48,7 @@ describe('ReactDOMServerHydration', () => { render() { return ( - + Name: {this.props.name} ); @@ -89,7 +91,7 @@ describe('ReactDOMServerHydration', () => { // Ensure the events system works after mount into server markup expect(numClicks).toEqual(0); - instance.refs.span.click(); + instance.spanRef.current.click(); expect(numClicks).toEqual(1); ReactDOM.unmountComponentAtNode(element); @@ -107,7 +109,7 @@ describe('ReactDOMServerHydration', () => { // Ensure the events system works after markup mismatch. expect(numClicks).toEqual(1); - instance.refs.span.click(); + instance.spanRef.current.click(); expect(numClicks).toEqual(2); } finally { document.body.removeChild(element); diff --git a/packages/react-dom/src/__tests__/ReactTestUtils-test.js b/packages/react-dom/src/__tests__/ReactTestUtils-test.js index 9f3cff86c1..cba5783886 100644 --- a/packages/react-dom/src/__tests__/ReactTestUtils-test.js +++ b/packages/react-dom/src/__tests__/ReactTestUtils-test.js @@ -221,13 +221,17 @@ describe('ReactTestUtils', () => { // Full-page components (html, head, body) can't be rendered into a div // directly... class Root extends React.Component { + htmlRef = React.createRef(); + headRef = React.createRef(); + bodyRef = React.createRef(); + render() { return ( - - + + hello - hello, world + hello, world ); } @@ -237,12 +241,12 @@ describe('ReactTestUtils', () => { const testDocument = getTestDocument(markup); const component = ReactDOM.hydrate(, testDocument); - expect(component.refs.html.tagName).toBe('HTML'); - expect(component.refs.head.tagName).toBe('HEAD'); - expect(component.refs.body.tagName).toBe('BODY'); - expect(ReactTestUtils.isDOMComponent(component.refs.html)).toBe(true); - expect(ReactTestUtils.isDOMComponent(component.refs.head)).toBe(true); - expect(ReactTestUtils.isDOMComponent(component.refs.body)).toBe(true); + expect(component.htmlRef.current.tagName).toBe('HTML'); + expect(component.headRef.current.tagName).toBe('HEAD'); + expect(component.bodyRef.current.tagName).toBe('BODY'); + expect(ReactTestUtils.isDOMComponent(component.htmlRef.current)).toBe(true); + expect(ReactTestUtils.isDOMComponent(component.headRef.current)).toBe(true); + expect(ReactTestUtils.isDOMComponent(component.bodyRef.current)).toBe(true); }); it('can scry with stateless components involved', () => { @@ -348,12 +352,13 @@ describe('ReactTestUtils', () => { it('should change the value of an input field in a component', () => { class SomeComponent extends React.Component { + inputRef = React.createRef(); render() { return (
@@ -373,7 +378,7 @@ describe('ReactTestUtils', () => { container, ); - const node = instance.refs.input; + const node = instance.inputRef.current; node.value = 'zebra'; ReactTestUtils.Simulate.change(node); diff --git a/packages/react-dom/src/__tests__/ReactUpdates-test.js b/packages/react-dom/src/__tests__/ReactUpdates-test.js index ec90ac5d29..7ac8438691 100644 --- a/packages/react-dom/src/__tests__/ReactUpdates-test.js +++ b/packages/react-dom/src/__tests__/ReactUpdates-test.js @@ -147,6 +147,7 @@ describe('ReactUpdates', () => { class Parent extends React.Component { state = {x: 0}; + childRef = React.createRef(); componentDidUpdate() { parentUpdateCount++; @@ -155,7 +156,7 @@ describe('ReactUpdates', () => { render() { return (
- +
); } @@ -176,7 +177,7 @@ describe('ReactUpdates', () => { } const instance = ReactTestUtils.renderIntoDocument(); - const child = instance.refs.child; + const child = instance.childRef.current; expect(instance.state.x).toBe(0); expect(child.state.y).toBe(0); @@ -200,6 +201,7 @@ describe('ReactUpdates', () => { class Parent extends React.Component { state = {x: 0}; + childRef = React.createRef(); componentDidUpdate() { parentUpdateCount++; @@ -208,7 +210,7 @@ describe('ReactUpdates', () => { render() { return (
- +
); } @@ -229,7 +231,7 @@ describe('ReactUpdates', () => { } const instance = ReactTestUtils.renderIntoDocument(); - const child = instance.refs.child; + const child = instance.childRef.current; expect(instance.state.x).toBe(0); expect(child.state.y).toBe(0); @@ -336,13 +338,15 @@ describe('ReactUpdates', () => { let childRenderCount = 0; class Parent extends React.Component { + childRef = React.createRef(); + shouldComponentUpdate() { return false; } render() { parentRenderCount++; - return ; + return ; } } @@ -370,7 +374,7 @@ describe('ReactUpdates', () => { expect(childRenderCount).toBe(1); ReactDOM.unstable_batchedUpdates(function() { - instance.refs.child.setState({x: 1}); + instance.childRef.current.setState({x: 1}); }); expect(parentRenderCount).toBe(1); @@ -428,28 +432,34 @@ describe('ReactUpdates', () => { }; class Box extends React.Component { + boxDivRef = React.createRef(); + render() { - return
{this.props.children}
; + return
{this.props.children}
; } } Object.assign(Box.prototype, UpdateLoggingMixin); class Child extends React.Component { + spanRef = React.createRef(); + render() { - return child; + return child; } } Object.assign(Child.prototype, UpdateLoggingMixin); class Switcher extends React.Component { state = {tabKey: 'hello'}; + boxRef = React.createRef(); + switcherDivRef = React.createRef(); render() { const child = this.props.children; return ( - +
@@ -462,10 +472,13 @@ describe('ReactUpdates', () => { Object.assign(Switcher.prototype, UpdateLoggingMixin); class App extends React.Component { + switcherRef = React.createRef(); + childRef = React.createRef(); + render() { return ( - - + + ); } @@ -513,21 +526,21 @@ describe('ReactUpdates', () => { expectUpdates(desiredWillUpdates, desiredDidUpdates); } testUpdates( - [root.refs.switcher.refs.box, root.refs.switcher], + [root.switcherRef.current.boxRef.current, root.switcherRef.current], // Owner-child relationships have inverse will and did ['Switcher', 'Box'], ['Box', 'Switcher'], ); testUpdates( - [root.refs.child, root.refs.switcher.refs.box], + [root.childRef.current, root.switcherRef.current.boxRef.current], // Not owner-child so reconcile independently ['Box', 'Child'], ['Box', 'Child'], ); testUpdates( - [root.refs.child, root.refs.switcher], + [root.childRef.current, root.switcherRef.current], // Switcher owns Box and Child, Box does not own Child ['Switcher', 'Box', 'Child'], ['Box', 'Switcher', 'Child'], @@ -588,12 +601,13 @@ describe('ReactUpdates', () => { class Outer extends React.Component { state = {x: 0}; + innerRef = React.createRef(); render() { updates.push('Outer-render-' + this.state.x); return (
- +
); } @@ -602,7 +616,7 @@ describe('ReactUpdates', () => { const x = this.state.x; updates.push('Outer-didUpdate-' + x); updates.push('Inner-setState-' + x); - this.refs.inner.setState({x: x}, function() { + this.innerRef.current.setState({x: x}, function() { updates.push('Inner-callback-' + x); }); } @@ -945,12 +959,14 @@ describe('ReactUpdates', () => { it('does not update one component twice in a batch (#2410)', () => { class Parent extends React.Component { + childRef = React.createRef(); + getChild = () => { - return this.refs.child; + return this.childRef.current; }; render() { - return ; + return ; } } diff --git a/packages/react-dom/src/__tests__/refs-destruction-test.js b/packages/react-dom/src/__tests__/refs-destruction-test.js index 2b3f60aea9..aaa1fe1ab7 100644 --- a/packages/react-dom/src/__tests__/refs-destruction-test.js +++ b/packages/react-dom/src/__tests__/refs-destruction-test.js @@ -30,6 +30,9 @@ describe('refs-destruction', () => { } TestComponent = class extends React.Component { + theInnerDivRef = React.createRef(); + theInnerClassComponentRef = React.createRef(); + render() { if (this.props.destroy) { return
; @@ -43,8 +46,8 @@ describe('refs-destruction', () => { } else { return (
-
- +
+
); } @@ -55,52 +58,45 @@ describe('refs-destruction', () => { it('should remove refs when destroying the parent', () => { const container = document.createElement('div'); const testInstance = ReactDOM.render(, container); - expect(ReactTestUtils.isDOMComponent(testInstance.refs.theInnerDiv)).toBe( - true, - ); + expect( - Object.keys(testInstance.refs || {}).filter(key => testInstance.refs[key]) - .length, - ).toEqual(2); + ReactTestUtils.isDOMComponent(testInstance.theInnerDivRef.current), + ).toBe(true); + expect(testInstance.theInnerClassComponentRef.current).toBeTruthy(); + ReactDOM.unmountComponentAtNode(container); - expect( - Object.keys(testInstance.refs || {}).filter(key => testInstance.refs[key]) - .length, - ).toEqual(0); + + expect(testInstance.theInnerDivRef.current).toBe(null); + expect(testInstance.theInnerClassComponentRef.current).toBe(null); }); it('should remove refs when destroying the child', () => { const container = document.createElement('div'); const testInstance = ReactDOM.render(, container); - expect(ReactTestUtils.isDOMComponent(testInstance.refs.theInnerDiv)).toBe( - true, - ); expect( - Object.keys(testInstance.refs || {}).filter(key => testInstance.refs[key]) - .length, - ).toEqual(2); + ReactTestUtils.isDOMComponent(testInstance.theInnerDivRef.current), + ).toBe(true); + expect(testInstance.theInnerClassComponentRef.current).toBeTruthy(); + ReactDOM.render(, container); - expect( - Object.keys(testInstance.refs || {}).filter(key => testInstance.refs[key]) - .length, - ).toEqual(0); + + expect(testInstance.theInnerDivRef.current).toBe(null); + expect(testInstance.theInnerClassComponentRef.current).toBe(null); }); it('should remove refs when removing the child ref attribute', () => { const container = document.createElement('div'); const testInstance = ReactDOM.render(, container); - expect(ReactTestUtils.isDOMComponent(testInstance.refs.theInnerDiv)).toBe( - true, - ); + expect( - Object.keys(testInstance.refs || {}).filter(key => testInstance.refs[key]) - .length, - ).toEqual(2); + ReactTestUtils.isDOMComponent(testInstance.theInnerDivRef.current), + ).toBe(true); + expect(testInstance.theInnerClassComponentRef.current).toBeTruthy(); + ReactDOM.render(, container); - expect( - Object.keys(testInstance.refs || {}).filter(key => testInstance.refs[key]) - .length, - ).toEqual(0); + + expect(testInstance.theInnerDivRef.current).toBe(null); + expect(testInstance.theInnerClassComponentRef.current).toBe(null); }); it('should not error when destroying child with ref asynchronously', () => { @@ -135,7 +131,7 @@ describe('refs-destruction', () => { render() { return ( - + ); } diff --git a/packages/react-dom/src/__tests__/refs-test.js b/packages/react-dom/src/__tests__/refs-test.js index 14c8261d02..9d1965c1a4 100644 --- a/packages/react-dom/src/__tests__/refs-test.js +++ b/packages/react-dom/src/__tests__/refs-test.js @@ -11,86 +11,12 @@ let React = require('react'); let ReactDOM = require('react-dom'); +let ReactFeatureFlags = require('shared/ReactFeatureFlags'); let ReactTestUtils = require('react-dom/test-utils'); -/** - * Counts clicks and has a renders an item for each click. Each item rendered - * has a ref of the form "clickLogN". - */ -class ClickCounter extends React.Component { - state = {count: this.props.initialCount}; - - triggerReset = () => { - this.setState({count: this.props.initialCount}); - }; - - handleClick = () => { - this.setState({count: this.state.count + 1}); - }; - - render() { - const children = []; - let i; - for (i = 0; i < this.state.count; i++) { - children.push( -
, - ); - } - return ( - - {children} - - ); - } -} - -/** - * Only purpose is to test that refs are tracked even when applied to a - * component that is injected down several layers. Ref systems are difficult to - * build in such a way that ownership is maintained in an airtight manner. - */ -class GeneralContainerComponent extends React.Component { - render() { - return
{this.props.children}
; - } -} - -/** - * Notice how refs ownership is maintained even when injecting a component - * into a different parent. - */ -class TestRefsComponent extends React.Component { - doReset = () => { - this.refs.myCounter.triggerReset(); - }; - - render() { - return ( -
-
- Reset Me By Clicking This. -
- - - -
- ); - } -} - -const expectClickLogsLengthToBe = function(instance, length) { - const clickLogs = ReactTestUtils.scryRenderedDOMComponentsWithClass( - instance, - 'clickLogDiv', - ); - expect(clickLogs.length).toBe(length); - expect(Object.keys(instance.refs.myCounter.refs).length).toBe(length); -}; - +// This is testing if string refs are deleted from `instance.refs` +// Once support for string refs is removed, this test can be removed. +// Detaching is already tested in refs-detruction-test.js describe('reactiverefs', () => { let container; @@ -98,6 +24,7 @@ describe('reactiverefs', () => { jest.resetModules(); React = require('react'); ReactDOM = require('react-dom'); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); ReactTestUtils = require('react-dom/test-utils'); }); @@ -108,13 +35,117 @@ describe('reactiverefs', () => { } }); + /** + * Counts clicks and has a renders an item for each click. Each item rendered + * has a ref of the form "clickLogN". + */ + class ClickCounter extends React.Component { + state = {count: this.props.initialCount}; + + triggerReset = () => { + this.setState({count: this.props.initialCount}); + }; + + handleClick = () => { + this.setState({count: this.state.count + 1}); + }; + + render() { + const children = []; + let i; + for (i = 0; i < this.state.count; i++) { + children.push( +
, + ); + } + return ( + + {children} + + ); + } + } + + const expectClickLogsLengthToBe = function(instance, length) { + const clickLogs = ReactTestUtils.scryRenderedDOMComponentsWithClass( + instance, + 'clickLogDiv', + ); + expect(clickLogs.length).toBe(length); + expect(Object.keys(instance.refs.myCounter.refs).length).toBe(length); + }; + /** * Render a TestRefsComponent and ensure that the main refs are wired up. */ const renderTestRefsComponent = function() { + /** + * Only purpose is to test that refs are tracked even when applied to a + * component that is injected down several layers. Ref systems are difficult to + * build in such a way that ownership is maintained in an airtight manner. + */ + class GeneralContainerComponent extends React.Component { + render() { + return
{this.props.children}
; + } + } + + /** + * Notice how refs ownership is maintained even when injecting a component + * into a different parent. + */ + class TestRefsComponent extends React.Component { + doReset = () => { + this.refs.myCounter.triggerReset(); + }; + + render() { + return ( +
+
+ Reset Me By Clicking This. +
+ + + +
+ ); + } + } + container = document.createElement('div'); document.body.appendChild(container); - const testRefsComponent = ReactDOM.render(, container); + + let testRefsComponent; + expect(() => { + testRefsComponent = ReactDOM.render(, container); + }).toErrorDev( + ReactFeatureFlags.warnAboutStringRefs + ? [ + 'Warning: Component "div" contains the string ref "resetDiv". ' + + 'Support for string refs will be removed in a future major release. ' + + 'We recommend using useRef() or createRef() instead. ' + + 'Learn more about using refs safely here: https://reactjs.org/link/strict-mode-string-ref\n' + + ' in div (at **)\n' + + ' in TestRefsComponent (at **)', + 'Warning: Component "span" contains the string ref "clickLog0". ' + + 'Support for string refs will be removed in a future major release. ' + + 'We recommend using useRef() or createRef() instead. ' + + 'Learn more about using refs safely here: https://reactjs.org/link/strict-mode-string-ref\n' + + ' in span (at **)\n' + + ' in ClickCounter (at **)\n' + + ' in div (at **)\n' + + ' in GeneralContainerComponent (at **)\n' + + ' in div (at **)\n' + + ' in TestRefsComponent (at **)', + ] + : [], + ); + expect(testRefsComponent instanceof TestRefsComponent).toBe(true); const generalContainer = testRefsComponent.refs.myContainer; @@ -156,13 +187,14 @@ describe('reactiverefs', () => { }); }); -if (!require('shared/ReactFeatureFlags').disableModulePatternComponents) { +if (!ReactFeatureFlags.disableModulePatternComponents) { describe('factory components', () => { it('Should correctly get the ref', () => { function Comp() { return { + elemRef: React.createRef(), render() { - return
; + return
; }, }; } @@ -177,7 +209,7 @@ if (!require('shared/ReactFeatureFlags').disableModulePatternComponents) { '`Comp.prototype = React.Component.prototype`. ' + "Don't use an arrow function since it cannot be called with `new` by React.", ); - expect(inst.refs.elemRef.tagName).toBe('DIV'); + expect(inst.elemRef.current.tagName).toBe('DIV'); }); }); } @@ -191,10 +223,15 @@ describe('ref swapping', () => { jest.resetModules(); React = require('react'); ReactDOM = require('react-dom'); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); ReactTestUtils = require('react-dom/test-utils'); RefHopsAround = class extends React.Component { state = {count: 0}; + hopRef = React.createRef(); + divOneRef = React.createRef(); + divTwoRef = React.createRef(); + divThreeRef = React.createRef(); moveRef = () => { this.setState({count: this.state.count + 1}); @@ -212,15 +249,15 @@ describe('ref swapping', () => {
); @@ -244,28 +281,28 @@ describe('ref swapping', () => { 'third', ); - expect(refHopsAround.refs.hopRef).toEqual(firstDiv); - expect(refHopsAround.refs.divTwoRef).toEqual(secondDiv); - expect(refHopsAround.refs.divThreeRef).toEqual(thirdDiv); + expect(refHopsAround.hopRef.current).toEqual(firstDiv); + expect(refHopsAround.divTwoRef.current).toEqual(secondDiv); + expect(refHopsAround.divThreeRef.current).toEqual(thirdDiv); refHopsAround.moveRef(); - expect(refHopsAround.refs.divOneRef).toEqual(firstDiv); - expect(refHopsAround.refs.hopRef).toEqual(secondDiv); - expect(refHopsAround.refs.divThreeRef).toEqual(thirdDiv); + expect(refHopsAround.divOneRef.current).toEqual(firstDiv); + expect(refHopsAround.hopRef.current).toEqual(secondDiv); + expect(refHopsAround.divThreeRef.current).toEqual(thirdDiv); refHopsAround.moveRef(); - expect(refHopsAround.refs.divOneRef).toEqual(firstDiv); - expect(refHopsAround.refs.divTwoRef).toEqual(secondDiv); - expect(refHopsAround.refs.hopRef).toEqual(thirdDiv); + expect(refHopsAround.divOneRef.current).toEqual(firstDiv); + expect(refHopsAround.divTwoRef.current).toEqual(secondDiv); + expect(refHopsAround.hopRef.current).toEqual(thirdDiv); /** * Make sure that after the third, we're back to where we started and the * refs are completely restored. */ refHopsAround.moveRef(); - expect(refHopsAround.refs.hopRef).toEqual(firstDiv); - expect(refHopsAround.refs.divTwoRef).toEqual(secondDiv); - expect(refHopsAround.refs.divThreeRef).toEqual(thirdDiv); + expect(refHopsAround.hopRef.current).toEqual(firstDiv); + expect(refHopsAround.divTwoRef.current).toEqual(secondDiv); + expect(refHopsAround.divThreeRef.current).toEqual(thirdDiv); }); it('always has a value for this.refs', () => { @@ -309,7 +346,20 @@ describe('ref swapping', () => { return
; } } - const a = ReactTestUtils.renderIntoDocument(); + let a; + expect(() => { + a = ReactTestUtils.renderIntoDocument(); + }).toErrorDev( + ReactFeatureFlags.warnAboutStringRefs + ? [ + 'Warning: Component "A" contains the string ref "1". ' + + 'Support for string refs will be removed in a future major release. ' + + 'We recommend using useRef() or createRef() instead. ' + + 'Learn more about using refs safely here: https://reactjs.org/link/strict-mode-string-ref\n' + + ' in A (at **)', + ] + : [], + ); expect(a.refs[1].nodeName).toBe('DIV'); }); @@ -464,7 +514,7 @@ describe('root level refs', () => { }); }); -describe('creating element with ref in constructor', () => { +describe('creating element with string ref in constructor', () => { class RefTest extends React.Component { constructor(props) { super(props); @@ -521,13 +571,41 @@ describe('strings refs across renderers', () => { const div1 = document.createElement('div'); const div2 = document.createElement('div'); - const inst = ReactDOM.render(, div1); + + let inst; + expect(() => { + inst = ReactDOM.render(, div1); + }).toErrorDev( + ReactFeatureFlags.warnAboutStringRefs + ? [ + 'Warning: Component "Indirection" contains the string ref "child1". ' + + 'Support for string refs will be removed in a future major release. ' + + 'We recommend using useRef() or createRef() instead. ' + + 'Learn more about using refs safely here: https://reactjs.org/link/strict-mode-string-ref\n' + + ' in Indirection (at **)\n' + + ' in Parent (at **)', + ] + : [], + ); + // Only the first ref has rendered yet. expect(inst.refs.child1.tagName).toBe('DIV'); expect(inst.refs.child1).toBe(div1.firstChild); - // Now both refs should be rendered. - ReactDOM.render(, div1); + expect(() => { + // Now both refs should be rendered. + ReactDOM.render(, div1); + }).toErrorDev( + ReactFeatureFlags.warnAboutStringRefs + ? [ + 'Warning: Component "Root" contains the string ref "child2". ' + + 'Support for string refs will be removed in a future major release. ' + + 'We recommend using useRef() or createRef() instead. ' + + 'Learn more about using refs safely here: https://reactjs.org/link/strict-mode-string-ref', + ] + : [], + {withoutStack: true}, + ); expect(inst.refs.child1.tagName).toBe('DIV'); expect(inst.refs.child1).toBe(div1.firstChild); expect(inst.refs.child2.tagName).toBe('DIV'); diff --git a/packages/react-native-renderer/src/__tests__/ResponderEventPlugin-test.internal.js b/packages/react-native-renderer/src/__tests__/ResponderEventPlugin-test.internal.js index 58e269aee1..a4bd4aa088 100644 --- a/packages/react-native-renderer/src/__tests__/ResponderEventPlugin-test.internal.js +++ b/packages/react-native-renderer/src/__tests__/ResponderEventPlugin-test.internal.js @@ -1385,25 +1385,36 @@ describe('ResponderEventPlugin', () => { const ReactDOMComponentTree = require('react-dom-bindings/src/client/ReactDOMComponentTree'); class ChildComponent extends React.Component { + divRef = React.createRef(); + div1Ref = React.createRef(); + div2Ref = React.createRef(); + render() { return ( -
-
-
+
+
+
); } } class ParentComponent extends React.Component { + pRef = React.createRef(); + p_P1Ref = React.createRef(); + p_P1Ref = React.createRef(); + p_P1_C1Ref = React.createRef(); + p_P1_C2Ref = React.createRef(); + p_OneOffRef = React.createRef(); + render() { return ( -
-
- - +
+
+ +
-
+
); } @@ -1414,41 +1425,45 @@ describe('ResponderEventPlugin', () => { const ancestors = [ // Common ancestor with self is self. { - one: parent.refs.P_P1_C1.refs.DIV_1, - two: parent.refs.P_P1_C1.refs.DIV_1, - com: parent.refs.P_P1_C1.refs.DIV_1, + one: parent.p_P1_C1Ref.current.div1Ref.current, + two: parent.p_P1_C1Ref.current.div1Ref.current, + com: parent.p_P1_C1Ref.current.div1Ref.current, }, // Common ancestor with self is self - even if topmost DOM. - {one: parent.refs.P, two: parent.refs.P, com: parent.refs.P}, + { + one: parent.pRef.current, + two: parent.pRef.current, + com: parent.pRef.current, + }, // Siblings { - one: parent.refs.P_P1_C1.refs.DIV_1, - two: parent.refs.P_P1_C1.refs.DIV_2, - com: parent.refs.P_P1_C1.refs.DIV, + one: parent.p_P1_C1Ref.current.div1Ref.current, + two: parent.p_P1_C1Ref.current.div2Ref.current, + com: parent.p_P1_C1Ref.current.divRef.current, }, // Common ancestor with parent is the parent. { - one: parent.refs.P_P1_C1.refs.DIV_1, - two: parent.refs.P_P1_C1.refs.DIV, - com: parent.refs.P_P1_C1.refs.DIV, + one: parent.p_P1_C1Ref.current.div1Ref.current, + two: parent.p_P1_C1Ref.current.divRef.current, + com: parent.p_P1_C1Ref.current.divRef.current, }, // Common ancestor with grandparent is the grandparent. { - one: parent.refs.P_P1_C1.refs.DIV_1, - two: parent.refs.P_P1, - com: parent.refs.P_P1, + one: parent.p_P1_C1Ref.current.div1Ref.current, + two: parent.p_P1Ref.current, + com: parent.p_P1Ref.current, }, // Grandparent across subcomponent boundaries. { - one: parent.refs.P_P1_C1.refs.DIV_1, - two: parent.refs.P_P1_C2.refs.DIV_1, - com: parent.refs.P_P1, + one: parent.p_P1_C1Ref.current.div1Ref.current, + two: parent.p_P1_C2Ref.current.div1Ref.current, + com: parent.p_P1Ref.current, }, // Something deep with something one-off. { - one: parent.refs.P_P1_C1.refs.DIV_1, - two: parent.refs.P_OneOff, - com: parent.refs.P, + one: parent.p_P1_C1Ref.current.div1Ref.current, + two: parent.p_OneOffRef.current, + com: parent.pRef.current, }, ]; let i; diff --git a/packages/react-reconciler/src/ReactChildFiber.new.js b/packages/react-reconciler/src/ReactChildFiber.new.js index 6a984baec0..ea72a3de92 100644 --- a/packages/react-reconciler/src/ReactChildFiber.new.js +++ b/packages/react-reconciler/src/ReactChildFiber.new.js @@ -97,6 +97,10 @@ if (__DEV__) { }; } +function isReactClass(type) { + return type.prototype && type.prototype.isReactComponent; +} + function coerceRef( returnFiber: Fiber, current: Fiber | null, @@ -120,7 +124,16 @@ function coerceRef( element._owner && element._self && element._owner.stateNode !== element._self - ) + ) && + // Will already throw with "Function components cannot have string refs" + !( + element._owner && + ((element._owner: any): Fiber).tag !== ClassComponent + ) && + // Will already warn with "Function components cannot be given refs" + !(typeof element.type === 'function' && !isReactClass(element.type)) && + // Will already throw with "Element ref was specified as a string (someStringRef) but no owner was set" + element._owner ) { const componentName = getComponentNameFromFiber(returnFiber) || 'Component'; diff --git a/packages/react-reconciler/src/ReactChildFiber.old.js b/packages/react-reconciler/src/ReactChildFiber.old.js index 8b6d4b8130..9ced326c55 100644 --- a/packages/react-reconciler/src/ReactChildFiber.old.js +++ b/packages/react-reconciler/src/ReactChildFiber.old.js @@ -97,6 +97,10 @@ if (__DEV__) { }; } +function isReactClass(type) { + return type.prototype && type.prototype.isReactComponent; +} + function coerceRef( returnFiber: Fiber, current: Fiber | null, @@ -120,7 +124,16 @@ function coerceRef( element._owner && element._self && element._owner.stateNode !== element._self - ) + ) && + // Will already throw with "Function components cannot have string refs" + !( + element._owner && + ((element._owner: any): Fiber).tag !== ClassComponent + ) && + // Will already warn with "Function components cannot be given refs" + !(typeof element.type === 'function' && !isReactClass(element.type)) && + // Will already throw with "Element ref was specified as a string (someStringRef) but no owner was set" + element._owner ) { const componentName = getComponentNameFromFiber(returnFiber) || 'Component'; diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.js index 635e426cd7..6bac75ab30 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.js @@ -11,6 +11,7 @@ 'use strict'; let React; +let ReactFeatureFlags; let ReactNoop; let Scheduler; @@ -19,6 +20,7 @@ describe('ReactIncrementalSideEffects', () => { jest.resetModules(); React = require('react'); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); ReactNoop = require('react-noop-renderer'); Scheduler = require('scheduler'); }); @@ -1306,8 +1308,19 @@ describe('ReactIncrementalSideEffects', () => { } ReactNoop.render(); - expect(Scheduler).toFlushWithoutYielding(); - + expect(() => { + expect(Scheduler).toFlushWithoutYielding(); + }).toErrorDev( + ReactFeatureFlags.warnAboutStringRefs + ? [ + 'Warning: Component "Foo" contains the string ref "bar". ' + + 'Support for string refs will be removed in a future major release. ' + + 'We recommend using useRef() or createRef() instead. ' + + 'Learn more about using refs safely here: https://reactjs.org/link/strict-mode-string-ref\n' + + ' in Foo (at **)', + ] + : [], + ); expect(fooInstance.refs.bar.test).toEqual('test'); }); }); diff --git a/packages/react-test-renderer/src/__tests__/ReactTestRenderer-test.internal.js b/packages/react-test-renderer/src/__tests__/ReactTestRenderer-test.internal.js index d5bc5e4454..a2c458f6aa 100644 --- a/packages/react-test-renderer/src/__tests__/ReactTestRenderer-test.internal.js +++ b/packages/react-test-renderer/src/__tests__/ReactTestRenderer-test.internal.js @@ -271,13 +271,15 @@ describe('ReactTestRenderer', () => { return
Hello, world
; } class Foo extends React.Component { + fooRef = React.createRef(); render() { - return ; + return ; } } class Baz extends React.Component { + bazRef = React.createRef(); render() { - return
; + return
; } } ReactTestRenderer.create(); @@ -298,11 +300,12 @@ describe('ReactTestRenderer', () => { const mockAnchorInstance = {hover: () => {}}; const log = []; class Foo extends React.Component { + barRef = React.createRef(); componentDidMount() { - log.push(this.refs.bar); + log.push(this.barRef.current); } render() { - return Hello, world; + return Hello, world; } } function createNodeMock(element) { @@ -355,7 +358,7 @@ describe('ReactTestRenderer', () => { it('supports unmounting when using refs', () => { class Foo extends React.Component { render() { - return
; + return
; } } const inst = ReactTestRenderer.create(, { @@ -394,7 +397,11 @@ describe('ReactTestRenderer', () => { }; class Foo extends React.Component { render() { - return this.props.useDiv ?
: ; + return this.props.useDiv ? ( +
+ ) : ( + + ); } } const inst = ReactTestRenderer.create(, { diff --git a/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee b/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee index c6edb3c09e..c9dee4ac6c 100644 --- a/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee +++ b/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee @@ -9,6 +9,7 @@ PropTypes = null React = null ReactDOM = null ReactDOMClient = null +ReactFeatureFlags = null act = null describe 'ReactCoffeeScriptClass', -> @@ -22,6 +23,7 @@ describe 'ReactCoffeeScriptClass', -> React = require 'react' ReactDOM = require 'react-dom' ReactDOMClient = require 'react-dom/client' + ReactFeatureFlags = require 'shared/ReactFeatureFlags' act = require('jest-react').act PropTypes = require 'prop-types' container = document.createElement 'div' @@ -528,7 +530,7 @@ describe 'ReactCoffeeScriptClass', -> test React.createElement(Foo), 'DIV', 'bar-through-context' - it 'supports classic refs', -> + it 'supports string refs', -> class Foo extends React.Component render: -> React.createElement(InnerComponent, @@ -537,7 +539,19 @@ describe 'ReactCoffeeScriptClass', -> ) ref = React.createRef() - test(React.createElement(Foo, ref: ref), 'DIV', 'foo') + expect(-> + test(React.createElement(Foo, ref: ref), 'DIV', 'foo') + ).toErrorDev( + if ReactFeatureFlags.warnAboutStringRefs + then [ + 'Warning: Component "Foo" contains the string ref "inner". ' + + 'Support for string refs will be removed in a future major release. ' + + 'We recommend using useRef() or createRef() instead. ' + + 'Learn more about using refs safely here: https://reactjs.org/link/strict-mode-string-ref\n' + + ' in Foo (at **)' + ] + else [] + ); expect(ref.current.refs.inner.getName()).toBe 'foo' it 'supports drilling through to the DOM using findDOMNode', -> diff --git a/packages/react/src/__tests__/ReactContextValidator-test.js b/packages/react/src/__tests__/ReactContextValidator-test.js index 2a21ba18df..e4895d5c15 100644 --- a/packages/react/src/__tests__/ReactContextValidator-test.js +++ b/packages/react/src/__tests__/ReactContextValidator-test.js @@ -46,6 +46,8 @@ describe('ReactContextValidator', () => { }; class ComponentInFooBarContext extends React.Component { + childRef = React.createRef(); + getChildContext() { return { foo: 'abc', @@ -54,7 +56,7 @@ describe('ReactContextValidator', () => { } render() { - return ; + return ; } } ComponentInFooBarContext.childContextTypes = { @@ -65,7 +67,7 @@ describe('ReactContextValidator', () => { const instance = ReactTestUtils.renderIntoDocument( , ); - expect(instance.refs.child.context).toEqual({foo: 'abc'}); + expect(instance.childRef.current.context).toEqual({foo: 'abc'}); }); it('should pass next context to lifecycles', () => { diff --git a/packages/react/src/__tests__/ReactES6Class-test.js b/packages/react/src/__tests__/ReactES6Class-test.js index f316db8896..53a44f9fb1 100644 --- a/packages/react/src/__tests__/ReactES6Class-test.js +++ b/packages/react/src/__tests__/ReactES6Class-test.js @@ -13,6 +13,7 @@ let PropTypes; let React; let ReactDOM; let ReactDOMClient; +let ReactFeatureFlags; let act; describe('ReactES6Class', () => { @@ -31,6 +32,7 @@ describe('ReactES6Class', () => { React = require('react'); ReactDOM = require('react-dom'); ReactDOMClient = require('react-dom/client'); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); act = require('jest-react').act; container = document.createElement('div'); root = ReactDOMClient.createRoot(container); @@ -568,14 +570,26 @@ describe('ReactES6Class', () => { test(, 'DIV', 'bar-through-context'); }); - it('supports classic refs', () => { + it('supports string refs', () => { class Foo extends React.Component { render() { return ; } } const ref = React.createRef(); - test(, 'DIV', 'foo'); + expect(() => { + test(, 'DIV', 'foo'); + }).toErrorDev( + ReactFeatureFlags.warnAboutStringRefs + ? [ + 'Warning: Component "Foo" contains the string ref "inner". ' + + 'Support for string refs will be removed in a future major release. ' + + 'We recommend using useRef() or createRef() instead. ' + + 'Learn more about using refs safely here: https://reactjs.org/link/strict-mode-string-ref\n' + + ' in Foo (at **)', + ] + : [], + ); expect(ref.current.refs.inner.getName()).toBe('foo'); }); diff --git a/packages/react/src/__tests__/ReactElement-test.js b/packages/react/src/__tests__/ReactElement-test.js index 245ba507e1..f1e9cffb84 100644 --- a/packages/react/src/__tests__/ReactElement-test.js +++ b/packages/react/src/__tests__/ReactElement-test.js @@ -93,7 +93,7 @@ describe('ReactElement', () => { render() { return (
- +
); } diff --git a/packages/react/src/__tests__/ReactElementClone-test.js b/packages/react/src/__tests__/ReactElementClone-test.js index 279ab84476..1aecc91359 100644 --- a/packages/react/src/__tests__/ReactElementClone-test.js +++ b/packages/react/src/__tests__/ReactElementClone-test.js @@ -82,8 +82,10 @@ describe('ReactElementClone', () => { it('should keep the original ref if it is not overridden', () => { class Grandparent extends React.Component { + yoloRef = React.createRef(); + render() { - return } />; + return } />; } } @@ -96,7 +98,7 @@ describe('ReactElementClone', () => { } const component = ReactTestUtils.renderIntoDocument(); - expect(component.refs.yolo.tagName).toBe('DIV'); + expect(component.yoloRef.current.tagName).toBe('DIV'); }); it('should transfer the key property', () => { @@ -174,21 +176,25 @@ describe('ReactElementClone', () => { it('should support keys and refs', () => { class Parent extends React.Component { + xyzRef = React.createRef(); + render() { const clone = React.cloneElement(this.props.children, { key: 'xyz', - ref: 'xyz', + ref: this.xyzRef, }); expect(clone.key).toBe('xyz'); - expect(clone.ref).toBe('xyz'); + expect(clone.ref).toBe(this.xyzRef); return
{clone}
; } } class Grandparent extends React.Component { + parentRef = React.createRef(); + render() { return ( - + ); @@ -196,30 +202,37 @@ describe('ReactElementClone', () => { } const component = ReactTestUtils.renderIntoDocument(); - expect(component.refs.parent.refs.xyz.tagName).toBe('SPAN'); + expect(component.parentRef.current.xyzRef.current.tagName).toBe('SPAN'); }); it('should steal the ref if a new ref is specified', () => { class Parent extends React.Component { + xyzRef = React.createRef(); + render() { - const clone = React.cloneElement(this.props.children, {ref: 'xyz'}); + const clone = React.cloneElement(this.props.children, { + ref: this.xyzRef, + }); return
{clone}
; } } class Grandparent extends React.Component { + parentRef = React.createRef(); + childRef = React.createRef(); + render() { return ( - - + + ); } } const component = ReactTestUtils.renderIntoDocument(); - expect(component.refs.child).toBeUndefined(); - expect(component.refs.parent.refs.xyz.tagName).toBe('SPAN'); + expect(component.childRef).toEqual({current: null}); + expect(component.parentRef.current.xyzRef.current.tagName).toBe('SPAN'); }); it('should overwrite props', () => { diff --git a/packages/react/src/__tests__/ReactElementJSX-test.js b/packages/react/src/__tests__/ReactElementJSX-test.js index 854618bf18..ad42046087 100644 --- a/packages/react/src/__tests__/ReactElementJSX-test.js +++ b/packages/react/src/__tests__/ReactElementJSX-test.js @@ -221,7 +221,7 @@ describe('ReactElement.jsx', () => { class Parent extends React.Component { render() { return JSXRuntime.jsx('div', { - children: JSXRuntime.jsx(Child, {ref: 'childElement'}), + children: JSXRuntime.jsx(Child, {ref: React.createRef()}), }); } } diff --git a/packages/react/src/__tests__/ReactJSXElement-test.js b/packages/react/src/__tests__/ReactJSXElement-test.js index 7a65555c34..7567e2b58f 100644 --- a/packages/react/src/__tests__/ReactJSXElement-test.js +++ b/packages/react/src/__tests__/ReactJSXElement-test.js @@ -78,10 +78,11 @@ describe('ReactJSXElement', () => { }); it('extracts key and ref from the rest of the props', () => { - const element = ; + const ref = React.createRef(); + const element = ; expect(element.type).toBe(Component); expect(element.key).toBe('12'); - expect(element.ref).toBe('34'); + expect(element.ref).toBe(ref); const expectation = {foo: '56'}; Object.freeze(expectation); expect(element.props).toEqual(expectation); diff --git a/packages/react/src/__tests__/ReactStrictMode-test.js b/packages/react/src/__tests__/ReactStrictMode-test.js index a266af4575..b41497ae3a 100644 --- a/packages/react/src/__tests__/ReactStrictMode-test.js +++ b/packages/react/src/__tests__/ReactStrictMode-test.js @@ -926,12 +926,18 @@ describe('string refs', () => { expect(() => { ReactDOM.render(, container); }).toErrorDev( - 'Warning: A string ref, "somestring", has been found within a strict mode tree. ' + - 'String refs are a source of potential bugs and should be avoided. ' + - 'We recommend using useRef() or createRef() instead. ' + - 'Learn more about using refs safely here: ' + - 'https://reactjs.org/link/strict-mode-string-ref\n' + - ' in OuterComponent (at **)', + ReactFeatureFlags.warnAboutStringRefs + ? 'Warning: Component "StrictMode" contains the string ref "somestring". ' + + 'Support for string refs will be removed in a future major release. ' + + 'We recommend using useRef() or createRef() instead. ' + + 'Learn more about using refs safely here: https://reactjs.org/link/strict-mode-string-ref\n' + + ' in OuterComponent (at **)' + : 'Warning: A string ref, "somestring", has been found within a strict mode tree. ' + + 'String refs are a source of potential bugs and should be avoided. ' + + 'We recommend using useRef() or createRef() instead. ' + + 'Learn more about using refs safely here: ' + + 'https://reactjs.org/link/strict-mode-string-ref\n' + + ' in OuterComponent (at **)', ); // Dedup @@ -967,13 +973,20 @@ describe('string refs', () => { expect(() => { ReactDOM.render(, container); }).toErrorDev( - 'Warning: A string ref, "somestring", has been found within a strict mode tree. ' + - 'String refs are a source of potential bugs and should be avoided. ' + - 'We recommend using useRef() or createRef() instead. ' + - 'Learn more about using refs safely here: ' + - 'https://reactjs.org/link/strict-mode-string-ref\n' + - ' in InnerComponent (at **)\n' + - ' in OuterComponent (at **)', + ReactFeatureFlags.warnAboutStringRefs + ? 'Warning: Component "InnerComponent" contains the string ref "somestring". ' + + 'Support for string refs will be removed in a future major release. ' + + 'We recommend using useRef() or createRef() instead. ' + + 'Learn more about using refs safely here: https://reactjs.org/link/strict-mode-string-ref\n' + + ' in InnerComponent (at **)\n' + + ' in OuterComponent (at **)' + : 'Warning: A string ref, "somestring", has been found within a strict mode tree. ' + + 'String refs are a source of potential bugs and should be avoided. ' + + 'We recommend using useRef() or createRef() instead. ' + + 'Learn more about using refs safely here: ' + + 'https://reactjs.org/link/strict-mode-string-ref\n' + + ' in InnerComponent (at **)\n' + + ' in OuterComponent (at **)', ); // Dedup diff --git a/packages/react/src/__tests__/ReactTypeScriptClass-test.ts b/packages/react/src/__tests__/ReactTypeScriptClass-test.ts index ad621bbb40..39304e9d6f 100644 --- a/packages/react/src/__tests__/ReactTypeScriptClass-test.ts +++ b/packages/react/src/__tests__/ReactTypeScriptClass-test.ts @@ -17,6 +17,7 @@ import ReactDOMClient = require('react-dom/client'); import ReactDOMTestUtils = require('react-dom/test-utils'); import PropTypes = require('prop-types'); import internalAct = require('jest-react'); +import ReactFeatureFlags = require('shared/ReactFeatureFlags') // Before Each @@ -686,9 +687,21 @@ describe('ReactTypeScriptClass', function() { test(React.createElement(ProvideContext), 'DIV', 'bar-through-context'); }); - it('supports classic refs', function() { + it('supports string refs', function() { const ref = React.createRef(); - test(React.createElement(ClassicRefs, {ref: ref}), 'DIV', 'foo'); + expect(() => { + test(React.createElement(ClassicRefs, {ref: ref}), 'DIV', 'foo'); + }).toErrorDev( + ReactFeatureFlags.warnAboutStringRefs + ? [ + 'Warning: Component "ClassicRefs" contains the string ref "inner". ' + + 'Support for string refs will be removed in a future major release. ' + + 'We recommend using useRef() or createRef() instead. ' + + 'Learn more about using refs safely here: https://reactjs.org/link/strict-mode-string-ref\n' + + ' in ClassicRefs (at **)', + ] + : [], + ); expect(ref.current.refs.inner.getName()).toBe('foo'); }); diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 9008562472..3a52dfd100 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -219,7 +219,7 @@ export const warnAboutDefaultPropsOnFunctionComponents = false; // deprecate lat // a deprecated pattern we want to get rid of in the future export const warnAboutSpreadingKeyToJSX = true; -export const warnAboutStringRefs = false; +export const warnAboutStringRefs = true; // ----------------------------------------------------------------------------- // Debugging and DevTools diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 1777de0519..f14d33de63 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -42,7 +42,7 @@ export const enableScopeAPI = false; export const enableCreateEventHandleAPI = false; export const enableSuspenseCallback = false; export const warnAboutDefaultPropsOnFunctionComponents = false; -export const warnAboutStringRefs = false; +export const warnAboutStringRefs = true; export const disableLegacyContext = false; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 28052ac785..296d4012ab 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -32,7 +32,7 @@ export const enableScopeAPI = false; export const enableCreateEventHandleAPI = false; export const enableSuspenseCallback = false; export const warnAboutDefaultPropsOnFunctionComponents = false; -export const warnAboutStringRefs = false; +export const warnAboutStringRefs = true; export const disableLegacyContext = false; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 88a6513c0b..86f0d54eea 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -32,7 +32,7 @@ export const enableScopeAPI = false; export const enableCreateEventHandleAPI = false; export const enableSuspenseCallback = false; export const warnAboutDefaultPropsOnFunctionComponents = false; -export const warnAboutStringRefs = false; +export const warnAboutStringRefs = true; export const disableLegacyContext = false; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index dcbd65ad71..f734afa272 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -32,7 +32,7 @@ export const enableScopeAPI = false; export const enableCreateEventHandleAPI = false; export const enableSuspenseCallback = false; export const warnAboutDefaultPropsOnFunctionComponents = false; -export const warnAboutStringRefs = false; +export const warnAboutStringRefs = true; export const disableLegacyContext = false; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index e6e94b66ec..7ec00cbb2e 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -32,7 +32,7 @@ export const enableScopeAPI = true; export const enableCreateEventHandleAPI = false; export const enableSuspenseCallback = true; export const warnAboutDefaultPropsOnFunctionComponents = false; -export const warnAboutStringRefs = false; +export const warnAboutStringRefs = true; export const disableLegacyContext = false; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js index 97c1a0f443..efe5ef51e2 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -32,7 +32,7 @@ export const enableScopeAPI = false; export const enableCreateEventHandleAPI = false; export const enableSuspenseCallback = false; export const warnAboutDefaultPropsOnFunctionComponents = false; -export const warnAboutStringRefs = false; +export const warnAboutStringRefs = true; export const disableLegacyContext = false; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js index e018c20d11..f9e6b26567 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -32,7 +32,7 @@ export const enableScopeAPI = true; export const enableCreateEventHandleAPI = true; export const enableSuspenseCallback = true; export const warnAboutDefaultPropsOnFunctionComponents = false; -export const warnAboutStringRefs = false; +export const warnAboutStringRefs = true; export const disableLegacyContext = __EXPERIMENTAL__; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 51ff1be193..b0516f7f49 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -66,7 +66,7 @@ export const enableSchedulingProfiler: boolean = export const enableSchedulerDebugging = true; export const warnAboutDeprecatedLifecycles = true; export const disableLegacyContext = __EXPERIMENTAL__; -export const warnAboutStringRefs = false; +export const warnAboutStringRefs = true; export const warnAboutDefaultPropsOnFunctionComponents = false; export const enableGetInspectorDataForInstanceInProduction = false;