feat: Use concurrent React when available #937
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit f9c851e:
|
eps1lon
changed the title
Codecov Report
@@ Coverage Diff @@
## alpha #937 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 4 4
Lines 140 179 +39
Branches 28 38 +10
=========================================
+ Hits 140 179 +39
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
eps1lon
force-pushed
the
feat/concurrent-render
branch
from
d8f6c91 to
14d5753
Compare
Otherwise we can't see directly which version is failing
These only exists as invariants.
…ng-library#929)" This reverts commit c1878a9.
eps1lon
force-pushed
the
feat/concurrent-render
branch
from
c42e134 to
60395b4
Compare
| unstable_advanceTimersWrapper: cb => { | ||
| return act(cb) | ||
| }, | ||
| asyncWrapper: cb => cb(), |
There was a problem hiding this comment.
Is this necessary to configure?
There was a problem hiding this comment.
TL;DR: I misunderstood the question. I think we don't need to configure that because it's the default. I still leave my original long winded explainer for future reference and because it made me realize I still don't fully understand why act() behaves the way it does.
So let's say we have
waitFor(() => expect(getByRole('button')).toBeInTheDocument())and the button would only appear after two intervals. This would previously translate to
act(() => {
jest.advanceTimersByTime();
checkCallback();
jest.advanceTimersByTime();
checkCallback();
});The problem is that React only flushes effects when exiting the act call. So checkCallback could not assert on any effect scheduled due to timers being advanced.
If we just configure unstable_advanceTimersWrapper to use act we would now have
act(() => {
jest.advanceTimersByTime();
act(() => {
checkCallback();
})
jest.advanceTimersByTime();
act(() => {
checkCallback();
});
});
The problem now is that React only flushes scheduled effects in the outermost act so we need to remove that.
I think I good question is why only the outermost act is considered.
There was a problem hiding this comment.
I need to double check something though. React 17 will no longer have act as the asyncWrapper but somehow our React 17 tests are still passing. So either our tests are incomplete or this was dead code.
There was a problem hiding this comment.
I need to double check something though. React 17 will no longer have
actas theasyncWrapperbut somehow our React 17 tests are still passing. So either our tests are incomplete or this was dead code.
Did you mean React 18? If so, following this comment I'm not sure it won't work, it's just not needed anymore, isn't it?
There was a problem hiding this comment.
Did you mean React 18? If so, following this comment I'm not sure it won't work, it's just not needed anymore, isn't it?
No, 17. I was reffering to our usage though. I changed the behavior for React 17 as well but no tests started failing.
There was a problem hiding this comment.
I need to double check something though. React 17 will no longer have
actas theasyncWrapperbut somehow our React 17 tests are still passing. So either our tests are incomplete or this was dead code.
Tests are incomplete due to usage of class components. Backported the new tests with a function component in #962 that should ensure asyncWrapper is still present in React 17.
There was a problem hiding this comment.
Prevented mild disaster since the new tests also uncovered silly lines such as typeof React.startTransition !== undefined. It's either React.startTransition !== undefined or typeof React.startTransition !== 'undefined'.
Also uncovered that the "configure wrapper" approach is not compatible with supporting different timers across multiple versions of react.
There was a problem hiding this comment.
Going to leave this a bit until the next WG meeting. Unclear what the timeline is for work on act. Feels like the core team already moved on but there are still glaring holes in the behavior of act().
There was a problem hiding this comment.
Though I'm personally fine with shipping this as an alpha to test integration. We haven't solved all problems yet for the alpha but neither has React itself. I'm fine with shipping this in an incomplete state since I can just blame React about all the missing parts 😎
eps1lon
force-pushed
the
feat/concurrent-render
branch
from
5c5c433 to
f9c851e
Compare
|
🎉 This PR is included in version 13.0.0-alpha.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
|
Testing this |
@nstepien I'm also experiencing this on tests using |
|
@nstepien @diegohaz Please read #509 (comment) |
BREAKING CHANGE: If you have React 18 installed, we'll use the new [`createRoot` API](reactwg/react-18#5) by default which comes with a set of [changes while also enabling support for concurrent features](reactwg/react-18#4). To can opt-out of this change by using `render(ui, { legacyRoot: true } )`. But be aware that the legacy root API is deprecated in React 18 and its usage will trigger console warnings.
BREAKING CHANGE: If you have React 18 installed, we'll use the new
createRootAPI by default which comes with a set of changes while also enabling support for concurrent features.To can opt-out of this change by using
render(ui, { legacyRoot: true } ). But be aware that the legacy root API is deprecated in React 18 and its usage will trigger console warnings.If you want to try out this build follow the "local install instructions" in https://ci.codesandbox.io/status/testing-library/react-testing-library/pr/937/
Continuation of #925 that GitHub is unable to re-open.
Integration tests:
What:
Closes #509
Why:
Concurrent features will be available in React 18.
Opt-out (breaking change) vs opt-in (feature release):
createRootis considered to be the default in React 18 so we should follow.How:
Add a
concurrentoption torender. Note that there are still some unsolved problems with testing concurrent features like reactwg/react-18#21 (comment) and reactwg/react-18#23 (comment). So testing concurrent features will change. We will only support the latest pre-release of React 18. So any release of@testing-library/reactmay break your testing setup if don't have the latest alpha/beta/release candidate installed.Checklist:
docs site