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 usage of ReactTestUtils from ReactFunctionComponent #28331

Merged
merged 2 commits into from
Feb 20, 2024
Merged

Remove usage of ReactTestUtils from ReactFunctionComponent #28331

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5b3cf78
Remove usage of ReactTestUtils from ReactFunctionComponent
Feb 14, 2024
7ebf75d
Test that we don't call ref
Feb 14, 2024
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
204 changes: 139 additions & 65 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 @@ -12,7 +12,6 @@
let PropTypes;
let React;
let ReactDOMClient;
let ReactTestUtils;
let act;

function FunctionComponent(props) {
Expand All @@ -26,7 +25,6 @@ describe('ReactFunctionComponent', () => {
React = require('react');
ReactDOMClient = require('react-dom/client');
act = require('internal-test-utils').act;
ReactTestUtils = require('react-dom/test-utils');
});

it('should render stateless component', async () => {
Expand Down Expand Up @@ -161,25 +159,33 @@ describe('ReactFunctionComponent', () => {
);
});

it('should not throw when stateless component returns undefined', () => {
it('should not throw when stateless component returns undefined', async () => {
function NotAComponent() {}
expect(function () {
ReactTestUtils.renderIntoDocument(
<div>
<NotAComponent />
</div>,
);
}).not.toThrowError();
const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);
await expect(
act(() => {
root.render(
<div>
<NotAComponent />
</div>,
);
}),
).resolves.not.toThrowError();
});

it('should throw on string refs in pure functions', () => {
it('should throw on string refs in pure functions', async () => {
function Child() {
return <div ref="me" />;
}

expect(function () {
ReactTestUtils.renderIntoDocument(<Child test="test" />);
}).toThrowError(
const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);
await expect(
act(() => {
root.render(<Child test="test" />);
}),
).rejects.toThrowError(
__DEV__
? 'Function components cannot have string refs. We recommend using useRef() instead.'
: // It happens because we don't save _owner in production for
Expand All @@ -193,7 +199,7 @@ describe('ReactFunctionComponent', () => {
);
});

it('should warn when given a string ref', () => {
it('should warn when given a string ref', async () => {
function Indirection(props) {
return <div>{props.children}</div>;
}
Expand All @@ -208,9 +214,13 @@ describe('ReactFunctionComponent', () => {
}
}

expect(() =>
ReactTestUtils.renderIntoDocument(<ParentUsingStringRef />),
).toErrorDev(
await expect(async () => {
const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);
await act(() => {
root.render(<ParentUsingStringRef />);
});
}).toErrorDev(
'Warning: Function components cannot be given refs. ' +
'Attempts to access this ref will fail. ' +
'Did you mean to use React.forwardRef()?\n\n' +
Expand All @@ -223,32 +233,36 @@ describe('ReactFunctionComponent', () => {
);

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

it('should warn when given a function ref', () => {
it('should warn when given a function ref and ignore them', 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={arg => {
expect(arg).toBe(null);
}}
/>
<FunctionComponent name="A" ref={ref} />
</Indirection>
);
}
}

expect(() =>
ReactTestUtils.renderIntoDocument(<ParentUsingFunctionRef />),
).toErrorDev(
await expect(async () => {
const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);
await act(() => {
root.render(<ParentUsingFunctionRef />);
});
}).toErrorDev(
'Warning: Function components cannot be given refs. ' +
'Attempts to access this ref will fail. ' +
'Did you mean to use React.forwardRef()?\n\n' +
Expand All @@ -259,12 +273,17 @@ describe('ReactFunctionComponent', () => {
' in Indirection (at **)\n' +
' in ParentUsingFunctionRef (at **)',
);
expect(ref).not.toHaveBeenCalled();

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

it('deduplicates ref warnings based on element or owner', () => {
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() {
Expand All @@ -274,15 +293,24 @@ describe('ReactFunctionComponent', () => {

let instance1;

expect(() => {
instance1 = ReactTestUtils.renderIntoDocument(
<AnonymousParentUsingJSX />,
);
await expect(async () => {
const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);

await act(() => {
root.render(
<AnonymousParentUsingJSX ref={current => (instance1 = current)} />,
);
});
}).toErrorDev('Warning: 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):
ReactTestUtils.renderIntoDocument(<AnonymousParentUsingJSX />);
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 {
Expand All @@ -295,15 +323,23 @@ describe('ReactFunctionComponent', () => {
}

let instance2;
expect(() => {
instance2 = ReactTestUtils.renderIntoDocument(
<AnonymousParentNotUsingJSX />,
);
await expect(async () => {
container = document.createElement('div');
root = ReactDOMClient.createRoot(container);
await act(() => {
root.render(
<AnonymousParentNotUsingJSX ref={current => (instance2 = current)} />,
);
});
}).toErrorDev('Warning: 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)
ReactTestUtils.renderIntoDocument(<AnonymousParentNotUsingJSX />);
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 {
Expand All @@ -315,19 +351,29 @@ describe('ReactFunctionComponent', () => {
}
}
let instance3;
expect(() => {
instance3 = ReactTestUtils.renderIntoDocument(<NamedParentNotUsingJSX />);
await expect(async () => {
container = document.createElement('div');
root = ReactDOMClient.createRoot(container);
await act(() => {
root.render(
<NamedParentNotUsingJSX ref={current => (instance3 = current)} />,
);
});
}).toErrorDev('Warning: 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):
ReactTestUtils.renderIntoDocument(<NamedParentNotUsingJSX />);
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__
it('should warn when giving a function ref with context', () => {
it('should warn when giving a function ref with context', async () => {
function Child() {
return null;
}
Expand All @@ -349,7 +395,13 @@ describe('ReactFunctionComponent', () => {
}
}

expect(() => ReactTestUtils.renderIntoDocument(<Parent />)).toErrorDev(
await expect(async () => {
const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);
await act(() => {
root.render(<Parent />);
});
}).toErrorDev(
'Warning: Function components cannot be given refs. ' +
'Attempts to access this ref will fail. ' +
'Did you mean to use React.forwardRef()?\n\n' +
Expand All @@ -360,36 +412,40 @@ describe('ReactFunctionComponent', () => {
);
});

it('should provide a null ref', () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was testing ReactTestUtils not function components. I opted to instead adjust the should warn when given a function ref which had an assertion that we never hit.

function Child() {
return <div />;
}

const comp = ReactTestUtils.renderIntoDocument(<Child />);
expect(comp).toBe(null);
});

it('should use correct name in key warning', () => {
it('should use correct name in key warning', async () => {
function Child() {
return <div>{[<span />]}</div>;
}

expect(() => ReactTestUtils.renderIntoDocument(<Child />)).toErrorDev(
await expect(async () => {
const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);
await act(() => {
root.render(<Child />);
});
}).toErrorDev(
'Each child in a list should have a unique "key" prop.\n\n' +
'Check the render method of `Child`.',
);
});

// TODO: change this test after we deprecate default props support
// for function components
it('should support default props and prop types', () => {
it('should support default props and prop types', async () => {
function Child(props) {
return <div>{props.test}</div>;
}
Child.defaultProps = {test: 2};
Child.propTypes = {test: PropTypes.string};

expect(() => ReactTestUtils.renderIntoDocument(<Child />)).toErrorDev([
await expect(async () => {
const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);

await act(() => {
root.render(<Child />);
});
}).toErrorDev([
'Warning: Child: Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead.',
'Warning: Failed prop type: Invalid prop `test` of type `number` ' +
'supplied to `Child`, expected `string`.\n' +
Expand Down Expand Up @@ -427,28 +483,46 @@ describe('ReactFunctionComponent', () => {
expect(el.textContent).toBe('en');
});

it('should work with arrow functions', () => {
it('should work with arrow functions', async () => {
let Child = function () {
return <div />;
};
// Will create a new bound function without a prototype, much like a native
// arrow function.
Child = Child.bind(this);

expect(() => ReactTestUtils.renderIntoDocument(<Child />)).not.toThrow();
await expect(async () => {
const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);
await act(() => {
root.render(<Child />);
});
}).not.toThrow();
});

it('should allow simple functions to return null', () => {
it('should allow simple functions to return null', async () => {
const Child = function () {
return null;
};
expect(() => ReactTestUtils.renderIntoDocument(<Child />)).not.toThrow();
await expect(async () => {
const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);
await act(() => {
root.render(<Child />);
});
}).not.toThrow();
});

it('should allow simple functions to return false', () => {
it('should allow simple functions to return false', async () => {
function Child() {
return false;
}
expect(() => ReactTestUtils.renderIntoDocument(<Child />)).not.toThrow();
const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);
await expect(
act(() => {
root.render(<Child />);
}),
).resolves.not.toThrow();
});
});