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.

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: (WIP) Support React 19 and remove Jest reliance in test utils #7686

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

LFDanLu
Copy link
Member

@LFDanLu LFDanLu commented Jan 30, 2025

Closes

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

Comment on lines +244 to +254
// else {
// // TODO: this is a bit unfortunate, but from a submenu point of view, we can't perform a check that waits for
// // focus to return to the original trigger since we don't have that information in the submenu trigger tester. Even if we did
// // it is a bit unpredicatable where focus might end up landing and waiting for the option/menu to disappear isn't sufficient if there are
// // more transitions in play. For now advance times by a certain amount of time and rely on the user to advance them even more if need be
// if (this._advanceTimer == null) {
// throw new Error('No advanceTimers provided for long press.');
// } else {
// await act(async () => await this._advanceTimer(1000));
// }
// }
Copy link
Member Author

Choose a reason for hiding this comment

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

Did some poking around trying to get rid of the last jest.runAllTimers() here but no luck. The above is ok, but can extend test duration by quite a bit, also seemed to have some problems with hanging that I didn't quite figure out

@LFDanLu
Copy link
Member Author

LFDanLu commented Jan 30, 2025

ugh lint blew up, not sure why

@@ -29,7 +29,7 @@ export interface UserOpts {
* A function used by the test utils to advance timers during interactions. Required for certain aria patterns (e.g. table). This can be overridden
* at the aria pattern tester level if needed.
*/
advanceTimer?: (time?: number) => void | Promise<unknown>
advanceTimer?: (time?: number) => void | Promise<unknown> | any
Copy link
Member Author

Choose a reason for hiding this comment

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

the typing will need to be looked at again for advanceTimers, providing jest.runAllTimers as is threw some TS issues depending on the configuration.

@@ -423,7 +423,7 @@ import {theme} from '@react-spectrum/theme-default';
import {User} from '@react-spectrum/test-utils';

let testUtilUser = new User({interactionType: 'mouse'});
// ...
// Other setup, be sure to check out the suggested mocks mentioned above in https://react-spectrum.adobe.com/react-spectrum/ListBox.html#testing
Copy link
Member Author

Choose a reason for hiding this comment

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

Hopefully makes it a little more clear that additional setup might be needed for RSP components in general

Comment on lines +28 to +29
"@testing-library/jest-dom": "^6.0.0",
"@testing-library/react": "^16.0.0",
Copy link
Member Author

Choose a reason for hiding this comment

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

Doesn't seem to have any breaking changes with this bump and the tests ran clean but will need to confirm on a React 19 project that the test utils work now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant