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

test: Run with React 18 #2526

Closed
wants to merge 5 commits into from
Closed

test: Run with React 18 #2526

wants to merge 5 commits into from

Conversation

eps1lon
Copy link
Contributor

@eps1lon eps1lon commented Nov 8, 2021

Adjusted the tests to work with React 18. Some tests are still failing but it's unclear from an outside perspective whether this is a testing issue or implementation issue. Failing tests are skipped and annotated with FIXME(react18).

renderHook from @testing-library/react replacing @testing-library/react-hooks is not final.

Most of the adjustments are batching adjustments:

Before:

act(() => {
   keydown(); // updates are batched 
   runTimers(); // this will not contain timers from upates of keydown
})

Fixed:

keydown(); // already wrapped in act by testing-library
act(() => {
   runTimers();
})

You've got plenty of "cannot update a component while rendering another" (e.g. "Cannot update a component (PopoverTrigger) while rendering a different component (ForwardRef(Dialog))"). This is likely an implementation issue.

✅ 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:

yarn test

🧢 Your Project:

@eps1lon eps1lon marked this pull request as ready for review November 8, 2021 09:44
@snowystinger
Copy link
Member

Hey! thanks for taking a look at this. We've done some work around React 18 in this previous PR #2142
We closed it to wait for more progress from React towards a stable release. I'm not sure of our timeline for when we were going to pick that back up again. I'll check with the team.

@snowystinger
Copy link
Member

snowystinger commented Dec 11, 2021

gah, so sorry about all the extra commits, I was not expecting that. sorry about my little snafu if you noticed it, accidentally pushed a ton of commits from a bad merge

I think I've got it cleared up, I'll make a better commit on Monday, thanks again for starting this

@snowystinger snowystinger mentioned this pull request Feb 2, 2022
@devongovett devongovett mentioned this pull request May 6, 2022
@eps1lon eps1lon deleted the test/react-18 branch May 21, 2022 05:25
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.

2 participants