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

Edit Suspense Priority Warning Message #16186

Merged
merged 2 commits into from
Jul 24, 2019

Conversation

lunaruan
Copy link
Contributor

@lunaruan lunaruan commented Jul 23, 2019

move 'component that triggered the update' in suspense priority warning message to the beginning of the message instead of the middle for better readability

Message after update:

Warning: The components that triggered the update: COMPONENT_NAME

The following components suspended during a user-blocking update: COMPONENT_NAME

Updates triggered by user interactions (e.g. click events) are considered user-blocking by default. 
They should not suspend. Updates that can afford to take a bit longer should be wrapped with 
`Scheduler.next` (or an equivalent abstraction). This typically includes any update that shows new 
content, like a navigation.

@sizebot
Copy link

sizebot commented Jul 23, 2019

No significant bundle size changes to report.

Generated by 🚫 dangerJS

@acdlite
Copy link
Collaborator

acdlite commented Jul 23, 2019

Here's my suggested text:

The following components triggered a user-blocking update:

  ParentComponentThatUpdated

that was then suspended by:

  ComponentThatDependsOnNetworkData, AnotherOneOfThose, AndAnother

The fix is to split the update into multiple parts: a user-blocking update to
provide immediate feedback, and another update that triggers the bulk of
the changes.

Refer to the documentation for useSuspenseTransition to learn how to implement
this pattern.

@acdlite
Copy link
Collaborator

acdlite commented Jul 23, 2019

If it was triggered by the root, then the message should be:

A user-blocking update was suspended by:

  ComponentThatDependsOnNetworkData, AnotherOneOfThose, AndAnother

The fix is to split the update into multiple parts: a user-blocking update to
provide immediate feedback, and another update that triggers the bulk of
the changes.

Refer to the documentation for useSuspenseTransition to learn how to implement
this pattern.

and we'll add a separate warning inside ReactDOM.render et al, as discussed previously.

Copy link
Collaborator

@acdlite acdlite left a comment

Choose a reason for hiding this comment

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

Nice, thanks! Probably isn't the last time we'll bikeshed this message before releasing :D

@lunaruan lunaruan merged commit 06cc996 into facebook:master Jul 24, 2019
@lunaruan lunaruan deleted the edit_suspense_priority_message branch July 24, 2019 17:30
@nuragic
Copy link

nuragic commented Sep 24, 2019

@acdlite @lunaruan 👋 👀 I'm getting this message but I can't find any reference to useSuspenseTransition... any tips? Thanks!

@gaearon
Copy link
Collaborator

gaearon commented Sep 24, 2019

Are you using Concurrent Mode as an unstable API? It’s not officially supported.

@nuragic
Copy link

nuragic commented Sep 24, 2019

@gaearon I know it's unstable yet, but we're using it already and it's good enough as far as we can see... we have some use case in which it improved performance a lot. Just wondering if I can find any reference about this warning, orherwise it's fine... I'll wait :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants