erikras / erikras/react-redux-universal-hot-example

React Router `requireLogin` promise issue in `routes.js`

Open
#1,156 2 comments 1 reaction 0 assignees View on GitHub
Dominant language
JavaScript
Stars
12.1k
Forks
2.5k
PR merge metrics
No merged PRs in 30d

Description

The current flow is: When navigating to page '/chat', call `loadAuth` first. This api will resolve and never fail. It returns a null user even if the user is not login. So the promise only has one then for the success case. `store.dispatch(loadAuth()).then(checkAuth);`

However, in real case, 'loadAuth' call would return errors. In my project the server return 401 error if the user is not authorised. So in the `requireLogin` in `routes.js`, the promise should be have `catch`.

``` javascript
if (needAuthLoad(store.getState())) {
store.dispatch(loadAuth())
.then(() => cb()).catch(() => {
replace('/');
cb();
});
} else {
checkAuth();
}
```

Otherwise the promise will never be resolved.

If server is configured to return error in bad cases, the failed calls should also be treated as load, which means `loaded` should be true for `LOAD_FAIL`. Otherwise every navigate will call the api again. For example, if the user is not login, `loadAuth` return 401 error and `auth.loaded` is always false. Every component asyncConnecting to `loadAuth` will call it because `isLoaded` is always false. When user navigates to other url path, `loadAuth` will always be called, which I think is not efficient especially for bad networks.

Also `isLoaded` should also consider the case if `loading` is true.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.