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

fix: refetch session even if there is no current session #11075

Merged
merged 4 commits into from
Aug 4, 2024

Conversation

OrJDev
Copy link
Contributor

@OrJDev OrJDev commented Jun 4, 2024

☕️ Reasoning

I have an app that automatically sign in on the server side after signing up with credentials, trying to refetch the session (after calling the server function) without hard refreshing the page seems to be impossible, that is because no functions update the session directly except for update, however this function is only a thing when the session is already fetched and exists on the client side, that makes it so there is no possible way to refetch the session at any time i want it to.

   const session = await getSession();
                        console.log({ session });
                        // const t = toast.success("Loggin in...");
                        const r = await update();
                        console.log({ r });

In this example session is an object and r is not defined

🧢 Checklist

  • Documentation
  • Tests
  • Ready to be merged

🎫 Affected issues

📌 Resources

@OrJDev OrJDev requested a review from ThangHuuVu as a code owner June 4, 2024 19:28
Copy link

vercel bot commented Jun 4, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
auth-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 1, 2024 9:27am
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
next-auth-docs ⬜️ Ignored (Inspect) Visit Preview Aug 1, 2024 9:27am

Copy link

vercel bot commented Jun 4, 2024

@OrJDev is attempting to deploy a commit to the authjs Team on Vercel.

A member of the Team first needs to authorize it.

@OrJDev
Copy link
Contributor Author

OrJDev commented Jun 8, 2024

Bumping this

@OrJDev
Copy link
Contributor Author

OrJDev commented Aug 1, 2024

@balazsorban44 really need this one its just 1 line of changes

@OrJDev
Copy link
Contributor Author

OrJDev commented Aug 1, 2024

so currently having this:

  const s = await checkSignup.mutateAsync({
                        email,
                        password,
                        countryCode: country,
                        refUser,
                        oId: currentOffer?.id,
                      });
                      if (s) {
                        setLogin(true);
                        const s = await getSession({
                          broadcast: true,
                        });
                        console.log(s);
                        // window.location.href = "/earn";
                      } else {
                        toast.success("Account created successfully");
                        await router.push(`/auth?type=sign_in&email=${email}`);
                      }

so as you see it does print the user, however it doesn't update it client side so it looks like im signed out, that is because i signed in from the server side in order to allow automatic sign in for known users using my own logic, after refreshing the page ofc the session does seems to be correct

Screenshot 2024-08-01 at 12 30 04

Copy link

codecov bot commented Aug 1, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 40.08%. Comparing base (cd5ab90) to head (bdb1c1c).
Report is 2 commits behind head on main.

Files Patch % Lines
packages/next-auth/src/react.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #11075   +/-   ##
=======================================
  Coverage   40.08%   40.08%           
=======================================
  Files         179      179           
  Lines       28739    28739           
  Branches     1254     1260    +6     
=======================================
  Hits        11519    11519           
  Misses      17220    17220           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@ThangHuuVu ThangHuuVu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @OrJDev , I looked at the code change - I'm not 100% sure why the logic is here in the first place. IMO, it is OK to get this in, but if there's any regression I am afraid that we'll have to revert.

@ThangHuuVu ThangHuuVu merged commit 748c9ec into nextauthjs:main Aug 4, 2024
11 of 14 checks passed
@OrJDev
Copy link
Contributor Author

OrJDev commented Aug 4, 2024

Awesome thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants