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

Nhost Auth 1.0 #1725

Merged
merged 23 commits into from
Feb 22, 2021
Merged

Nhost Auth 1.0 #1725

merged 23 commits into from
Feb 22, 2021

Conversation

nunopato
Copy link
Contributor

@nunopato nunopato commented Feb 4, 2021

Adding Nhost as an authentication provider:

  • Adds Nhost as a authClient
  • Adds Nhost to the CLI tool

This PR will be updated and finalized in the next couple of days since we have some minor work to do on our SDK.

Some relevant links if you want to know more about Nhost:

Feedback much appreciated!

@thedavidprice
Copy link
Contributor

This is amazing @nunopato 🚀 Thank you for getting this started.

I'm setting this as a Draft release. When things are ready for review, just change the status and let us know.

Looping in @dthyresson and @Tobbe, either who can help run point of contact for this.

Lastly, there are two things that could be great next steps beyond this PR:

  • could you highlight and talk about https://nhost.io in the Redwood Forums? Would be great to know about hosting options for Redwood. Bonus if you have an example Redwood site running on Nhost! https://community.redwoodjs.com
  • what about adding official deploy target support for Nhost? The DX would be 1) run yarn rw setup deploy-nhost and then 2) yarn rw deploy nhost. Assuming you integrate with GitHub, we could support the effort on your end to build and deploy respective components to CDN and Server. If you're interested, please start a forum post or GH issue and loop me in.

@thedavidprice thedavidprice marked this pull request as draft February 4, 2021 21:35
@Tobbe
Copy link
Member

Tobbe commented Feb 5, 2021

Thanks for the PR @nunopato. Great start on the auth client!

Looking at the code I'm missing decoder stuff. Can you please take a look at redwood\packages\api\src\auth\decoders\index.ts and see what would be needed for Nhost Auth?

Also please fix the lint errors and then I'll have another look at the code.

Thanks again for getting this started 🙂

@nunopato
Copy link
Contributor Author

nunopato commented Feb 6, 2021

@thedavidprice @Tobbe thank you for your comments.

could you highlight and talk about https://nhost.io in the Redwood Forums? Would be great to know about hosting options for Redwood. Bonus if you have an example Redwood site running on Nhost! https://community.redwoodjs.com

Will do! I actually built a very simple example app with the changes from this PR + a Nhost project (Auth+GraphQL) in production. Everything seems to be working just fine.

what about adding official deploy target support for Nhost? The DX would be 1) run yarn rw setup deploy-nhost and then 2) yarn rw deploy nhost. Assuming you integrate with GitHub, we could support the effort on your end to build and deploy respective components to CDN and Server. If you're interested, please start a forum post or GH issue and loop me in.

I assume that by "server components" you mean functions specified in ./api/src/functions, correct?

We support functions (we call it custom API) written in JavaScript/TypeScript and the example app I built has no ./api directory. I am simply pointing the apiProxyPath config to the GraphQL API on Nhost. Let's discuss further how this deployment would look like.

Can you please take a look at redwood\packages\api\src\auth\decoders\index.ts and see what would be needed for Nhost Auth?

Done.

The build is failing because I still need to release a new version (currently in preview) of our SDK. I will do that during the weekend.

Thank you again, really appreciate it.

@github-actions
Copy link

github-actions bot commented Feb 8, 2021

📦 PR Packages

Click to Show Package Download Links

https://rw-pr-redwoodjs-com.s3.amazonaws.com/1725/create-redwood-app-0.25.1-a7222d3.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1725/redwoodjs-api-0.25.1-a7222d3.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1725/redwoodjs-api-server-0.25.1-a7222d3.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1725/redwoodjs-auth-0.25.1-a7222d3.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1725/redwoodjs-cli-0.25.1-a7222d3.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1725/redwoodjs-core-0.25.1-a7222d3.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1725/redwoodjs-dev-server-0.25.1-a7222d3.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1725/redwoodjs-eslint-config-0.25.1-a7222d3.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1725/redwoodjs-eslint-plugin-redwood-0.25.1-a7222d3.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1725/redwoodjs-forms-0.25.1-a7222d3.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1725/redwoodjs-internal-0.25.1-a7222d3.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1725/redwoodjs-prerender-0.25.1-a7222d3.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1725/redwoodjs-router-0.25.1-a7222d3.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1725/redwoodjs-structure-0.25.1-a7222d3.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1725/redwoodjs-testing-0.25.1-a7222d3.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1725/redwoodjs-web-0.25.1-a7222d3.tgz

Install this PR by running yarn rw upgrade --pr 1725:0.25.1-a7222d3

@nunopato nunopato marked this pull request as ready for review February 8, 2021 11:54
@nunopato
Copy link
Contributor Author

nunopato commented Feb 9, 2021

@thedavidprice @Tobbe just wanted to let you know that I have marked the PR as ready. An example app can be found here https://github.com/nhost-examples/nhost-redwoodjs and I will post something to the community forum soon depending on your feedback.

@Tobbe
Copy link
Member

Tobbe commented Feb 10, 2021

@nunopato Great work! I'll try to take a look tonight.

@Tobbe
Copy link
Member

Tobbe commented Feb 11, 2021

Could you also please write a PR for https://github.com/redwoodjs/redwoodjs.com/blob/main/docs/authentication.md, adding instructions for Nhost?

And we also try to show off our auth implementations in our auth playground, so if you could get an example running there as well, that'd be great :) https://github.com/redwoodjs/playground-auth

@Tobbe
Copy link
Member

Tobbe commented Feb 11, 2021

Ohh, and there's also README.md in the auth package. At the end there's a list of all providers, please add nhost there as well

@nunopato
Copy link
Contributor Author

@Tobbe PRs for the playground and redwoodjs.com (docs) can be found here: https://github.com/redwoodjs/redwoodjs.com/pull/562 & redwoodjs/playground-auth#20. The README.md for the Auth package is also updated. Let me know if there is anything else missing.

* Log In via a OAuth provider also registers the account in case it doesn't exist
* @param options.email The user's email address
* @param options.password The user's password
* @param options.provider One of NhostProvider
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @param options.provider One of NhostProvider
* @param {NHostProvider} options.provider The authentication provider to use

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could improve the typings here. I'm not 100% sure of this syntax, but maybe this would work: https://jsdoc.app/tags-typedef.html

Copy link
Contributor

Choose a reason for hiding this comment

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

Also my comment for the type is lame.

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.

None yet

5 participants