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: Remove duplicated signUpHandler #71

Merged
merged 1 commit into from
Apr 17, 2024
Merged

Conversation

anthlasserre
Copy link
Contributor

@anthlasserre anthlasserre commented Apr 17, 2024

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
Copy link
Owner

fedeya commented Apr 17, 2024

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

@anthlasserre
Copy link
Contributor Author

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
Copy link
Owner

fedeya commented Apr 17, 2024

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
Copy link
Owner

fedeya commented Apr 17, 2024

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

@anthlasserre
Copy link
Contributor Author

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 fedeya merged commit 33f73ee into fedeya:main Apr 17, 2024
2 checks passed
@fedeya
Copy link
Owner

fedeya commented Apr 17, 2024

Thanks for contributing!

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

Successfully merging this pull request may close these issues.

2 participants