-
Notifications
You must be signed in to change notification settings - Fork 3
Template suggestions #6
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
Conversation
app/layout.tsx
Outdated
| }: Readonly<{ | ||
| children: React.ReactNode; | ||
| }>) { | ||
| const expectAuth = !!(await withAuth()).user |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love this refactor! Super clear.
One thing to point out is the call to withAuth here causes all children of the layout to be dynamic routes, removing the option for the more performant static routes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great point, seems like we should be declaring routes/component trees as expecting auth instead of asking whether the user is authed to decide.
Ideally <Authenticated> could take care of this: <Authenticated> could render its children during SSR and on hydration when authed, or if we can't have that access token available (related issues) then at least know that auth is coming, and we can do a more localized version of expectAuth: true.
@nicknisi any plans to be able to check on the first render whether a route is authed, or more specifically whether an accessToken is coming?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like we want grouped routes here so we can use different layouts here, yea? app/(authed)/layout.tsx so it can be server-rendered
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any plans to be able to check on the first render whether a route is authed, or more specifically whether an accessToken is coming?
@thomasballinger wondering if the changes in workos/authkit-nextjs#297 would help to simplify this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thomasballinger After some testing I made some minor tweaks in cd7d9a5 and upgraded to the latest authkit-nextjs in 1dc0978. I pushed them up to this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
| const preloaded = await preloadQuery(api.myFunctions.listNumbers, { | ||
| count: 3, | ||
| }); | ||
| const { accessToken } = await withAuth(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this call to withAuth essentially instant since we already authenticated in the middleware.ts? Or does this make a server round trip, meaning we have a round trip in middleware followed by a second round trip here (on an already authenticated route)?
If it is the latter, would it make sense for us to switch the middleware.ts to page based auth instead of the current middleware auth? Middleware documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great question! the withAuth() call is essentially instant. It reads the session from a header that the middleware already set. There's no additional authentication or API call happening.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooh sweet. Thanks for the info!
Uh oh!
There was an error while loading. Please reload this page.