Remove RTR from ReactSuspense-test #28390
Conversation
facebook-github-bot
added
CLA Signed
React Core Team
|
|
||
| await resolveText('Tab: 0'); | ||
| await waitForPaint(['Tab: 0']); | ||
| expect(root).toMatchRenderedOutput('Tab: 0 + sibling'); | ||
| assertLog(['Tab: 0']); |
There was a problem hiding this comment.
Any concerns with the waitForPaint calls moving to sync assertLog for legacy render tests? The text resolution already completes and populates scheduler logger when using ReactDOM.render as oppose to ReactTestRenderer.create() but I'm not sure of the reason.
|
Comparing: a515d75...794dc0c Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
| @@ -136,19 +141,20 @@ describe('ReactSuspense', () => { | |||
| 'Suspend! [A]', | |||
| 'Loading...', | |||
| ]); | |||
| expect(root).toMatchRenderedOutput(null); | |||
| expect(container.textContent).toEqual(''); | |||
There was a problem hiding this comment.
why is this empty and not null?
There was a problem hiding this comment.
textContent is always a string
document.createElement('div').textContent // ""| ); | ||
| await waitForAll(['Initial']); | ||
| expect(root).toMatchRenderedOutput('Initial'); | ||
| assertLog(['Initial']); |
There was a problem hiding this comment.
It's fine to use the waitForAll, it's basically a shortcut for act + assertLog.
|
|
||
| // This update will suspend. | ||
| root.update(<App shouldSuspend={true} step={1} />); | ||
| await act(() => { |
There was a problem hiding this comment.
act will flush everything, but the waitFor only flushes up to a certain log. I would expect flushing everything here to break something, since the point of this test is to partially flush, then schedule an update.
| // React has a heuristic to batch all updates that occur within the same | ||
| // event. This is a trick to circumvent that heuristic. | ||
| ReactTestRenderer.create('whatever'); | ||
| await act(() => { |
There was a problem hiding this comment.
It might not be correct for the interrupt to flush, see the comment above that this is a trick.
There was a problem hiding this comment.
Yeah still working on this one as it failed in the production build when the gating was applied. Trying to figure out how to recreate the interrupt trick without RTR
|
|
||
| await resolveText('Tab: 0'); | ||
| await waitForPaint(['Tab: 0']); | ||
| expect(root).toMatchRenderedOutput('Tab: 0 + sibling'); | ||
| assertLog(['Tab: 0']); |
### React upstream changes - facebook/react#28438 - facebook/react#28436 - facebook/react#25954 - facebook/react#28434 - facebook/react#28433 - facebook/react#28432 - facebook/react#28415 - facebook/react#27903 - facebook/react#28430 - facebook/react#28424 - facebook/react#28400 - facebook/react#28422 - facebook/react#28423 - facebook/react#28412 - facebook/react#28418 - facebook/react#28421 - facebook/react#28417 - facebook/react#28399 - facebook/react#28408 - facebook/react#28350 - facebook/react#28387 - facebook/react#28403 - facebook/react#28384 - facebook/react#28409 - facebook/react#28398 - facebook/react#28405 - facebook/react#28328 - facebook/react#28402 - facebook/react#28386 - facebook/react#28388 - facebook/react#28379 - facebook/react#28383 - facebook/react#28390 - facebook/react#28389 - facebook/react#28382 - facebook/react#28348 Closes NEXT-2600
## Summary Cleaning up internal usage of ReactTestRenderer ## How did you test this change? `yarn test packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js`
## Summary Cleaning up internal usage of ReactTestRenderer ## How did you test this change? `yarn test packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js` DiffTrain build for commit 90a0ae1.
Summary
Cleaning up internal usage of ReactTestRenderer
How did you test this change?
yarn test packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js