Skip to content

Commit

Permalink
Enable JSX and defaultProps deprecation warnings
Browse files Browse the repository at this point in the history
These will help users upgrade to React 19 by finding places where they
rely on deprecated behavior.

Backports these PRs:

- #25699
- #25383
  • Loading branch information
acdlite committed Apr 16, 2024
1 parent 2cfb474 commit a06a364
Show file tree
Hide file tree
Showing 44 changed files with 727 additions and 335 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -882,7 +882,11 @@ describe('ReactHooksInspectionIntegration', () => {

await LazyFoo;

Scheduler.unstable_flushAll();
expect(() => {
Scheduler.unstable_flushAll();
}).toErrorDev([
'Foo: Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead.',
]);

const childFiber = renderer.root._currentFiber();
const tree = ReactDebugTools.inspectHooksOfFiber(childFiber);
Expand Down
29 changes: 25 additions & 4 deletions packages/react-dom/src/__tests__/ReactComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
let React;
let ReactDOM;
let ReactDOMServer;
let ReactFeatureFlags;
let ReactTestUtils;

describe('ReactComponent', () => {
Expand All @@ -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');
});

Expand All @@ -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 = <div ref="badDiv" />;
expect(function() {
instance = ReactTestUtils.renderIntoDocument(instance);
Expand Down Expand Up @@ -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 = {};

Expand Down Expand Up @@ -133,10 +135,29 @@ describe('ReactComponent', () => {
}
}

ReactTestUtils.renderIntoDocument(<Component />);
expect(() => {
ReactTestUtils.renderIntoDocument(<Component />);
}).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 (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ describe('ReactComponentLifeCycle', () => {
}
// you would *NEVER* do anything like this in real code!
this.state.hasRenderCompleted = true;
return <div ref="theDiv">I am the inner DIV</div>;
return <div ref={React.createRef()}>I am the inner DIV</div>;
}

componentWillUnmount() {
Expand Down Expand Up @@ -477,7 +477,9 @@ describe('ReactComponentLifeCycle', () => {
class Component extends React.Component {
render() {
return (
<Tooltip ref="tooltip" tooltip={<div>{this.props.tooltipText}</div>}>
<Tooltip
ref={React.createRef()}
tooltip={<div>{this.props.tooltipText}</div>}>
{this.props.text}
</Tooltip>
);
Expand Down
63 changes: 37 additions & 26 deletions packages/react-dom/src/__tests__/ReactCompositeComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,16 +72,18 @@ describe('ReactCompositeComponent', () => {
MorphingComponent = class extends React.Component {
state = {activated: false};

xRef = React.createRef();

_toggleActivatedState = () => {
this.setState({activated: !this.state.activated});
};

render() {
const toggleActivatedState = this._toggleActivatedState;
return !this.state.activated ? (
<a ref="x" onClick={toggleActivatedState} />
<a ref={this.xRef} onClick={toggleActivatedState} />
) : (
<b ref="x" onClick={toggleActivatedState} />
<b ref={this.xRef} onClick={toggleActivatedState} />
);
}
};
Expand All @@ -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 ? (
<a ref="anch" className={className} />
<a ref={this.anchorRef} className={className} />
) : (
<b />
);
Expand Down Expand Up @@ -186,11 +190,11 @@ describe('ReactCompositeComponent', () => {
it('should rewire refs when rendering to different child types', () => {
const instance = ReactTestUtils.renderIntoDocument(<MorphingComponent />);

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', () => {
Expand Down Expand Up @@ -739,25 +743,28 @@ describe('ReactCompositeComponent', () => {
}

class Wrapper extends React.Component {
parentRef = React.createRef();
childRef = React.createRef();

render() {
return (
<Parent ref="parent">
<Child ref="child" />
<Parent ref={this.parentRef}>
<Child ref={this.childRef} />
</Parent>
);
}
}

const wrapper = ReactTestUtils.renderIntoDocument(<Wrapper />);

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 <Parent /> while <Child /> 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', () => {
Expand Down Expand Up @@ -1142,25 +1149,28 @@ describe('ReactCompositeComponent', () => {
}

class Component extends React.Component {
static0Ref = React.createRef();
static1Ref = React.createRef();

render() {
if (this.props.flipped) {
return (
<div>
<Static ref="static0" key="B">
<Static ref={this.static0Ref} key="B">
B (ignored)
</Static>
<Static ref="static1" key="A">
<Static ref={this.static1Ref} key="A">
A (ignored)
</Static>
</div>
);
} else {
return (
<div>
<Static ref="static0" key="A">
<Static ref={this.static0Ref} key="A">
A
</Static>
<Static ref="static1" key="B">
<Static ref={this.static1Ref} key="B">
B
</Static>
</div>
Expand All @@ -1171,14 +1181,14 @@ describe('ReactCompositeComponent', () => {

const container = document.createElement('div');
const comp = ReactDOM.render(<Component flipped={false} />, 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(<Component flipped={true} />, 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', () => {
Expand Down Expand Up @@ -1453,10 +1463,11 @@ describe('ReactCompositeComponent', () => {
this.state = {
color: 'green',
};
this.appleRef = React.createRef();
}

render() {
return <Apple color={this.state.color} ref="apple" />;
return <Apple color={this.state.color} ref={this.appleRef} />;
}
}

Expand Down Expand Up @@ -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);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = <div ref="inner">Inner</div>;
const inner = <div ref={this.innerRef}>Inner</div>;
return (
<div>
<div onMouseOut={onMouseOut} id="outer">
Expand Down
21 changes: 15 additions & 6 deletions packages/react-dom/src/__tests__/ReactDOMInput-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<div>
<input
ref="a"
ref={this.aRef}
type="radio"
name="fruit"
checked={true}
onChange={emptyFunction}
/>
A
<input ref="b" type="radio" name="fruit" onChange={emptyFunction} />
<input
ref={this.bRef}
type="radio"
name="fruit"
onChange={emptyFunction}
/>
B
<form>
<input
ref="c"
ref={this.cRef}
type="radio"
name="fruit"
defaultChecked={true}
Expand All @@ -1099,9 +1108,9 @@ describe('ReactDOMInput', () => {
}

const stub = ReactDOM.render(<RadioGroup />, 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ describe('ReactDOMServerIntegration', () => {
itRenders('no ref attribute', async render => {
class RefComponent extends React.Component {
render() {
return <div ref="foo" />;
return <div ref={React.createRef()} />;
}
}
const e = await render(<RefComponent />);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegratio
let React;
let ReactDOM;
let ReactDOMServer;
let ReactFeatureFlags;
let ReactTestUtils;

function initModules() {
Expand All @@ -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.
Expand Down Expand Up @@ -91,10 +93,22 @@ describe('ReactDOMServerIntegration', () => {
root.innerHTML = markup;
let component = null;
resetModules();
await asyncReactDOMRender(
<RefsComponent ref={e => (component = e)} />,
root,
true,
await expect(async () => {
await asyncReactDOMRender(
<RefsComponent ref={e => (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);
});
Expand Down
Loading

0 comments on commit a06a364

Please sign in to comment.