-
Notifications
You must be signed in to change notification settings - Fork 2.8k
fix(react): migrate tests from react-test-renderer to @testing-library/react and resolve React 19 issues #35314
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.
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
fix(react): migrate tests from react-test-renderer to @testing-library/react and resolve React 19 issues #35314
Conversation
📊 Bundle size reportUnchanged fixtures
|
Pull request demo site: URL |
0763312
to
3731080
Compare
3731080
to
9e1ff67
Compare
// if props changed (as opposed to a state update), set the value | ||
// TODO: switch to strict controlled pattern instead | ||
if (prevProps !== this.props && this.props.value !== undefined) { | ||
if (prevProps.value !== this.props.value && this.props.value !== undefined) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix inifite loop in React 19
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is great fix ! this most probably aslo affected users runtime with R17,18
const { container, getByLabelText } = render(<DatePickerBase />); | ||
const input = container.querySelectorAll('div')[5]; | ||
|
||
it('renders Calendar with provided props and custom formatter', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved to a separate file DatePicker.CalendarMock.test.tsx
to be able to mock the Calendar component to verify its props with RTL
public componentDidUpdate(prevProps: Readonly<IColorPickerProps>, prevState: Readonly<IColorPickerState>): void { | ||
// if props changed (as opposed to a state update), update the color | ||
if (prevProps !== this.props) { | ||
if (prevProps.color !== this.props.color) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix infinite loop in React 19
9e1ff67
to
5b58fb0
Compare
"@fluentui/example-data": "*", | ||
"@fluentui/jest-serializer-merge-styles": "*", | ||
"@fluentui/react-conformance": "*", | ||
"@fluentui/test-utilities": "*", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the package is not used anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
// if props changed (as opposed to a state update), set the value | ||
// TODO: switch to strict controlled pattern instead | ||
if (prevProps !== this.props && this.props.value !== undefined) { | ||
if (prevProps.value !== this.props.value && this.props.value !== undefined) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is great fix ! this most probably aslo affected users runtime with R17,18
Updated the type from 'none' to 'patch' to reflect the nature of the changes.
input.props.onClick(); | ||
}); | ||
// open the datepicker then dismiss | ||
await userEvent.click(input); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like hallucination. the version of user-event we use has not async methods.
- we are on v13
- v14 has all async https://github.com/testing-library/user-event/releases/tag/v14.0.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a few tests 458045f#diff-a7e2503f10a441adc194a14a0d703cca90431d7f8ad97951622408f0ce7cb0a2L217 had { delay: 100 }
which made the userEvent.type
async. So we need to use awaits for those, or remove the delay
I removed delay and awaits in 458045f
Previous Behavior
The
@fluentui/react
's tests previously usedreact-test-renderer
, which is now deprecated, making them unsuitable for React 19 integration testing.The main test errors encountered during React 19 integration were:
console.error] react-test-renderer is deprecated
Error: Maximum update depth exceeded. This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate. React limits the number of nested updates to prevent infinite loops. at getRootForUpdatedFiber
Can't access .root on unmounted test renderer
New Behavior
react-test-renderer
to@testing-library/react
, addressing the first and third issues.ColorPicker
component'scomponentDidUpdate
.@fluentui/react
package.Related Issue(s)