hoangvvo / nextjs-mongodb-app

A Next.js and MongoDB web application, designed with simplicity for learning and real-world applicability in mind.
https://nextjs-mongodb.now.sh/
MIT License
1.54k stars 289 forks source link

Higher order component #72

Closed IRediTOTO closed 4 years ago

IRediTOTO commented 4 years ago

Hi, I know you use useCurrentUser to check login but we have many pages, and we must do it many times. Is there anyway to change from:

if (!user) {
    return (
      <div style={{ color: '#555', textAlign: 'center' }}>
        Please sign in to post
      </div>
    );
  } 

to something like this exports default withUser(<ThisPageJustForLoginUser />) This function check for user login, if not it will direct to login.

hoangvvo commented 4 years ago

@warcraft14115 Try this

function getDisplayName(WrappedComponent) {
  return WrappedComponent.displayName || WrappedComponent.name || 'Component';
}

function withUser(Page) {
  const [user, { mutate }] = useCurrentUser();
  function WithUser(props) {
    return user ? <Page {...props} /> : <div>Please sign in</div>
  }
  WithUser.displayName = `withUser(${getDisplayName(Page)})`;
  return WithUser;
}

Usage:

exports default withUser(ThisPageJustForLoginUser)
ErnestBrandi commented 4 years ago

Hi, I also have several questions related to that matter (handling user connection).

Having user object from a "single source of truth"

That would mean gathering user's infos from the _app file for instance. In that case one should pass over data through props (for each component that needs user's infos) which might be annoying and redundant. React Context (https://reactjs.org/docs/context.html) would also be an option. What do you think ?

Redirecting user when not logged

The following redirects us before user info is gathered, what's the correct way to do it ?

const [user] = useCurrentUser();
// ...
React.useEffect(() => {
    if (!user) {
      Router.push('/');
    }
}, [user]);

[Project related] Way to get users' infos

In pages/user/[userId], you get profile's infos in getServerSideProps and logged user's infos via a SWR hook.

Could it have been possible to create another custom SWR hook (ex: useSpecificUser({id})) and compare returned data with useCurrentUser() one ?

On the other hand, you could have done everything server side by comparing data returned with req.user in getServerSideProps; and returning props { isItMe: Boolean } for example.

Is there drawbacks doing any of those methods ?

Thanks for the overall project :)

hoangvvo commented 4 years ago

Hey @ErnestBrandi make sure you create a new issue next time instead of commenting here so I will not miss your question.

1) Why would you like to have user object from a single source of truth? And by user, do you mean the current user or all users? useSWR, which powered our current useCurrentUser hook, will deduplicate the requests. Say, if we use that hook different places, only one of them will be fired and the others will use the result from the fired one, which I think can be looked at some sort of "single source of truth"

2) There isn't a reliable way. I think if the data has not loaded useSWR will return undefined (not null). So you may want to check that specifically instead of using falsy (if (!user)) value which can either be undefined or null. While it is being loaded, you can block the render.

React.useEffect(() => {
    // user === null means it has loaded and resulted as `null`. `undefined` may signify that it has not loaded.
    if (user === null) {
      Router.push('/');
    }
}, [user]);

if (!user) return 'loading...' // or null

An even better way is to use Suspense!

I'm not sure about the undefined vs null thing so you should double check that.

3) Either way should be fine. I prefer to check it client-side instead of isItMe because I don't like adding an addition props and if you need to check it in child component you will need to passing it down which is annoying.

Your proposed useSpecificUser({id}) may actually be much better because you can do the check anywhere without passing down the prop ("prop drilling" problem)

Make sure the useSpecificUser (I assume also use swr) hook can take advatange of the value from getServerSideProps (See initialData)

ErnestBrandi commented 4 years ago

Hey @hoangvvo , sorry for using someone else thread. I thought my questions were closely related to the main subject first but it wasn't that much the case after all.

1) I have a hard time picturing swr that way ! So different calls in different pages will send back the same prefetched data (if so) ?

2) You were absolutely right about being specific for null ! Still, there could be a problem if there is no stale data at all (for example just after a login). Maybe explicitly calling mutate could solve that.

3) Great tip ! Never heard about initialData before (maybe should I have scrolled further down the doc ;)

Thanks a lot for your feedback ! You are unlocking doors for me :)

hoangvvo commented 4 years ago

@ErnestBrandi No worry! I just hate missing out questions.

even if the hook is used at different places, only one request (not many calls) will be made. when the request is finished, it will update everywhere the hook is used https://github.com/vercel/swr/issues/306. On different pages, the hook will use the cache (previous result) until it is revalidated.

IRediTOTO commented 4 years ago

Thank you @hoangvvo :)) I use getServerSideProps to check Just need

handler = nextConnect()
handler.use(userAuths)
await handler.apply(req,res)
...

:) And its ok, just for user page don't really need SEO for those pages. Your nextConnect very very usefull. Why people dont use it ? :\ I wonder