-
Notifications
You must be signed in to change notification settings - Fork 46.9k
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
Remove maxDuration #15272
Remove maxDuration #15272
Conversation
We instead assume a 150ms duration.
5a3cc07
to
8ac7e4c
Compare
Could you explain what the heuristic and different mechanism will entail? I'm looking to integrate React Async with Suspense, but it seems to still be in flux. It's hard to work towards compatibility with a moving target. |
Agree it’s hard — which is why using Suspense for data fetching in Concurrent Mode is not marked as stable. If you’re doing it you’re kind of on your own. We’re still figuring out some things in the design. |
More on what’s stable and what isn’t: |
) { | ||
earliestTimeoutMs = timeoutPropMs; | ||
} | ||
const defaultSuspenseTimeout = 150; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's a global const(config), why not lift it to the top?
Summary: We're replacing this user-specific attribute with an internal heuristic. Related PR facebook/react#15272 cc jbrown215 Pull Request resolved: #7613 Reviewed By: bvaughn Differential Revision: D14784577 Pulled By: jbrown215 fbshipit-source-id: c93609a2f095c34fd85242798d4989abffb273a4
We're going to replace it with a heuristic and different mechanism instead.
This PR just hard codes 150ms in place and fixes the tests. A follow up will refactor a bit more.