Skip to content

Commit

Permalink
[cleanup] remove feature flags warnAboutDefaultPropsOnFunctionCompone…
Browse files Browse the repository at this point in the history 
…nts and warnAboutStringRefs (#25980)

These feature flags are fully rolled out and easy to clean up. Let's
remove them!
  • Loading branch information
@kassens
kassens authored Jan 11, 2023
1 parent 7002a67 commit 0fce6bb
Show file tree
Hide file tree
Showing 20 changed files with 111 additions and 225 deletions.
34 changes: 14 additions & 20 deletions packages/react-dom/src/__tests__/ReactComponent-test.js
View file
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 @@ -137,24 +135,20 @@ describe('ReactComponent', () => {

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 **)',
]
: [],
);
}).toErrorDev([
'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', () => {
Expand Down
20 changes: 7 additions & 13 deletions packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js
View file
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 Down Expand Up @@ -99,17 +97,13 @@ describe('ReactDOMServerIntegration', () => {
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 **)',
]
: [],
);
}).toErrorDev([
'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
92 changes: 39 additions & 53 deletions packages/react-dom/src/__tests__/refs-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,28 +123,24 @@ describe('reactiverefs', () => {
let testRefsComponent;
expect(() => {
testRefsComponent = ReactDOM.render(<TestRefsComponent />, 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 **)',
]
: [],
);
}).toErrorDev([
'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);

Expand Down Expand Up @@ -349,17 +345,13 @@ describe('ref swapping', () => {
let a;
expect(() => {
a = ReactTestUtils.renderIntoDocument(<A />);
}).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 **)',
]
: [],
);
}).toErrorDev([
'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');
});

Expand Down Expand Up @@ -546,18 +538,14 @@ describe('strings refs across renderers', () => {
let inst;
expect(() => {
inst = ReactDOM.render(<Parent />, 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 **)',
]
: [],
);
}).toErrorDev([
'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');
Expand All @@ -567,14 +555,12 @@ describe('strings refs across renderers', () => {
// Now both refs should be rendered.
ReactDOM.render(<Parent />, 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',
]
: [],
[
'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');
Expand Down
34 changes: 9 additions & 25 deletions packages/react-reconciler/src/ReactChildFiber.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import {
} from 'shared/ReactSymbols';
import {ClassComponent, HostText, HostPortal, Fragment} from './ReactWorkTags';
import isArray from 'shared/isArray';
import {warnAboutStringRefs} from 'shared/ReactFeatureFlags';
import {checkPropStringCoercion} from 'shared/CheckStringCoercion';

import {
Expand All @@ -40,7 +39,6 @@ import {
createFiberFromPortal,
} from './ReactFiber';
import {isCompatibleFamilyForHotReloading} from './ReactFiberHotReloading';
import {StrictLegacyMode} from './ReactTypeOfMode';
import {getIsHydrating} from './ReactFiberHydrationContext';
import {pushTreeFork} from './ReactFiberTreeContext';

Expand Down Expand Up @@ -113,10 +111,7 @@ function coerceRef(
typeof mixedRef !== 'object'
) {
if (__DEV__) {
// TODO: Clean this up once we turn on the string ref warning for
// everyone, because the strict mode case will no longer be relevant
if (
(returnFiber.mode & StrictLegacyMode || warnAboutStringRefs) &&
// We warn in ReactElement.js if owner and self are equal for string refs
// because these cannot be automatically converted to an arrow function
// using a codemod. Therefore, we don't have to warn about string refs again.
Expand All @@ -138,26 +133,15 @@ function coerceRef(
const componentName =
getComponentNameFromFiber(returnFiber) || 'Component';
if (!didWarnAboutStringRefs[componentName]) {
if (warnAboutStringRefs) {
console.error(
'Component "%s" contains the string ref "%s". 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',
componentName,
mixedRef,
);
} else {
console.error(
'A string ref, "%s", 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',
mixedRef,
);
}
console.error(
'Component "%s" contains the string ref "%s". 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',
componentName,
mixedRef,
);
didWarnAboutStringRefs[componentName] = true;
}
}
Expand Down
11 changes: 2 additions & 9 deletions packages/react-reconciler/src/ReactFiberBeginWork.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ import {
disableModulePatternComponents,
enableProfilerCommitHooks,
enableProfilerTimer,
warnAboutDefaultPropsOnFunctionComponents,
enableScopeAPI,
enableCache,
enableLazyContextPropagation,
Expand Down Expand Up @@ -506,10 +505,7 @@ function updateMemoComponent(
getComponentNameFromType(type),
);
}
if (
warnAboutDefaultPropsOnFunctionComponents &&
Component.defaultProps !== undefined
) {
if (Component.defaultProps !== undefined) {
const componentName = getComponentNameFromType(type) || 'Unknown';
if (!didWarnAboutDefaultPropsOnFunctionComponent[componentName]) {
console.error(
Expand Down Expand Up @@ -2058,10 +2054,7 @@ function validateFunctionComponentInDev(workInProgress: Fiber, Component: any) {
}
}

if (
warnAboutDefaultPropsOnFunctionComponents &&
Component.defaultProps !== undefined
) {
if (Component.defaultProps !== undefined) {
const componentName = getComponentNameFromType(Component) || 'Unknown';

if (!didWarnAboutDefaultPropsOnFunctionComponent[componentName]) {
Expand Down
20 changes: 7 additions & 13 deletions packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
'use strict';

let React;
let ReactFeatureFlags;
let ReactNoop;
let Scheduler;

Expand All @@ -20,7 +19,6 @@ describe('ReactIncrementalSideEffects', () => {
jest.resetModules();

React = require('react');
ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactNoop = require('react-noop-renderer');
Scheduler = require('scheduler');
});
Expand Down Expand Up @@ -1310,17 +1308,13 @@ describe('ReactIncrementalSideEffects', () => {
ReactNoop.render(<Foo />);
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 **)',
]
: [],
);
}).toErrorDev([
'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');
});
});
6 changes: 1 addition & 5 deletions packages/react-server/src/ReactFizzServer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@ import ReactSharedInternals from 'shared/ReactSharedInternals';
import {
disableLegacyContext,
disableModulePatternComponents,
warnAboutDefaultPropsOnFunctionComponents,
enableScopeAPI,
enableSuspenseAvoidThisFallbackFizz,
enableFloat,
Expand Down Expand Up @@ -949,10 +948,7 @@ function validateFunctionComponentInDev(Component: any): void {
}
}

if (
warnAboutDefaultPropsOnFunctionComponents &&
Component.defaultProps !== undefined
) {
if (Component.defaultProps !== undefined) {
const componentName = getComponentNameFromType(Component) || 'Unknown';

if (!didWarnAboutDefaultPropsOnFunctionComponent[componentName]) {
Expand Down
Loading

0 comments on commit 0fce6bb

Please sign in to comment.