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

Inconsistent session returns after 3.8.0 #1461

Closed
2 of 5 tasks
theycallmeswift opened this issue Mar 6, 2021 · 11 comments · Fixed by #1462
Closed
2 of 5 tasks

Inconsistent session returns after 3.8.0 #1461

theycallmeswift opened this issue Mar 6, 2021 · 11 comments · Fixed by #1462
Labels
bug Something isn't working priority Priority fix or enhancement

Comments

@theycallmeswift
Copy link

Describe the bug

A bug was introduced starting in 3.8.0 that prevents the session from being read multiple times. If you have multiple components that rely on the useSession hook, only the first one will return a session object as expected.

Steps to reproduce

You can introduce this behavior to the next-auth-example with the following diff. Changes added to pages/index.js are for easy visibility.

diff --git a/package.json b/package.json
index c305f4a..9b0f580 100644
--- a/package.json
+++ b/package.json
@@ -21,7 +21,7 @@
   "license": "ISC",
   "dependencies": {
     "next": "^10.0.7",
-    "next-auth": "^3.6.0",
+    "next-auth": "^3.8.0",
     "react": "^17.0.1",
     "react-dom": "^17.0.1",
     "sqlite3": "^5.0.2"
diff --git a/pages/index.js b/pages/index.js
index f4aa3e1..ec76748 100644
--- a/pages/index.js
+++ b/pages/index.js
@@ -1,12 +1,17 @@
 import Layout from '../components/layout'

+import { signIn, signOut, useSession  } from "next-auth/client";
+
 export default function Page () {
+  const [session, loading] = useSession();
+
   return (
     <Layout>
       <h1>NextAuth.js Example</h1>
       <p>
         This is an example site to demonstrate how to use <a href={`https://next-auth.js.org`}>NextAuth.js</a> for authentication.
       </p>
+      <pre>{JSON.stringify(session?.user || {}, 2)}</pre>
     </Layout>

Expected behavior

image

Note: This works as expected with 3.7.1 and below.

Actual behavior

image

Note: The session works as expected for the header but returns an empty object the second time the hook is used.

Feedback
Documentation refers to searching through online documentation, code comments and issue history. The example project refers to next-auth-example.

  • Found the documentation helpful
  • Found documentation but was incomplete
  • Could not find relevant documentation
  • Found the example project helpful
  • Did not find the example project helpful
@theycallmeswift theycallmeswift added the bug Something isn't working label Mar 6, 2021
@balazsorban44 balazsorban44 added the priority Priority fix or enhancement label Mar 6, 2021
@balazsorban44
Copy link
Member

Interesting, I unfortunately managed to reproduce it and I'll try to identify the source of the problem! Thanks for reporting!

@theycallmeswift
Copy link
Author

My best guess is that one of the build dependencies changed between 3.7.1 and 3.8.0 and the transpiler introduced a new bug as a result.

Check out the diff between 3.7.1 and the actual packages:

@balazsorban44
Copy link
Member

balazsorban44 commented Mar 6, 2021

I am pretty sure this was introduced in 3aee24b. Made some significant changes to clean things up and re-use the session object if it was passed. It might have had unforeseen consequences. I am afraid that I did not fix #1084 after all..
But I am on it, will keep you posted!

UPDATES:

  • switching out the content of src/client/index.js to pre 3.8.0 worked. So I pinpointed the exact commit that introduced this issue. I am now going to see if I can push a fix, or I have to revert that commit.

@github-actions
Copy link

github-actions bot commented Mar 6, 2021

🎉 This issue has been resolved in version 3.10.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@balazsorban44
Copy link
Member

balazsorban44 commented Mar 6, 2021

So this should be fixed now in 3.10.1. Unfortunately, it was a dirty fix, reverting my optimization. We might have to wait to make the optimization until a major release, as I think we have to reorganize some code and integrate it with React more tightly.

@theycallmeswift could you confirm that this has indeed been fixed for now?

@avisra
Copy link

avisra commented Mar 6, 2021

omg lol I just uprooted my entire application trying to figure out what I broke and realized that it was 3.10.0 that caused it. so glad I stumbled upon this post. Any side effects to the dirty fix you mention?

@balazsorban44
Copy link
Member

balazsorban44 commented Mar 6, 2021

Not really, it was an optimization that did not work as intended. Reverted to how it worked before. In short, if you pass session to Provider, currently next-auth will still make a new request to /api/auth/session to get the session you just passed to it. 😄. In most cases it's probably negligible (as this is how nextauth has worked until recently). To get around this though, we have to tie the client logic closer to react, and make more use of the Provider context. This would mean that current users who don't use Provider in their apps would lose some functionality. This is a breaking change, so this fix might have to wait until the next major release. I already have a working prototype locally though, might be able to put it into a PR in a few days.

UPDATE: Here is my PR #1473

@filipesmedeiros
Copy link

Hey! First: love the work!

I'm still having this issue @3.23.3

@nextauthjs nextauthjs deleted a comment from github-actions bot May 26, 2021
@balazsorban44
Copy link
Member

Please open a new issue, with a reproduction.

@filipesmedeiros
Copy link

filipesmedeiros commented May 27, 2021

@balazsorban44

https://github.com/filipesmedeiros/nano-medium-poc/tree/show-next-auth-bug

This branch has the bug working. If you login using Github (sorry to force this, can I make it like a mock session? If you don't want to login), and then go to the "New Post" page, when you press "Publish" it will fail with session === undefined (a null pointer or wtv).

You can see in the source that session is being read once in the layout and once in that page.

If you go to the main branch, this is fixed app-side by adding a context above the _app where I just take the useSession() value and pass it down manually. This causes the issue to disappear.

Let me know if you need anything else!

EDIT: Btw, I'm running it with yarn && yarn dev

@balazsorban44
Copy link
Member

I am not convinced this is still an issue. Could you please open a completely new issue with your bug report and a detailed explanation instead of a new comment here? Thanks!

balazsorban44 added a commit that referenced this issue Jun 11, 2021
**What**:

These changes ensure that we work more tightly with React that can also result in unforeseen performance boosts. In case we would decide on expanding to other libraries/frameworks, a new file per framework could be added.

**Why**:

Some performance issues (#844) could only be fixed by moving more of the client code into the `Provider`.

**How**:

Refactoring `next-auth/client`

Related: #1461, #1084, #1462

BREAKING CHANGE:
**1.** `next-auth/client` is renamed to `next-auth/react`.

**2.** In the past, we exposed most of the functions with different names for convenience. To simplify our source code, the new React specific client code exports only the following functions, listed with the necessary changes:

- `setOptions`: Not exposed anymore, use `SessionProvider` props
- `options`: Not exposed anymore, use `SessionProvider` props
- `session`: Rename to `getSession`
- `providers`: Rename to `getProviders`
- `csrfToken`: Rename to `getCsrfToken`
- `signin`: Rename to `signIn`
- `signout`: Rename to `signOut`
- `Provider`: Rename to `SessionProvider`

**3.** `Provider` changes.
- `Provider` is renamed to `SessionProvider`
- The `options` prop is now flattened as the props of `SessionProvider`.
- `clientMaxAge` has been renamed to `staleTime`.
- `keepAlive` has been renamed to `refetchInterval`.
An example of the changes:
```diff
- <Provider options={{clientMaxAge: 0, keepAlive: 0}}>{children}</Provider>
+ <SessionProvider staleTime={0} refetchInterval={0}>{children}</SessionProvider> 
```

**4.** It is now **required** to wrap the part of your application that uses `useSession` into a `SessionProvider`.

Usually, the best place for this is in your `pages/_app.jsx` file:

```jsx
import { SessionProvider } from "next-auth/react"

export default function App({
  Component,
  pageProps: { session, ...pageProps }
}) {
  return (
    // `session` comes from `getServerSideProps` or `getInitialProps`.
    // Avoids flickering/session loading on first load.
    <SessionProvider session={session}>
      <Component {...pageProps} />
    </SessionProvider>
  )
}
```
mnphpexpert added a commit to mnphpexpert/next-auth that referenced this issue Sep 2, 2024
**What**:

These changes ensure that we work more tightly with React that can also result in unforeseen performance boosts. In case we would decide on expanding to other libraries/frameworks, a new file per framework could be added.

**Why**:

Some performance issues (nextauthjs#844) could only be fixed by moving more of the client code into the `Provider`.

**How**:

Refactoring `next-auth/client`

Related: nextauthjs#1461, nextauthjs#1084, nextauthjs#1462

BREAKING CHANGE:
**1.** `next-auth/client` is renamed to `next-auth/react`.

**2.** In the past, we exposed most of the functions with different names for convenience. To simplify our source code, the new React specific client code exports only the following functions, listed with the necessary changes:

- `setOptions`: Not exposed anymore, use `SessionProvider` props
- `options`: Not exposed anymore, use `SessionProvider` props
- `session`: Rename to `getSession`
- `providers`: Rename to `getProviders`
- `csrfToken`: Rename to `getCsrfToken`
- `signin`: Rename to `signIn`
- `signout`: Rename to `signOut`
- `Provider`: Rename to `SessionProvider`

**3.** `Provider` changes.
- `Provider` is renamed to `SessionProvider`
- The `options` prop is now flattened as the props of `SessionProvider`.
- `clientMaxAge` has been renamed to `staleTime`.
- `keepAlive` has been renamed to `refetchInterval`.
An example of the changes:
```diff
- <Provider options={{clientMaxAge: 0, keepAlive: 0}}>{children}</Provider>
+ <SessionProvider staleTime={0} refetchInterval={0}>{children}</SessionProvider> 
```

**4.** It is now **required** to wrap the part of your application that uses `useSession` into a `SessionProvider`.

Usually, the best place for this is in your `pages/_app.jsx` file:

```jsx
import { SessionProvider } from "next-auth/react"

export default function App({
  Component,
  pageProps: { session, ...pageProps }
}) {
  return (
    // `session` comes from `getServerSideProps` or `getInitialProps`.
    // Avoids flickering/session loading on first load.
    <SessionProvider session={session}>
      <Component {...pageProps} />
    </SessionProvider>
  )
}
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority Priority fix or enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants