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: use upgraded require optional #1743

Merged
merged 2 commits into from
Apr 16, 2021
Merged

Conversation

balazsorban44
Copy link
Member

What:

Having another stab at #1682

Why:

The end goal is to not bundle/require typeorm and nodemailer, if someone doesn't use any of these in their project.

How:

I took require_optional as a base, but published my own version under @balazsorban/require-optional. The difference is that it will use the now seemingly established fields peerDependencies and peerDependenciesMeta, instead of peerOptionalDependencies, which doesn't appear to be anywhere. In fact, googling it will point to one of this repository's issue with mongodb!

Side note, mongodb themselves decided to turn away from peerOptionalDependencies: https://jira.mongodb.org/browse/NODE-2867

Checklist:

  • Documentation
  • Tests
  • Ready to be merged

@vercel
Copy link

vercel bot commented Apr 16, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/nextauthjs/next-auth/964cDCS73H27Y5dRF62r8hMmRMXu
✅ Preview: https://next-auth-git-fix-upgraded-require-optional-nextauthjs.vercel.app

@github-actions github-actions bot added adapters Changes related to the core code concerning database adapters core Refers to `@auth/core` providers labels Apr 16, 2021
@balazsorban44 balazsorban44 merged commit a6f5f4c into next Apr 16, 2021
@balazsorban44 balazsorban44 deleted the fix/upgraded-require-optional branch April 16, 2021 14:05
@github-actions
Copy link

🎉 This PR is included in version 4.0.0-next.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ndom91
Copy link
Member

ndom91 commented Apr 17, 2021

@balazsorban44 Yeah strange, right? I couldn't find peerOptionalDependencies referenced anywhere online really. Not sure where that originated from haha.

Anyway, is your require-optional fork doing the trick?

@balazsorban44
Copy link
Member Author

balazsorban44 commented Apr 17, 2021

peerOptionalDependencies comes from the require_optional package. It was written BEFORE peerDependenciesMeta was a thing. And no, unfortunately my solution did not work. Searching for peerDependenciesMeta up in the node_modules tree stopped looking for nodemailer in my next's package.json.. 😵 I start to think that the best option is to drop the database config, asking the user to explicitly choose a adapter, and use the form next-auth/providers/your-provider for importing built-in providers. (wrote about this here as well: #1682 (comment)). These two things would mean that we don't need any additional package for requiring optionally. Having imports like this would make it explicit and opt-in which packages you will have to use. I could use your help @ndom91 with testing.

mnphpexpert added a commit to mnphpexpert/next-auth that referenced this pull request Sep 2, 2024
* chore(deps): switch back to (updated) require_optional

* fix: use @balazsorban/require-optional
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
adapters Changes related to the core code concerning database adapters core Refers to `@auth/core` providers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants