Skip to content
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Tests: Batching multiple attach/detach operations
I added some tests to illustrate what should happen when multiple
attach/detach operations happen in a single event handler, and the
corresponding scenario for when they happen inside an effect. They
need to be batched into a single operation — similar to how a setState
would work.

The simplest case to think about is when you call attach, and then
immediately detach on the very next line. This should be a no-op.

To implement this properly, we need to queue the operations, like we
do for other state updates.
  • Loading branch information
acdlite authored and sammy-SC committed Dec 12, 2022
commit fd230d9906374e9c7082bf749f526a5eac743a63
80 changes: 80 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactOffscreen-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1968,4 +1968,84 @@ describe('ReactOffscreen', () => {

expect(Scheduler).toHaveYielded(['unmount layout inner', 'unmount inner']);
});

// @gate enableOffscreen
it('batches multiple attach and detach calls scheduled from an event handler', async () => {
function Child() {
useEffect(() => {
Scheduler.unstable_yieldValue('Attach child');
return () => {
Scheduler.unstable_yieldValue('Detach child');
};
}, []);
return 'Child';
}

const offscreen = React.createRef(null);
function App() {
return (
<Offscreen ref={offscreen} mode="manual">
<Child />
</Offscreen>
);
}

const root = ReactNoop.createRoot();
await act(() => {
root.render(<App />);
});
expect(Scheduler).toHaveYielded(['Attach child']);

await act(async () => {
const instance = offscreen.current;

// Attach then immediately re-attach the instance. This should be a
// no-op because the operations are batched.
instance.detach();
instance.attach();
});
// No effects should have attached or detached
expect(Scheduler).toHaveYielded([]);
});

// @gate enableOffscreen
it('batches multiple attach and detach calls scheduled from an effect', async () => {
function Child() {
useEffect(() => {
Scheduler.unstable_yieldValue('Attach child');
return () => {
Scheduler.unstable_yieldValue('Detach child');
};
}, []);
return 'Child';
}

function App() {
const offscreen = useRef(null);
useLayoutEffect(() => {
const instance = offscreen.current;

// Attach then immediately re-attach the instance. This should be a
// no-op because the operations are batched.
instance.detach();
instance.attach();
}, []);
return (
<Offscreen ref={offscreen} mode="manual">
<Child />
</Offscreen>
);
}

const root = ReactNoop.createRoot();
await act(() => {
root.render(<App />);
});
expect(Scheduler).toHaveYielded([
'Parent effect',
'Attach child',

// The child effects should not be toggled
]);
});
});