Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove enableRefAsProp feature flag #30346

Merged
merged 1 commit into from
Nov 4, 2024
Merged

Remove enableRefAsProp feature flag #30346

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/jest-react/src/JestReact.js
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading