search

fedeya / next-auth-sanity

NextAuth Adapter and Provider for Sanity
https://sanity.io/plugins/next-auth-sanity
MIT License
76 stars 21 forks source link

Fix: Remove duplicated signUpHandler #71

Closed anthlasserre closed 2 months ago

anthlasserre commented 2 months ago

Hey @fedeya 👋🏼

Thanks a lot for this useful plugin. So far so good except for the build today.

Summary

I was facing a few issues related to @argon2 and @mapbox/node-pre-gyp. It was running on the browser side. But I had no idea why.

Investigation

I have mainly followed the example folder and by giving a second check I might have found a duplication which can cause some troubles.

Starting NextJS 13 all things related to the server should be within /app folder instead. https://nextjs.org/docs/messages/prerender-error

Screenshot 2024-04-17 at 22 53 13

Answer

So I have basically removed examples/full-example/src/pages/api/sanity/signUp.ts because examples/full-example/src/app/api/sanity/signUp.ts is already there to cover the signUpHandler export.

Also, I'd recommend adding "next": ">= 13.0.0" in peerDependencies to be sure everyone is targeting NextJS 13 at least. What do you think ?

Issues maybe related

It can potentially solve a few issues:

fedeya commented 2 months ago

Hi @anthlasserre, i'm not sure what this fixes, because the examples folder is not being pushed to npm.

anthlasserre commented 2 months ago

Hi @anthlasserre, i'm not sure what this fixes, because the examples folder is not being pushed to npm.

No indeed. But by taking reference to the example folder on Next 13 / Next 14 it does break on build because of this duplication and by the fact that the signUpHandler should be exported under app/api instead of pages/api.

I stupidly followed the example folder to build my app and I realised this was wrong.

fedeya commented 2 months ago

Ohh ok now i get it, yes i duplicated the api route to show the example using pages or app directory, but i forgot that it can break the app

fedeya commented 2 months ago

About the question i think it is not necessary because the library should work with a version lower than 13

anthlasserre commented 2 months ago

This is what I thought, no worries mate. Got you 👍🏼 Thanks for your quick responses! Hopefully, no one else has made the same mistake as me. 😅

fedeya commented 2 months ago

Thanks for contributing!