Skip to content

Commit

Permalink
Remove enableRefAsProp feature flag
Browse files Browse the repository at this point in the history 
**NOTE:** To be merged after React 19 is released.

The flag is fully rolled out.
  • Loading branch information
@kassens
kassens committed Nov 4, 2024
1 parent 543eb09 commit 75223d4
Show file tree
Hide file tree
Showing 29 changed files with 113 additions and 859 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 @@ -44,7 +44,6 @@ import {
disableStringRefs,
enableBinaryFlight,
enablePostpone,
enableRefAsProp,
enableFlightReadableStream,
enableOwnerStacks,
enableServerComponentLogs,
Expand Down Expand Up @@ -676,7 +675,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
83 changes: 0 additions & 83 deletions 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 @@ -868,89 +868,6 @@ describe('Store (legacy)', () => {
`);
});

// TODO: These tests don't work when enableRefAsProp is on because the
// JSX runtime that's injected into the test environment by the compiler
// 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) {
it('should support expanding deep parts of the tree', () => {
const Wrapper = ({forwardedRef}) =>
React.createElement(Nested, {
depth: 3,
forwardedRef: forwardedRef,
});
const Nested = ({depth, forwardedRef}) =>
depth > 0
? React.createElement(Nested, {
depth: depth - 1,
forwardedRef: forwardedRef,
})
: React.createElement('div', {
ref: forwardedRef,
});
let ref = null;
const refSetter = value => {
ref = value;
};
act(() =>
ReactDOM.render(
React.createElement(Wrapper, {
forwardedRef: refSetter,
}),
document.createElement('div'),
),
);
expect(store).toMatchInlineSnapshot(`
[root]
▸ <Wrapper>
`);
const deepestedNodeID = global.agent.getIDForHostInstance(ref);
act(() => store.toggleIsCollapsed(deepestedNodeID, false));
expect(store).toMatchInlineSnapshot(`
[root]
▾ <Wrapper>
▾ <Nested>
▾ <Nested>
▾ <Nested>
▾ <Nested>
<div>
`);
const rootID = store.getElementIDAtIndex(0);
act(() => store.toggleIsCollapsed(rootID, true));
expect(store).toMatchInlineSnapshot(`
[root]
▸ <Wrapper>
`);
act(() => store.toggleIsCollapsed(rootID, false));
expect(store).toMatchInlineSnapshot(`
[root]
▾ <Wrapper>
▾ <Nested>
▾ <Nested>
▾ <Nested>
▾ <Nested>
<div>
`);
const id = store.getElementIDAtIndex(1);
act(() => store.toggleIsCollapsed(id, true));
expect(store).toMatchInlineSnapshot(`
[root]
▾ <Wrapper>
▸ <Nested>
`);
act(() => store.toggleIsCollapsed(id, false));
expect(store).toMatchInlineSnapshot(`
[root]
▾ <Wrapper>
▾ <Nested>
▾ <Nested>
▾ <Nested>
▾ <Nested>
<div>
`);
});
}
it('should support reordering of children', () => {
const Root = ({children}) => React.createElement('div', null, children);
const Component = () => React.createElement('div', null);
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';
import ReactVersion from 'shared/ReactVersion';
Expand Down Expand Up @@ -833,7 +829,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
Loading

0 comments on commit 75223d4

Please sign in to comment.