Skip to content

Failing tests of combining <Activity> and use()#31417

Closed
josephsavona wants to merge 1 commit into
facebook:mainfrom
josephsavona:use-activity-bug
Closed

Failing tests of combining <Activity> and use()#31417
josephsavona wants to merge 1 commit into
facebook:mainfrom
josephsavona:use-activity-bug

Conversation

@josephsavona
Copy link
Copy Markdown
Member

I created a copy of ActivitySuspense-test that suspends via use(), which shows some errors that likely correspond to an infinite loop I'm seeing when trying to integrate use() in Relay.

@vercel
Copy link
Copy Markdown

vercel Bot commented Nov 4, 2024

Deployment failed with the following error:

You don't have permission to create a Preview Deployment for this project.

View Documentation: https://vercel.com/docs/accounts/team-members-and-roles

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Nov 4, 2024
@react-sizebot
Copy link
Copy Markdown

react-sizebot commented Nov 4, 2024

The size diff is too large to display in a single comment. The GitHub action for this pull request contains an artifact called 'sizebot-message.md' with the full message.

Generated by 🚫 dangerJS against 8c930ed

I created a copy of ActivitySuspense-test that suspends via `use()`, which shows some errors that likely correspond to an infinite loop I'm seeing when trying to integrate `use()` in Relay.
Comment on lines +174 to +216
// @gate enableActivity
test.only('suspend, resolve, hide (forces refresh), suspend, reveal', async () => {
global.IS_REACT_ACT_ENVIRONMENT = true;
const root = ReactNoop.createRoot();

let setMode;
function Container({text}) {
const [mode, _setMode] = React.useState('visible');
setMode = _setMode;
useEffect(() => {
return () => {
Scheduler.log(`Clear [${text}]`);
textCache.delete(text);
};
});
return (
//$FlowFixMe
<Suspense fallback="Loading">
<Activity mode={mode}>
<AsyncText text={text} />
</Activity>
</Suspense>
);
}

await React.act(() => {
root.render(<Container text="hello" />);
});
assertLog(['Suspend! [hello]']);
expect(root).toMatchRenderedOutput('Loading');

await React.act(() => {
resolveText('hello');
});
assertLog(['hello']);
expect(root).toMatchRenderedOutput('hello');

await React.act(() => {
setMode('hidden');
});
assertLog(['Clear [hello]', 'Suspend! [hello]']);
expect(root).toMatchRenderedOutput('');
});
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was the case that failed w an infinite loop

@rickhanlonii
Copy link
Copy Markdown
Member

Pushed tests to #31434 with you as co-author

rickhanlonii added a commit that referenced this pull request Nov 11, 2024
## Overview

In `scheduleTaskForRootDuringMicrotask` we clear `root.callbackNode` if
the work loop is [suspended waiting on
data](https://github.com/facebook/react/blob/ac3ca097aeecae8fe3ec7f9b286307a923676518/packages/react-reconciler/src/ReactFiberRootScheduler.js#L338).

But we don't null check `root.callbackNode` before returning a
continuation in `performWorkOnRootViaSchedulerTask` where
`scheduleTaskForRootDuringMicrotask` is synchronously called, causing an
infinite loop when the only thing in the queue is something suspended
waiting on data.

This essentially restores the behavior from here:
https://github.com/facebook/react/pull/26328/files#diff-72ff2175ae3569037f0b16802a41b0cda2b2d66bb97f2bda78ed8445ed487b58L1168

Found by investigating the failures for
#31417

## TODO
- add a test

---------

Co-authored-by: Joe Savona <joesavona@fb.com>
github-actions Bot pushed a commit that referenced this pull request Nov 11, 2024
## Overview

In `scheduleTaskForRootDuringMicrotask` we clear `root.callbackNode` if
the work loop is [suspended waiting on
data](https://github.com/facebook/react/blob/ac3ca097aeecae8fe3ec7f9b286307a923676518/packages/react-reconciler/src/ReactFiberRootScheduler.js#L338).

But we don't null check `root.callbackNode` before returning a
continuation in `performWorkOnRootViaSchedulerTask` where
`scheduleTaskForRootDuringMicrotask` is synchronously called, causing an
infinite loop when the only thing in the queue is something suspended
waiting on data.

This essentially restores the behavior from here:
https://github.com/facebook/react/pull/26328/files#diff-72ff2175ae3569037f0b16802a41b0cda2b2d66bb97f2bda78ed8445ed487b58L1168

Found by investigating the failures for
#31417

## TODO
- add a test

---------

Co-authored-by: Joe Savona <joesavona@fb.com>

DiffTrain build for [b836de6](b836de6)
github-actions Bot pushed a commit that referenced this pull request Nov 11, 2024
## Overview

In `scheduleTaskForRootDuringMicrotask` we clear `root.callbackNode` if
the work loop is [suspended waiting on
data](https://github.com/facebook/react/blob/ac3ca097aeecae8fe3ec7f9b286307a923676518/packages/react-reconciler/src/ReactFiberRootScheduler.js#L338).

But we don't null check `root.callbackNode` before returning a
continuation in `performWorkOnRootViaSchedulerTask` where
`scheduleTaskForRootDuringMicrotask` is synchronously called, causing an
infinite loop when the only thing in the queue is something suspended
waiting on data.

This essentially restores the behavior from here:
https://github.com/facebook/react/pull/26328/files#diff-72ff2175ae3569037f0b16802a41b0cda2b2d66bb97f2bda78ed8445ed487b58L1168

Found by investigating the failures for
#31417

## TODO
- add a test

---------

Co-authored-by: Joe Savona <joesavona@fb.com>

DiffTrain build for [b836de6](b836de6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed React Core Team Opened by a member of the React Core Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants