detectHostComponentNames: render test tree inside act to avoid timer leaks#1371
Merged
mdjastrzebski merged 4 commits intocallstack:mainfrom Mar 23, 2023
Merged
Conversation
f57ca72 to
6096251
Compare
mdjastrzebski
approved these changes
Mar 23, 2023
Member
mdjastrzebski
left a comment
There was a problem hiding this comment.
@jsnajdr Looks good! Again thanks for submitting this non-trivial fix!
jsnajdr
commented
Mar 23, 2023
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1371 +/- ##
=======================================
Coverage 96.13% 96.14%
=======================================
Files 49 50 +1
Lines 3314 3318 +4
Branches 491 492 +1
=======================================
+ Hits 3186 3190 +4
Misses 128 128
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
Member
|
Released in v12.0.1 🎉 |
hyochan
referenced
this pull request
in hyochan/react-native-iap
Mar 26, 2023
….1 (#2377) [](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [@testing-library/react-native](https://callstack.github.io/react-native-testing-library) ([source](https://github.com/callstack/react-native-testing-library)) | [`12.0.0` -> `12.0.1`](https://renovatebot.com/diffs/npm/@testing-library%2freact-native/12.0.0/12.0.1) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>callstack/react-native-testing-library</summary> ### [`v12.0.1`](https://github.com/callstack/react-native-testing-library/releases/tag/v12.0.1) [Compare Source](https://github.com/callstack/react-native-testing-library/compare/v12.0.0...v12.0.1) #### What's Changed ##### Bugfixes - Flush all pending microtasks and updates before returning from waitFor by [@​jsnajdr](https://github.com/jsnajdr) in [https://github.com/callstack/react-native-testing-library/pull/1366](https://github.com/callstack/react-native-testing-library/pull/1366) - Render host component name detection tree inside act to avoid timer leaks by [@​jsnajdr](https://github.com/jsnajdr) in [https://github.com/callstack/react-native-testing-library/pull/1371](https://github.com/callstack/react-native-testing-library/pull/1371) #### New Contributors 👏 - [@​jsnajdr](https://github.com/jsnajdr) made their first contributions in [https://github.com/callstack/react-native-testing-library/pull/1366](https://github.com/callstack/react-native-testing-library/pull/1366), [https://github.com/callstack/react-native-testing-library/pull/1371](https://github.com/callstack/react-native-testing-library/pull/1371) **Full Changelog**: callstack/react-native-testing-library@v12.0.0...v12.0.1 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/dooboolab/react-native-iap). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4xNC4yIiwidXBkYXRlZEluVmVyIjoiMzUuMTQuMiJ9--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes a bug that I discovered while working on #1366: when first calling
renderin a test suite, therenderfunction callsdetectHostComponentNames, which mounts a testing tree with a few components. This tree is rendered outside theactenvironment, and therefore schedules asetImmediatecallback to process effects. This scheduled callback then has an unwanted interaction with my actual testing code. If there is awaitForinside my test, the effects scheduled by my testing code will not be processed by mysetImmediate, but by the one scheduled bydetectHostComponentNames. That makes the precise order of events inside my test non-deterministic and unpredictable.Look at this call stack:
and note how it starts in my
waitFor.test.tstest, in therenderfunction, then callsdetectHostComponentNames, schedulessetImmediatecallback, and how thatsetImmediatelater processes an update scheduled by my test code (theApplecomponent).This PR avoids that by wrapping the render with
act. That way, all effects scheduled by the render will come into theactQueue, and will be flushed before returning. No leaks.In the code, I had to do some silly IIFE magic to make TypeScript happy: it doesn't know that
actwill synchronously and reliably initialize theresultvalue.