gladly-team / next-firebase-auth

Simple Firebase authentication for all Next.js rendering strategies
https://nfa-example-git-v1x-gladly-team.vercel.app/
MIT License
1.35k stars 290 forks source link

Feature Request: Offer Auth Provider #64

Closed lukebennett88 closed 3 years ago

lukebennett88 commented 3 years ago

Is your feature request related to a problem? Please describe. I'm looking at adding next-firebase-auth to a site I'm working on, but it has a persistent nav which shows the login status.

Unless I'm missing something, I would have to wrap every single page with the withAuthUser() HOC, as well as call the useAuthUser function in each of those pages. I would also need to move my Layout component out of my _app and into every page, meaning even more duplication on every page.

Describe the solution you'd like and how you'd implement it A React Context Provider could be exposed so that developer can wrap their _app component with this allowing them to call the useAuthUser hook from any nested component.

Is this a breaking change? I'm not sure if this would be a breaking change, I'm pretty sure this could be achieved without breaking the useAuthUser() API, but I haven't tried so I'm not sure.

Describe alternatives you've considered Since this functionality does appear to exist, the only way I can think to achieve this is to not use next-firebase-auth at all, and instead write the functionality myself.

Thanks for your consideration.

tlays commented 3 years ago

Hi @lukebennett88, it's an interesting request, and a good idea. In one of my projects I had actually built this feature myself base on this example which I had enhanced to lazy-load some components. However in the end, I scrapped all my code and came back to next-firebase-auth (regardless of the "duplication" of withAuthUser()) because it was slowing down the performance of some of my pages.

The biggest problem with nextJs and adding an AuthProvider at the root of your application (in _app) is that it is loaded on all pages, all the time ... which means the full firebase-client (at minimum) module is always loaded (even on pages that don't use/need it). This slows down the performance of your pages (due to the extra kb, and JS processing time). Of course you can always play with lazy-loading modules, but no matter what you do, you still end up with some additional JS bundles. Personally, I like keeping my _app as clean and light as possible.

Now if the app has a persistent login status in a permanent navbar (like yours), it's less of a problem, obviously. But for other sites/apps that don't want to extra bytes loaded on every page, it's a pain.

Now I'm not saying that next-firebase-auth cannot be enhanced to offer an AuthProvider for those that don't mind the extra bytes loaded all the time... I'm just not sure if it's worth it. Maybe a custom-built solution like the one I mentioned above is better for you ? Or you could also build the AuthProvider yourself on top of next-firebase-auth if you think it's worth the time (vs copy/pasting withAuthUser in your pages).

kmjennison commented 3 years ago

@lukebennett88 Thanks for the feature request.

If you're okay with using the Firebase user loaded only on the client side, you can wrap a component in withAuthUser and use that component in the _app.js structure. Then, you should be able to use useAuthUser once in your persistent nav bar component. Does this work in your case?

If you're hoping to include the Firebase user in _app.js during SSR, it's possible using getInitialProps and custom auth code, but I'm inclined to agree with @tlays that it's not a pattern we'd want to encourage because it disables static optimization for the whole app. Instead, I'd recommend creating common HOC and getServerSideProps wrappers for your pages.

Off the top of my head, here's another somewhat hacky approach you could take: in _app.js, pass the "AuthUserSerialized" page prop to the withAuthUser-wrapped component that's also rendered in _app.js. This should support SSR when the page includes withAuthUserTokenSSR but won't disable static optimization for the whole app, and the AuthUser should be available in the persistent nav bar component. (This relies on the next-firebase-auth internal prop name.)

lukebennett88 commented 3 years ago

I still think a provider might be a good thing to offer as some people might need it (but discourage people from using it in the README).

Until Next has support from running getStaticProps etc in _app, or an alternative for statically rendering external data sources, I think people are going to want to at least have the option to use global data, as you can easily end up with quite a lot of boilerplate code that you will otherwise need to add to every page.

Having said that, I've done a bit of thinking and I agree with you @tlays — it's not worth loading up Firebase on every page. I'm going to refactor how my nav works instead.

Thanks so much for your suggestions and detailed responses, I really appreciate it 😊

samos123 commented 3 years ago

I am pretty much in the same boat. I have a component called Layout that is loaded in _app.js. The layout component has the following navigation buttons:

        {AuthUser ? (
          <Button variant="primary" onClick={AuthUser.signOut}>
            Logout
          </Button>
        ) : (
          <SigninButton/>
        )}

I was able to get it working by wrapping the Layout component like this:

export default withAuthUser()(Layout)
kmjennison commented 3 years ago

I'm going to close this and aim to have this package follow Next.js's features/recommendations for data fetching. Right now, Next doesn't support getStaticProps or getServerSideProps in _app.js, though it likely eventually will.

dylangolow commented 3 years ago

I found this discussion after seeing a similar issue that @lukebennett88 mentioned with having to copy layouts between components and then add withAuthUser to each default export.

My solution was to have a LayoutWithNav component that gets set like this:

// SomePage.tsx

import LayoutWithNav from "../LayoutWithNav"

const SomePage = () => { ...details using useAuthUser() hook }

SomePage.Layout = LayoutWithNav;

export default withAuthUser()(SomePage);

LayoutWithNav looks like this:

// LayoutWithNav.tsx

const LayoutWithNav: React.FC = ({children}: Props) => {

  return (
          <Wrapper>
              <Nav hide={false} />
              <ContentWrap>
                  <Header />
                  <Content>{children}</Content>
              </ContentWrap>
          </Wrapper>
      );
}

export default withAuthUser()(LayoutWithNav);

This solved my issues so that I can use the useAuthUser() hook anywhere I add that LayoutWithNav to a page. Sharing here in case someone else comes across this issue and could use a concrete example.