Skip to content

feat: add fastify example#1

Merged
hillac merged 2 commits intohillac:mainfrom
songkeys:main
Oct 1, 2024
Merged

feat: add fastify example#1
hillac merged 2 commits intohillac:mainfrom
songkeys:main

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?

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.

@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
hillac added 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