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(adapter): add @auth/postgresjs-adapter #11200

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

mustaqimarifin
Copy link

☕️ Reasoning

Adding a new adapter using Postgres.js.

🧢 Checklist

  • Documentation
  • Tests
  • Ready to be merged

🎫 Affected issues

📌 Resources

Copy link

vercel bot commented Jun 18, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
auth-docs ❌ Failed (Inspect) Aug 2, 2024 3:00pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
next-auth-docs ⬜️ Ignored (Inspect) Visit Preview Aug 2, 2024 3:00pm

Copy link

vercel bot commented Jun 18, 2024

Someone is attempting to deploy a commit to the authjs Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions bot added the adapters Changes related to the core code concerning database adapters label Jun 18, 2024
Copy link

socket-security bot commented Jun 18, 2024

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

View full report↗︎

@ndom91
Copy link
Member

ndom91 commented Jun 20, 2024

Theres already an open PR adding an adapter using postgres.js (see: #10570).

Ideally you two could sync and work on just 1 PR together 🙏

EDIT: Looks like that other user, @CodyBontecou, closed his PR in favor of this one

@mustaqimarifin
Copy link
Author

Theres already an open PR adding an adapter using postgres.js (see: #10570).

Ideally you two could sync and work on just 1 PR together 🙏

EDIT: Looks like that other user, @CodyBontecou, closed his PR in favor of this one

hello! sounds good. are there any pending changes i need to make?

@ndom91
Copy link
Member

ndom91 commented Jun 24, 2024

Theres already an open PR adding an adapter using postgres.js (see: #10570).

Ideally you two could sync and work on just 1 PR together 🙏

EDIT: Looks like that other user, @CodyBontecou, closed his PR in favor of this one

hello! sounds good. are there any pending changes i need to make?

Is that _test.txt file not supposed to be a shell script instead? But otherwise looks alright at first glance.

Give us some time to look over things. We're a bit swamped getting in the qwik, nuxt and other client libs atm

@mustaqimarifin
Copy link
Author

Theres already an open PR adding an adapter using postgres.js (see: #10570).
Ideally you two could sync and work on just 1 PR together 🙏
EDIT: Looks like that other user, @CodyBontecou, closed his PR in favor of this one

hello! sounds good. are there any pending changes i need to make?

Is that _test.txt file not supposed to be a shell script instead? But otherwise looks alright at first glance.

Thanks for catching that... sorted!

Give us some time to look over things. We're a bit swamped getting in the qwik, nuxt and other client libs atm

no worries 😎

Copy link

socket-security bot commented Jul 21, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher

🚮 Removed packages: npm/@types/set-cookie-parser@2.4.7, npm/@types/uuid@8.3.4, npm/@typescript-eslint/parser@v6.19.1, npm/dotenv@10.0.0, npm/nodemailer@6.9.14

View full report↗︎

Copy link

codecov bot commented Jul 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (main@089566f). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #11200   +/-   ##
=======================================
  Coverage        ?   97.59%           
=======================================
  Files           ?        6           
  Lines           ?     1289           
  Branches        ?      142           
=======================================
  Hits            ?     1258           
  Misses          ?       31           
  Partials        ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


runBasicTests({
adapter: PostgresJSAdapter(sql),
db: {
Copy link
Member

@ndom91 ndom91 Jul 21, 2024

Choose a reason for hiding this comment

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

It looks like you've added the Authenticator (PassKeys) methods, but didn't add the tests.

Can you add the testWebAuthnMethods: true option here and the db: authenticator() .. method? Check out the Prisma index.test.ts as an example

Copy link
Author

Choose a reason for hiding this comment

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

It looks like you've added the Authenticator (PassKeys) methods, but didn't add the tests.

Can you add the testWebAuthnMethods: true option here and the db: authenticator() .. method? Check out the Prisma index.test.ts as an example

Alright.. ive added it!

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants