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

feat: add fastify example #1

Merged
merged 2 commits into from
Oct 1, 2024
Merged

feat: add fastify example #1

merged 2 commits into from
Oct 1, 2024

Conversation

songkeys
Copy link

The code works good in my machine!

I just finished the example in /dev fold. Pretty much a direct copy paste of your work tbh.

And I noticed reply.session should actually be typed in the plugin itself rather than the user end?

apps/examples/fastify/src/config/auth.config.ts Outdated Show resolved Hide resolved
@@ -141,6 +130,12 @@ import type { FastifyRequest, FastifyPluginAsync } from "fastify"
import formbody from "@fastify/formbody"
import { toWebRequest, toFastifyReply } from "./lib/index.js"

declare module "fastify" {
Copy link
Owner

Choose a reason for hiding this comment

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

I'll check with the maintainers but I think we probably should leave the session management up to the user given we are having them define their own auth prehandlers. nextauthjs#9587 (comment)

Copy link
Author

Choose a reason for hiding this comment

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

Ah, I overlooked the part where users need to define the session decorator themselves in their prehandlers. I've reverted that in the next commit.

However, I believe many Fastify plugin libraries within its ecosystem actually define the decorators themselves. For instance, take a look at https://github.com/fastify/fastify-passport. Users can create the auth middleware using a helper from the library:

server.get(
  '/',
  { preValidation: fastifyPassport.authenticate('test', { authInfo: false }) },
  async () => 'hello world!'
)

That said, this could result in duplicated names for decorator properties: fastify/fastify#1955 . But I suppose that's just how things work in the Fastify ecosystem. We can stick to the current design though.

Copy link
Owner

Choose a reason for hiding this comment

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

nextauthjs#9587 (comment) Yeah I agree it would be simpler for users but they want to keep it flexible which is also fair.

apps/dev/fastify/src/config/auth.config.ts Outdated Show resolved Hide resolved
apps/dev/fastify/views/protected.pug Outdated Show resolved Hide resolved
@songkeys
Copy link
Author

songkeys commented Oct 1, 2024

@hillac Hi! Could you please take another look? I've corrected the issues. Some mistakes are made because I copied files for the whole auth config from the express folder without double-checking after completing testing.

@hillac hillac merged commit 4a404b3 into hillac:main Oct 1, 2024
1 check passed
hillac pushed a commit that referenced this pull request Oct 1, 2024
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