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

Modernize tests #1108

Merged
merged 12 commits into from
Jan 15, 2025
Merged

Modernize tests #1108

merged 12 commits into from
Jan 15, 2025

Conversation

rpiaggio
Copy link
Collaborator

@rpiaggio rpiaggio commented Jan 14, 2025

This PR is a step forward into removing ReactTestUtils in favor of the newer ReactTestUtils2 (which will be renamed once the former is removed).

It also removes all uses of the sync version of act in favor of the async one, given that the React docs specify that:

We recommend using act with await and an async function. Although the sync version works in many cases, it doesn’t work in all cases and due to the way React schedules updates internally, it’s difficult to predict when you can use the sync version.

We will deprecate and remove the sync version in the future.

A number of scaffolding code was also implemented:

  • AsyncTestSuite allows running tests that return an AsyncCallback and runs them. This is not published but rather copied and pasted whenever it's used. I'm not sure where to place it and it would probably need to be in its own scalajs-react-utest-acb bundle.
  • polyfill.js now provides access to MessageChannel, which allows executing async/await within jsdom. Again, this is copied in the few test project where it is used.
  • Resource implements a managed resource. It is a generic (and therefore async) version of WithDsl. I think WithDsl can be replaced in the future with Resource[Id]. However, we may end up deprecating and removing WithDsl together with the sync version of act. This class only lives in the testing utils project, but it may be useful for other things (like AroundReact). If we decide to adopt it for client use, we may want to polish it up a bit. Eg: build an AST and then interpret it, rather than using vars, and making sure it is stack safe.

The few places where ReactTestUtils is still used is where class components are tested. These tests need access to the mounted versions of the components, which the modern testing framework doesn't seem to provide. Probably the course of action here will be just to remove these tests, or move them all, together with ReactTestUtils to a tests-legacy project. I'm open to suggestions.

@rpiaggio
Copy link
Collaborator Author

Doc update is pending.

Copy link
Contributor

@toddburnside toddburnside left a comment

Choose a reason for hiding this comment

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

Wow! That was a lot of work. LGTM.

@@ -1,20 +1,22 @@
package downstream
// package downstream
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this file go away?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We will reinstate it once we release 3.0.0.

@rpiaggio rpiaggio merged commit 79f9eac into topic/react18 Jan 15, 2025
2 checks passed
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