Skip to content

Commit

Permalink
Remove enableRefAsProp feature flag
Browse files Browse the repository at this point in the history
  • Loading branch information
@kassens
kassens committed Jul 16, 2024
1 parent fee423e commit 9d1f3fb
Show file tree
Hide file tree
Showing 29 changed files with 105 additions and 750 deletions.
4 changes: 2 additions & 2 deletions packages/jest-react/src/JestReact.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import {REACT_ELEMENT_TYPE, REACT_FRAGMENT_TYPE} from 'shared/ReactSymbols';
import {disableStringRefs, enableRefAsProp} from 'shared/ReactFeatureFlags';
import {disableStringRefs} from 'shared/ReactFeatureFlags';
const {assertConsoleLogsCleared} = require('internal-test-utils/consoleMock');

import isArray from 'shared/isArray';
Expand Down Expand Up @@ -42,7 +42,7 @@ function assertYieldsWereCleared(root) {
}

function createJSXElementForTestComparison(type, props) {
if (__DEV__ && enableRefAsProp) {
if (__DEV__) {
const element = {
$$typeof: REACT_ELEMENT_TYPE,
type: type,
Expand Down
3 changes: 1 addition & 2 deletions packages/react-client/src/ReactFlightClient.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ import {
disableStringRefs,
enableBinaryFlight,
enablePostpone,
enableRefAsProp,
enableFlightReadableStream,
enableOwnerStacks,
} from 'shared/ReactFeatureFlags';
Expand Down Expand Up @@ -630,7 +629,7 @@ function createElement(
| React$Element<any>
| LazyComponent<React$Element<any>, SomeChunk<React$Element<any>>> {
let element: any;
if (__DEV__ && enableRefAsProp) {
if (__DEV__) {
// `ref` is non-enumerable in dev
element = ({
$$typeof: REACT_ELEMENT_TYPE,
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-shared/src/__tests__/legacy/storeLegacy-v15-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -873,7 +873,7 @@ describe('Store (legacy)', () => {
// is not compatible with older versions of React. Need to configure the
// the test environment in such a way that certain test modules like this
// one can use an older transform.
if (!require('shared/ReactFeatureFlags').enableRefAsProp) {
if (false) {
it('should support expanding deep parts of the tree', () => {
const Wrapper = ({forwardedRef}) =>
React.createElement(Nested, {
Expand Down
217 changes: 0 additions & 217 deletions packages/react-dom/src/__tests__/ReactFunctionComponent-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,223 +197,6 @@ describe('ReactFunctionComponent', () => {
.rejects.toThrowError();
});

// @gate !enableRefAsProp || !__DEV__
it('should warn when given a string ref', async () => {
function Indirection(props) {
return <div>{props.children}</div>;
}

class ParentUsingStringRef extends React.Component {
render() {
return (
<Indirection>
<FunctionComponent name="A" ref="stateless" />
</Indirection>
);
}
}

await expect(async () => {
const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);
await act(() => {
root.render(<ParentUsingStringRef />);
});
}).toErrorDev(
'Function components cannot be given refs. ' +
'Attempts to access this ref will fail. ' +
'Did you mean to use React.forwardRef()?\n\n' +
'Check the render method ' +
'of `ParentUsingStringRef`.\n' +
' in FunctionComponent (at **)\n' +
' in div (at **)\n' +
' in Indirection (at **)\n' +
' in ParentUsingStringRef (at **)',
);

// No additional warnings should be logged
const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);
await act(() => {
root.render(<ParentUsingStringRef />);
});
});

// @gate !enableRefAsProp || !__DEV__
it('should warn when given a function ref', async () => {
function Indirection(props) {
return <div>{props.children}</div>;
}

const ref = jest.fn();
class ParentUsingFunctionRef extends React.Component {
render() {
return (
<Indirection>
<FunctionComponent name="A" ref={ref} />
</Indirection>
);
}
}

await expect(async () => {
const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);
await act(() => {
root.render(<ParentUsingFunctionRef />);
});
}).toErrorDev(
'Function components cannot be given refs. ' +
'Attempts to access this ref will fail. ' +
'Did you mean to use React.forwardRef()?\n\n' +
'Check the render method ' +
'of `ParentUsingFunctionRef`.\n' +
' in FunctionComponent (at **)\n' +
' in div (at **)\n' +
' in Indirection (at **)\n' +
' in ParentUsingFunctionRef (at **)',
);
expect(ref).not.toHaveBeenCalled();

// No additional warnings should be logged
const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);
await act(() => {
root.render(<ParentUsingFunctionRef />);
});
});

// @gate !enableRefAsProp || !__DEV__
it('deduplicates ref warnings based on element or owner', async () => {
// When owner uses JSX, we can use exact line location to dedupe warnings
class AnonymousParentUsingJSX extends React.Component {
render() {
return <FunctionComponent name="A" ref={() => {}} />;
}
}

let instance1;

await expect(async () => {
const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);

await act(() => {
root.render(
<AnonymousParentUsingJSX ref={current => (instance1 = current)} />,
);
});
}).toErrorDev('Function components cannot be given refs.');
// Should be deduped (offending element is on the same line):
instance1.forceUpdate();
// Should also be deduped (offending element is on the same line):
let container = document.createElement('div');
let root = ReactDOMClient.createRoot(container);
await act(() => {
root.render(<AnonymousParentUsingJSX />);
});

// When owner doesn't use JSX, and is anonymous, we warn once per internal instance.
class AnonymousParentNotUsingJSX extends React.Component {
render() {
return React.createElement(FunctionComponent, {
name: 'A',
ref: () => {},
});
}
}

let instance2;
await expect(async () => {
container = document.createElement('div');
root = ReactDOMClient.createRoot(container);
await act(() => {
root.render(
<AnonymousParentNotUsingJSX ref={current => (instance2 = current)} />,
);
});
}).toErrorDev('Function components cannot be given refs.');
// Should be deduped (same internal instance, no additional warnings)
instance2.forceUpdate();
// Could not be differentiated (since owner is anonymous and no source location)
container = document.createElement('div');
root = ReactDOMClient.createRoot(container);
await act(() => {
root.render(<AnonymousParentNotUsingJSX />);
});

// When owner doesn't use JSX, but is named, we warn once per owner name
class NamedParentNotUsingJSX extends React.Component {
render() {
return React.createElement(FunctionComponent, {
name: 'A',
ref: () => {},
});
}
}
let instance3;
await expect(async () => {
container = document.createElement('div');
root = ReactDOMClient.createRoot(container);
await act(() => {
root.render(
<NamedParentNotUsingJSX ref={current => (instance3 = current)} />,
);
});
}).toErrorDev('Function components cannot be given refs.');
// Should be deduped (same owner name, no additional warnings):
instance3.forceUpdate();
// Should also be deduped (same owner name, no additional warnings):
container = document.createElement('div');
root = ReactDOMClient.createRoot(container);
await act(() => {
root.render(<NamedParentNotUsingJSX />);
});
});

// This guards against a regression caused by clearing the current debug fiber.
// https://github.com/facebook/react/issues/10831
// @gate !disableLegacyContext || !__DEV__
// @gate !enableRefAsProp || !__DEV__
it('should warn when giving a function ref with context', async () => {
function Child() {
return null;
}
Child.contextTypes = {
foo: PropTypes.string,
};

class Parent extends React.Component {
static childContextTypes = {
foo: PropTypes.string,
};
getChildContext() {
return {
foo: 'bar',
};
}
render() {
return <Child ref={function () {}} />;
}
}

await expect(async () => {
const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);
await act(() => {
root.render(<Parent />);
});
}).toErrorDev(
'Function components cannot be given refs. ' +
'Attempts to access this ref will fail. ' +
'Did you mean to use React.forwardRef()?\n\n' +
'Check the render method ' +
'of `Parent`.\n' +
' in Child (at **)\n' +
' in Parent (at **)',
);
});

it('should use correct name in key warning', async () => {
function Child() {
return <div>{[<span />]}</div>;
Expand Down
18 changes: 0 additions & 18 deletions packages/react-dom/src/__tests__/refs-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -369,24 +369,6 @@ describe('ref swapping', () => {
});
}).rejects.toThrow('Expected ref to be a function');
});

// @gate !enableRefAsProp && www
it('undefined ref on manually inlined React element triggers error', async () => {
const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);
await expect(async () => {
await act(() => {
root.render({
$$typeof: Symbol.for('react.element'),
type: 'div',
props: {
ref: undefined,
},
key: null,
});
});
}).rejects.toThrow('Expected ref to be a function');
});
});

describe('root level refs', () => {
Expand Down
8 changes: 2 additions & 6 deletions packages/react-noop-renderer/src/createReactNoop.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,7 @@ import {
ConcurrentRoot,
LegacyRoot,
} from 'react-reconciler/constants';
import {
enableRefAsProp,
disableLegacyMode,
disableStringRefs,
} from 'shared/ReactFeatureFlags';
import {disableLegacyMode, disableStringRefs} from 'shared/ReactFeatureFlags';

import ReactSharedInternals from 'shared/ReactSharedInternals';

Expand Down Expand Up @@ -816,7 +812,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
let currentEventPriority = DefaultEventPriority;

function createJSXElementForTestComparison(type, props) {
if (__DEV__ && enableRefAsProp) {
if (__DEV__) {
const element = {
type: type,
$$typeof: REACT_ELEMENT_TYPE,
Expand Down
36 changes: 6 additions & 30 deletions packages/react-reconciler/src/ReactChildFiber.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ import {
} from './ReactWorkTags';
import isArray from 'shared/isArray';
import {
enableRefAsProp,
enableAsyncIterableChildren,
disableLegacyMode,
enableOwnerStacks,
Expand Down Expand Up @@ -239,21 +238,6 @@ function validateFragmentProps(
break;
}
}

if (!enableRefAsProp && element.ref !== null) {
if (fiber === null) {
// For unkeyed root fragments there's no Fiber. We create a fake one just for
// error stack handling.
fiber = createFiberFromElement(element, returnFiber.mode, 0);
if (__DEV__) {
fiber._debugInfo = currentDebugInfo;
}
fiber.return = returnFiber;
}
runWithFiberInDEV(fiber, () => {
console.error('Invalid attribute `ref` supplied to `React.Fragment`.');
});
}
}
}

Expand All @@ -272,21 +256,13 @@ function coerceRef(
workInProgress: Fiber,
element: ReactElement,
): void {
let ref;
if (enableRefAsProp) {
// TODO: This is a temporary, intermediate step. When enableRefAsProp is on,
// we should resolve the `ref` prop during the begin phase of the component
// it's attached to (HostComponent, ClassComponent, etc).
const refProp = element.props.ref;
ref = refProp !== undefined ? refProp : null;
} else {
// Old behavior.
ref = element.ref;
}

// TODO: If enableRefAsProp is on, we shouldn't use the `ref` field. We
// TODO: This is a temporary, intermediate step. Now that enableRefAsProp is on,
// we should resolve the `ref` prop during the begin phase of the component
// it's attached to (HostComponent, ClassComponent, etc).
const refProp = element.props.ref;
// TODO: With enableRefAsProp now rolled out, we shouldn't use the `ref` field. We
// should always read the ref from the prop.
workInProgress.ref = ref;
workInProgress.ref = refProp !== undefined ? refProp : null;
}

function throwOnInvalidObjectType(returnFiber: Fiber, newChild: Object) {
Expand Down
Loading

0 comments on commit 9d1f3fb

Please sign in to comment.