-
Notifications
You must be signed in to change notification settings - Fork 48.5k
Suspense: add messaging when suspense promise neither resolves nor rejects #17621
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
Comments
cc @sebmarkbage @gaearon. I think this one is similar to #17526 in that it is related to the ergonomics/DX for people using Suspense. |
This is really up to the library providing suspense to do. It's very common for a promise to not resolve in 3 seconds. There are also use cases for very long suspense times. Even It's better to address this using DevTools where we can hint of things that are slow and surface that and you can make a choice whether that's expected or not. Throwing a promise is not meant to be ergonomic nor even a public API. It's not meant to be used directly. There should be maybe 10 libraries that need to do it ever. If it's truly is a bug that it never resolves, then that is to be fixed in the high level API that does the throwing and then it's not an ergonomic problem in product development. It's likely that libraries need a timeout but that timeout will reject the Promise and actually error. |
I can understand your argument here, but disagree with it and am surprised by how strongly worded it is. It is simply untrue to say that throwing a promise is not a public API. Even though it is undocumented, it is something that is exposed to external code. It's already in the stable releases of React and, once data fetching is released as stable, will have to maintain backwards compatibility in order for third party code to continue working. I think it's unrealistic to expect everyone to use Relay (or similar), and that much of the wider React community will look to use suspense without a big abstraction on top of it. Certainly library authors will want to use suspense directly. In my opinion, the API for triggering suspense should be a good one. This includes ergonomics, documentation, and a well thought out API and implementation that React can stand behind. When I hear you emphasize how the bad or confusing things in the suspense API will be taken care of by third party libraries, what I hear is "it's sufficiently confusing and nuanced that we'd rather hide it than explain it." I imagine you might disagree with the above 😄, but I wanted to share my honest feedback on why I think Suspense is going to cause more pain than it is worth for many developers. The reason I created this Github issue is that a developer reached out to me after three full days of debugging a problem with suspense. The root cause of the problem was that the promise being thrown never resolved nor rejected, and she was unable to diagnose or debug it. In her words, "its been a real headache trying to debug and know whats happening." You may say that it is our fault for trying to use suspense without waiting for these ten abstraction libraries to appear, but I disagree that APIs exposed from React should be this easy to use incorrectly. |
I agree that it was a mistake to expose it in the first place (even if it’s undocumented) but I think the solution would that will rather be to completely deprecate the confusing API and replace it with the higher level one. I do think that relying on undocumented behavior is a bad pattern that we as an industry need to start moving away from. It puts unreasonable expectations on library authors. The reason it is strongly worded is because there are a number of issues all related to misusing this undocumented API. I understand that it’s ultimately our problem but it’s a bit annoying. That’s not your fault tho. |
👍 for me the important thing is that Suspense can be used directly. Making a higher level API for it could help with that.
Fair enough. Myself and 2-3 developers I've been working with on this project have definitely entered the waters of experimental and undocumented APIs. We have enjoyed learning (reverse engineering?) the behavior of the Suspense API as we've gone, even though it's taken us a while and we've had some surprises along the way. We have known we're in uncharted territory a bit and understand the costs and pains of that. I do not think it is your fault that we've spent a few days figuring it out. We're the kind of developers who enjoying getting into the nitty gritty 😄 |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution. |
Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you! |
Do you want to request a feature or report a bug?
Request a feature
What is the current behavior?
If you throw a promise that neither resolves nor rejects, you hit the suspense fallback UI indefinitely and receive no console messaging that helps you debug this.
Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
All versions that allow for suspense
Proposed feature
Log a warning to the console if a suspense promise takes longer than a certain amount of time to settle. Here is some code demonstrating what that could look like inside of the react codebase:
The text was updated successfully, but these errors were encountered: