Skip to content

Commit

Permalink
Drop support for string refs
Browse files Browse the repository at this point in the history
  • Loading branch information
eps1lon committed Nov 17, 2022
1 parent 6fb8133 commit e0e03eb
Show file tree
Hide file tree
Showing 33 changed files with 145 additions and 1,047 deletions.
20 changes: 17 additions & 3 deletions packages/jest-react/src/JestReact.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,15 @@ export function unstable_toMatchRenderedOutput(root, expectedJSX) {
props: {
children: actualJSXChildren,
},
_owner: null,
_store: __DEV__ ? {} : undefined,
};

Object.defineProperty(actualJSX, '_owner', {
configurable: false,
enumerable: false,
writable: false,
value: null,
});
}
}
} else {
Expand All @@ -82,7 +88,7 @@ function jsonChildToJSXChild(jsonChild) {
return jsonChild;
} else {
const jsxChildren = jsonChildrenToJSXChildren(jsonChild.children);
return {
const element = {
$$typeof: REACT_ELEMENT_TYPE,
type: jsonChild.type,
key: null,
Expand All @@ -91,9 +97,17 @@ function jsonChildToJSXChild(jsonChild) {
jsxChildren === null
? jsonChild.props
: {...jsonChild.props, children: jsxChildren},
_owner: null,
_store: __DEV__ ? {} : undefined,
};

Object.defineProperty(element, '_owner', {
configurable: false,
enumerable: false,
writable: false,
value: null,
});

return element;
}
}

Expand Down
12 changes: 9 additions & 3 deletions packages/react-client/src/ReactFlightClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -379,15 +379,21 @@ function createElement(type, key, props): React$Element<any> {
key: key,
ref: null,
props: props,

// Record the component responsible for creating this element.
_owner: null,
};
if (__DEV__) {
// We don't really need to add any of these but keeping them for good measure.
// Unfortunately, _store is enumerable in jest matchers so for equality to
// work, I need to keep it or make _store non-enumerable in the other file.
element._store = {};

// Record the component responsible for creating this element.
Object.defineProperty(element, '_owner', {
configurable: false,
enumerable: false,
writable: false,
value: null,
});

Object.defineProperty(element._store, 'validated', {
configurable: false,
enumerable: false,
Expand Down
79 changes: 0 additions & 79 deletions packages/react-dom/src/__tests__/ReactComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
let React;
let ReactDOM;
let ReactDOMServer;
let ReactFeatureFlags;
let ReactTestUtils;

describe('ReactComponent', () => {
Expand All @@ -22,7 +21,6 @@ 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 Down Expand Up @@ -104,83 +102,6 @@ describe('ReactComponent', () => {
}
});

it('should support string refs on owned components', () => {
const innerObj = {};
const outerObj = {};

class Wrapper extends React.Component {
getObject = () => {
return this.props.object;
};

render() {
return <div>{this.props.children}</div>;
}
}

class Component extends React.Component {
render() {
const inner = <Wrapper object={innerObj} ref="inner" />;
const outer = (
<Wrapper object={outerObj} ref="outer">
{inner}
</Wrapper>
);
return outer;
}

componentDidMount() {
expect(this.refs.inner.getObject()).toEqual(innerObj);
expect(this.refs.outer.getObject()).toEqual(outerObj);
}
}

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 string refs on unmounted components', () => {
class Parent extends React.Component {
render() {
return (
<Child>
<div ref="test" />
</Child>
);
}

componentDidMount() {
expect(this.refs && this.refs.test).toEqual(undefined);
}
}

class Child extends React.Component {
render() {
return <div />;
}
}

ReactTestUtils.renderIntoDocument(<Parent child={<span />} />);
});

it('should support callback-style refs', () => {
const innerObj = {};
const outerObj = {};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegratio
let React;
let ReactDOM;
let ReactDOMServer;
let ReactFeatureFlags;
let ReactTestUtils;

function initModules() {
Expand All @@ -23,7 +22,6 @@ 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 All @@ -36,7 +34,6 @@ function initModules() {

const {
resetModules,
asyncReactDOMRender,
clientRenderOnServerString,
expectMarkupMatch,
} = ReactDOMServerIntegrationUtils(initModules);
Expand Down Expand Up @@ -80,38 +77,6 @@ describe('ReactDOMServerIntegration', () => {
expect(refElement).not.toBe(null);
expect(refElement).toBe(e);
});

it('should have string refs on client when rendered over server markup', async () => {
class RefsComponent extends React.Component {
render() {
return <div ref="myDiv" />;
}
}

const markup = ReactDOMServer.renderToString(<RefsComponent />);
const root = document.createElement('div');
root.innerHTML = markup;
let component = null;
resetModules();
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);
});
});

it('should forward refs', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ let React;
let ReactFeatureFlags;
let ReactNoop;
let Scheduler;
let JSXDEVRuntime;

describe('ReactDeprecationWarnings', () => {
beforeEach(() => {
Expand All @@ -22,16 +21,11 @@ describe('ReactDeprecationWarnings', () => {
ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactNoop = require('react-noop-renderer');
Scheduler = require('scheduler');
if (__DEV__) {
JSXDEVRuntime = require('react/jsx-dev-runtime');
}
ReactFeatureFlags.warnAboutDefaultPropsOnFunctionComponents = true;
ReactFeatureFlags.warnAboutStringRefs = true;
});

afterEach(() => {
ReactFeatureFlags.warnAboutDefaultPropsOnFunctionComponents = false;
ReactFeatureFlags.warnAboutStringRefs = false;
});

it('should warn when given defaultProps', () => {
Expand All @@ -50,99 +44,4 @@ describe('ReactDeprecationWarnings', () => {
'release. Use JavaScript default parameters instead.',
);
});

it('should warn when given string refs', () => {
class RefComponent extends React.Component {
render() {
return null;
}
}
class Component extends React.Component {
render() {
return <RefComponent ref="refComponent" />;
}
}

ReactNoop.render(<Component />);
expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev(
'Warning: Component "Component" contains the string ref "refComponent". ' +
'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 warn when owner and self are the same for string refs', () => {
ReactFeatureFlags.warnAboutStringRefs = false;

class RefComponent extends React.Component {
render() {
return null;
}
}
class Component extends React.Component {
render() {
return <RefComponent ref="refComponent" __self={this} />;
}
}
ReactNoop.renderLegacySyncRoot(<Component />);
expect(Scheduler).toFlushWithoutYielding();
});

it('should warn when owner and self are different for string refs', () => {
class RefComponent extends React.Component {
render() {
return null;
}
}
class Component extends React.Component {
render() {
return <RefComponent ref="refComponent" __self={{}} />;
}
}

ReactNoop.render(<Component />);
expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev([
'Warning: Component "Component" contains the string ref "refComponent". ' +
'Support for string refs will be removed in a future major release. ' +
'This case cannot be automatically converted to an arrow function. ' +
'We ask you to manually fix this case by using useRef() or createRef() instead. ' +
'Learn more about using refs safely here: ' +
'https://reactjs.org/link/strict-mode-string-ref',
]);
});

if (__DEV__) {
it('should warn when owner and self are different for string refs', () => {
class RefComponent extends React.Component {
render() {
return null;
}
}
class Component extends React.Component {
render() {
return JSXDEVRuntime.jsxDEV(
RefComponent,
{ref: 'refComponent'},
null,
false,
{},
{},
);
}
}

ReactNoop.render(<Component />);
expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev(
'Warning: Component "Component" contains the string ref "refComponent". ' +
'Support for string refs will be removed in a future major release. ' +
'This case cannot be automatically converted to an arrow function. ' +
'We ask you to manually fix this case by using useRef() or createRef() instead. ' +
'Learn more about using refs safely here: ' +
'https://reactjs.org/link/strict-mode-string-ref',
);
});
}
});
Loading

0 comments on commit e0e03eb

Please sign in to comment.