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

SWR should throw an error when suspense is enabled but key is not ready #1359

Closed
shuding opened this issue Aug 20, 2021 · 3 comments · Fixed by #1402
Closed

SWR should throw an error when suspense is enabled but key is not ready #1359

shuding opened this issue Aug 20, 2021 · 3 comments · Fixed by #1402

Comments

@shuding
Copy link
Member

shuding commented Aug 20, 2021

Related to #339. When the suspense option is enabled, SWR can not return any value to continue rendering, neither throwing any promise if it can't fetch. This includes 2 situations:

  1. suspense: true and key is falsy.
  2. suspense: true and fetcher is null.

In both cases SWR should throw an error.

@koba04
Copy link
Collaborator

koba04 commented Aug 28, 2021

@shuding
How are you going to add the error?
The easiest way to add this is like the following.

if (suspense && (!key || fetcher === null)) { 
  throw new Error("Either key or fetcher must be specified with suspense mode");
}

But I think SWR hasn't had any errors like this yet and its bundle size is important. Those error messages cannot be minified and would increase the bundle size. Is it acceptable for SWR?
Or are you going to adopt a way to avoid increasing its bundle size like what React and Redux do?

I remember that there was a similar discussion before.
#898

@shuding
Copy link
Member Author

shuding commented Aug 28, 2021

I think maintaining a list of error codes like React is a good idea. 👍

And in longer term we need to create 2 builds, one for development (warnings, errors, useDebugValue) and one for production use (minimal bundle with only error codes).

@shuding
Copy link
Member Author

shuding commented Aug 28, 2021

That said I'm good with fixing this with the proper error message for now and address error codes later, correctness is more important than size. 👍

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

Successfully merging a pull request may close this issue.

2 participants