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

Add an option to disable fetching in UserProvider #313

Closed
adarnon opened this issue Feb 27, 2021 · 8 comments · Fixed by #325
Closed

Add an option to disable fetching in UserProvider #313

adarnon opened this issue Feb 27, 2021 · 8 comments · Fixed by #325
Labels
enhancement New feature or request

Comments

@adarnon
Copy link

adarnon commented Feb 27, 2021

Describe the problem you'd like to have solved

Hi,

In my app, I have a few pages where I don't want the user object to be loaded at all. These are public-facing pages and I want to save the unnecessary fetches to /api/auth/me because the user object shouldn't be available there.

Due to the implementation of UserProvider and useUser, a UserProvider MUST exist in the App tree in order to call useUser, otherwise an exception is thrown. Some of my components call the useUser hook internally, and I'd like them to return user as undefined in that case. However, UserProvider will ALWAYS fetch /api/auth/me, so there is no way to avoid that.

Describe the ideal solution

I think that a prop flag could be used in order to instruct UserProvider not to perform any fetching. In that case, it would either use the user object provided in the user prop (e.g., from SSR) or nothing.

Another solution would be to export the ReactContext object (User), so that the context provider may be manually overridden. This could also be useful for unit testing, e.g. to mock checkSession().

Alternatives and current work-arounds

A current workaround is to pass profileUrl="" to UserProvider in these routes, causing the fetch to fail.

Additional information, if any

@adamjmcgrath
Copy link
Contributor

Hi @adarnon - performing the fetch to /api/auth/profile is pretty much the only thing that the useUser hook does.

it would either use the user object provided in the user prop (e.g., from SSR) or nothing.

Have you considered writing your own context and using that instead?

// use-user.js
export const UserContext = React.createContext(
  user: undefined;
);

// _app.js
const App = ({ Component, pageProps }) => (
  <UserContext.Provider value={{ user: pageProps.user  }}>
    <Component {...pageProps} />
  </UserContext.Provider>
);

@adamjmcgrath adamjmcgrath added the enhancement New feature or request label Mar 1, 2021
@adarnon
Copy link
Author

adarnon commented Mar 1, 2021

@adamjmcgrath I've considered using my own provider (that's what I've been doing before v1.0 was released), but I was hoping to migrate this the built-in one so I that I can also use the other provided utilities withPageAuthRequired etc.

@adamjmcgrath
Copy link
Contributor

@adarnon

so I that I can also use the other provided utilities withPageAuthRequired etc.

Ah yep - that makes sense.

Some of my components call the useUser hook internally, and I'd like them to return user as undefined in that case.

Can you give me a more concrete example (do you have some code or example app)? I'm still not sure why you'd want to use the useUser in a context where you always want it to return undefined for the user.

Another solution would be to export the ReactContext object (User), so that the context provider may be manually overridden.

This sounds reasonable, can you tell me more about what specifically you'd want to export, and how that would fix your problem?

@saqbach
Copy link

saqbach commented Mar 2, 2021

I can't speak for @adarnon, but at least in our implementation, we have the <UserProvider> component wrapping everything _app.tsx and we use it to do things like calling useUser on our main navigation to toggle visibility of our Login/Signup buttons vs a Dashboard button by doing something like {user ? ( <Dashboard /> ) : ( <><Login /><Signup /></> )}

However, what this results in is a 401 console error when it's trying to call /api/auth/me for every single page visit to the non-authenticated parts of the site, regardless of whether we're calling useUser on the page or not. Might be a slightly different use case to what was described, but I believe it's similar in that we'd love for two things:

  1. Option to somehow explicitly tell <UserProvider> to not make the call on specific pages.
  2. In a perfect world, modify the check somehow so even on pages where we do want to call useUser it's not throwing console errors upon verifying the visitor is not authenticated.

@adarnon
Copy link
Author

adarnon commented Mar 2, 2021

I have a similar case like @austinhale.

Another example - we use Segment for analytics. I wrapped our track calls with a hook called useTrack() that adds some extra props to each call. This hook internally calls useUser() in order to add a few things about the user, if a user is logged in. However, in the public pages I don't want to log the user in at all, and there's no way to call useUser() conditionally because of the rules of hooks.

@adarnon
Copy link
Author

adarnon commented Mar 2, 2021

This sounds reasonable, can you tell me more about what specifically you'd want to export, and how that would fix your problem?

If const User = createContext() was exported, I'd be able to add my own User.Provider to the Application tree with empty fields for the user ({user: undefined, loading: false}), so that the "provider missing" error wouldn't be thrown. That would also be useful for testing as it'll allow me to create more specific cases and mock out checkSession without having to deal with msw and waiting for useEffects

@adamjmcgrath
Copy link
Contributor

adamjmcgrath commented Mar 3, 2021

Thanks for the context @adarnon - I'll raise some work to export User = createContext() shortly

@itsthekeming
Copy link

itsthekeming commented Aug 4, 2021

Can you give me a more concrete example (do you have some code or example app)? I'm still not sure why you'd want to use the useUser in a context where you always want it to return undefined for the user.

In my use case, I have an app that uses a different layout in the same route based on the authenticated state of the user. I'll call them "app layout" and "marketing layout".

If the user is not logged in, / returns the marketing layout, but if they are, it returns the app layout. I achieve this by getting the session in getServerSideProps and passing whether it is truthy as a prop that conditionally renders the appropriate layout.

However, I want my users to be able to access that original marketing layout once they are logged in, so I expose it at /home, similar to how GitHub exposes github.com/home once you are logged in, now that github.com returns your dashboard.

Within the navigation bar in marketing layout, I want to display either a login/signup UI or an account menu UI based on the authenticated state. To do this, I need to call useUser, but if I wrap my marketing layout in a UserProvider, I get that same 401 error message on routes that don't require authentication that @austinhale mentioned.

If I use the custom UserContext.Provider illustrated in the PR description, useUser no longer works when the user is authenticated.

A fairly specific use case I realize, and those error messages are not breaking, just annoying.

Could there be an option for the UserProvider to perform the user fetch 'just-in-time' when useUser is called instead of immediately?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
4 participants