Skip to content

Commit

Permalink
Update test split into two tasks
Browse files Browse the repository at this point in the history
The ReactDOMSelect has an overlapping act error but I don't know why that wasn't erroring before.

I also don't get why waitFor isn't enough for useSubscription.

Nor why ReactExpiration changes. What does the additionalLogsAfterAttemptingToYield helper do?
  • Loading branch information
sebmarkbage committed Dec 17, 2024
1 parent f06e30b commit b228136
Show file tree
Hide file tree
Showing 9 changed files with 78 additions and 22 deletions.
4 changes: 1 addition & 3 deletions packages/react-dom/src/__tests__/ReactDOMSelect-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -911,9 +911,7 @@ describe('ReactDOMSelect', () => {
const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);
async function changeView() {
await act(() => {
root.unmount();
});
root.unmount();
}

const stub = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3743,6 +3743,10 @@ describe('ReactDOMServerPartialHydration', () => {
await waitForPaint(['App']);
expect(visibleRef.current).toBe(visibleSpan);

if (gate(flags => flags.enableYieldingBeforePassive)) {
// Passive effects.
await waitForPaint([]);
}
// Subsequently, the hidden child is prerendered on the client
await waitForPaint(['HiddenChild']);
expect(container).toMatchInlineSnapshot(`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,10 @@ describe('ReactDeferredValue', () => {
revealContent();
// Because the preview state was already prerendered, we can reveal it
// without any addditional work.
if (gate(flags => flags.enableYieldingBeforePassive)) {
// Passive effects.
await waitForPaint([]);
}
await waitForPaint([]);
expect(root).toMatchRenderedOutput(<div>Preview [B]</div>);
});
Expand Down
12 changes: 9 additions & 3 deletions packages/react-reconciler/src/__tests__/ReactExpiration-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -755,10 +755,16 @@ describe('ReactExpiration', () => {

// The update finishes without yielding. But it does not flush the effect.
await waitFor(['B1'], {
additionalLogsAfterAttemptingToYield: ['C1'],
additionalLogsAfterAttemptingToYield: gate(
flags => flags.enableYieldingBeforePassive,
)
? ['C1', 'Effect: 1']
: ['C1'],
});
});
// The effect flushes after paint.
assertLog(['Effect: 1']);
if (!gate(flags => flags.enableYieldingBeforePassive)) {
// The effect flushes after paint.
assertLog(['Effect: 1']);
}
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -2695,13 +2695,23 @@ describe('ReactHooksWithNoopRenderer', () => {
React.startTransition(() => {
ReactNoop.render(<Counter count={1} />);
});
await waitForPaint([
'Create passive [current: 0]',
'Destroy insertion [current: 0]',
'Create insertion [current: 0]',
'Destroy layout [current: 1]',
'Create layout [current: 1]',
]);
if (gate(flags => flags.enableYieldingBeforePassive)) {
await waitForPaint(['Create passive [current: 0]']);
await waitForPaint([
'Destroy insertion [current: 0]',
'Create insertion [current: 0]',
'Destroy layout [current: 1]',
'Create layout [current: 1]',
]);
} else {
await waitForPaint([
'Create passive [current: 0]',
'Destroy insertion [current: 0]',
'Create insertion [current: 0]',
'Destroy layout [current: 1]',
'Create layout [current: 1]',
]);
}
expect(committedText).toEqual('1');
});
assertLog([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,10 @@ describe('ReactSiblingPrerendering', () => {
await waitForPaint(['A']);
expect(root).toMatchRenderedOutput('A');

if (gate(flags => flags.enableYieldingBeforePassive)) {
// Passive effects.
await waitForPaint([]);
}
// The second render is a prerender of the hidden content.
await waitForPaint([
'Suspend! [B]',
Expand Down Expand Up @@ -237,6 +241,10 @@ describe('ReactSiblingPrerendering', () => {
// Immediately after the fallback commits, retry the boundary again. This
// time we include B, since we're not blocking the fallback from showing.
if (gate('enableSiblingPrerendering')) {
if (gate(flags => flags.enableYieldingBeforePassive)) {
// Passive effects.
await waitForPaint([]);
}
await waitForPaint(['Suspend! [A]', 'Suspend! [B]']);
}
});
Expand Down Expand Up @@ -452,6 +460,10 @@ describe('ReactSiblingPrerendering', () => {
</>,
);

if (gate(flags => flags.enableYieldingBeforePassive)) {
// Passive effects.
await waitForPaint([]);
}
// Immediately after the fallback commits, retry the boundary again.
// Because the promise for A resolved, this is a normal render, _not_
// a prerender. So when we proceed to B, and B suspends, we unwind again
Expand All @@ -471,6 +483,10 @@ describe('ReactSiblingPrerendering', () => {
</>,
);

if (gate(flags => flags.enableYieldingBeforePassive)) {
// Passive effects.
await waitForPaint([]);
}
// Now we can proceed to prerendering C.
if (gate('enableSiblingPrerendering')) {
await waitForPaint(['Suspend! [B]', 'Suspend! [C]']);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1901,6 +1901,10 @@ describe('ReactSuspenseWithNoopRenderer', () => {
// be throttled because the fallback would have appeared too recently.
Scheduler.unstable_advanceTime(10000);
jest.advanceTimersByTime(10000);
if (gate(flags => flags.enableYieldingBeforePassive)) {
// Passive effects.
await waitForPaint([]);
}
await waitForPaint(['A']);
expect(ReactNoop).toMatchRenderedOutput(
<>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,26 @@ describe('ReactUpdatePriority', () => {
// Schedule another update at default priority
setDefaultState(2);

// The default update flushes first, because
await waitForPaint([
// Idle update is scheduled
'Idle update',

// The default update flushes first, without including the idle update
'Idle: 1, Default: 2',
]);
if (gate(flags => flags.enableYieldingBeforePassive)) {
// The default update flushes first, because
await waitForPaint([
// Idle update is scheduled
'Idle update',
]);
await waitForPaint([
// The default update flushes first, without including the idle update
'Idle: 1, Default: 2',
]);
} else {
// The default update flushes first, because
await waitForPaint([
// Idle update is scheduled
'Idle update',

// The default update flushes first, without including the idle update
'Idle: 1, Default: 2',
]);
}
});
// Now the idle update has flushed
assertLog(['Idle: 2, Default: 2']);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ let ReplaySubject;
let assertLog;
let waitForAll;
let waitFor;
let waitForPaint;

describe('useSubscription', () => {
beforeEach(() => {
Expand All @@ -37,6 +38,7 @@ describe('useSubscription', () => {

const InternalTestUtils = require('internal-test-utils');
waitForAll = InternalTestUtils.waitForAll;
waitForPaint = InternalTestUtils.waitForPaint;
assertLog = InternalTestUtils.assertLog;
waitFor = InternalTestUtils.waitFor;
});
Expand Down Expand Up @@ -595,7 +597,7 @@ describe('useSubscription', () => {
React.startTransition(() => {
mutate('C');
});
await waitFor(['render:first:C', 'render:second:C']);
await waitForPaint(['render:first:C', 'render:second:C']);
React.startTransition(() => {
mutate('D');
});
Expand Down

0 comments on commit b228136

Please sign in to comment.