Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a regression test for an infinite suspense + Fix #27703

Merged
merged 3 commits into from
Dec 8, 2023
Merged

Conversation

tyao1
Copy link
Contributor

@tyao1 tyao1 commented Nov 15, 2023

Add a regression test for the minimal repro from @kassens

And includes the fix from @acdlite:

This is another place we special-case Retry lanes to opt them out of
expiration. The reason is we rely on time slicing to unwrap uncached
promises (i.e. async functions during render). Since that ability is
still experimental, and enableRetryLaneExpiration is Meta-only, we can
remove the special case when enableRetryLaneExpiration is on, for now.

@tyao1 tyao1 requested a review from kassens November 15, 2023 00:07
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Nov 15, 2023
@tyao1 tyao1 marked this pull request as draft November 15, 2023 00:08
@react-sizebot
Copy link

react-sizebot commented Nov 15, 2023

Comparing: aec521a...f04d179

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 175.90 kB 175.90 kB = 54.75 kB 54.76 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 177.97 kB 177.97 kB = 55.39 kB 55.39 kB
facebook-www/ReactDOM-prod.classic.js +0.02% 569.81 kB 569.92 kB +0.02% 100.29 kB 100.31 kB
facebook-www/ReactDOM-prod.modern.js +0.02% 553.67 kB 553.77 kB +0.02% 97.38 kB 97.40 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against f04d179

@tyao1 tyao1 marked this pull request as ready for review November 15, 2023 00:43
@@ -4008,4 +4008,76 @@ describe('ReactSuspenseWithNoopRenderer', () => {
);
},
);

// @gate enableLegacyCache
Copy link
Member

Choose a reason for hiding this comment

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

We can remove this, but probably need the enableRetryLaneExpiration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

enableLegacyCache is needed to make the test pass, otherwise text cache in the test setup throws.
enableRetryLaneExpiration has VARIANT flag so it runs with both true and false, I think it's better to have it run both.

@tyao1 tyao1 requested a review from acdlite November 16, 2023 22:51
This is another place we special-case Retry lanes to opt them out of
expiration. The reason is we rely on time slicing to unwrap uncached
promises (i.e. async functions during render). Since that ability is
still experimental, and enableRetryLaneExpiration is Meta-only, we can
remove the special case when enableRetryLaneExpiration is on, for now.
@tyao1 tyao1 changed the title Add a regression test for an infinite suspense Add a regression test for an infinite suspense + Fix Dec 8, 2023
@tyao1 tyao1 merged commit f193213 into main Dec 8, 2023
36 checks passed
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
Add a regression test for the [minimal
repro](https://codesandbox.io/s/react-18-suspense-state-never-resolving-bug-hmlny5?file=/src/App.js)
from @kassens

And includes the fix from @acdlite: 
> This is another place we special-case Retry lanes to opt them out of
expiration. The reason is we rely on time slicing to unwrap uncached
promises (i.e. async functions during render). Since that ability is
still experimental, and enableRetryLaneExpiration is Meta-only, we can
remove the special case when enableRetryLaneExpiration is on, for now.

---------

Co-authored-by: Andrew Clark <[email protected]>
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.

5 participants