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.

Sign up for GitHub

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

Jump to bottom

Add auth remix #810

Merged

diksipav merged 19 commits into master from add-auth-remix

Jan 3, 2024

Contributor

diksipav commented Dec 14, 2023

No description provided.

jaclarke reviewed

View reviewed changes

packages/auth-remix/package.json Outdated Show resolved Hide resolved

jaclarke reviewed

View reviewed changes

packages/auth-remix/package.json Outdated Show resolved Hide resolved

jaclarke reviewed

View reviewed changes

packages/auth-remix/src/client.ts Show resolved Hide resolved

jaclarke reviewed

View reviewed changes

packages/auth-remix/src/client.ts Outdated Show resolved Hide resolved

jaclarke reviewed

View reviewed changes

packages/auth-remix/src/server.ts Outdated Show resolved Hide resolved

jaclarke reviewed

View reviewed changes

packages/auth-remix/src/server.ts Show resolved Hide resolved

jaclarke reviewed

View reviewed changes

packages/auth-remix/src/server.ts Outdated Show resolved Hide resolved

jaclarke reviewed

View reviewed changes

packages/auth-remix/src/server.ts Outdated Show resolved Hide resolved

jaclarke reviewed

View reviewed changes

packages/auth-remix/src/server.ts Outdated Show resolved Hide resolved

jaclarke reviewed

View reviewed changes

packages/auth-remix/src/server.ts Outdated Show resolved Hide resolved

jaclarke reviewed

View reviewed changes

packages/auth-remix/src/server.ts Outdated Show resolved Hide resolved

jaclarke reviewed

View reviewed changes

packages/auth-remix/src/server.ts Show resolved Hide resolved

jaclarke reviewed

View reviewed changes

packages/auth-remix/src/server.ts Outdated Show resolved Hide resolved

jaclarke reviewed

View reviewed changes

packages/auth-remix/src/server.ts Outdated Show resolved Hide resolved

jaclarke reviewed

View reviewed changes

packages/auth-remix/src/server.ts Outdated Show resolved Hide resolved

jaclarke reviewed

View reviewed changes

packages/auth-remix/src/server.ts Outdated Show resolved Hide resolved

jaclarke reviewed

View reviewed changes

packages/auth-remix/src/server.ts Outdated Show resolved Hide resolved

jaclarke reviewed

View reviewed changes

packages/auth-remix/src/server.ts Outdated Show resolved Hide resolved

jaclarke reviewed

View reviewed changes

packages/auth-remix/src/server.ts Outdated Show resolved Hide resolved

packages/auth-remix/src/server.ts Outdated Show resolved Hide resolved

packages/auth-remix/src/server.ts Outdated Show resolved Hide resolved

jaclarke approved these changes

View reviewed changes

scotttrinh approved these changes

View reviewed changes

Collaborator

scotttrinh left a comment

Readme looks good! Small suggestion around the name fallback.

packages/auth-remix/readme.md Outdated Show resolved Hide resolved

packages/auth-remix/readme.md Outdated Show resolved Hide resolved

jaclarke reviewed

View reviewed changes

packages/auth-remix/readme.md Outdated Show resolved Hide resolved

jaclarke reviewed

View reviewed changes

packages/auth-remix/readme.md Outdated Show resolved Hide resolved

jaclarke reviewed

View reviewed changes

packages/auth-remix/src/server.ts Outdated Show resolved Hide resolved

jaclarke reviewed

View reviewed changes

packages/auth-remix/readme.md Outdated Show resolved Hide resolved

jaclarke reviewed

View reviewed changes

packages/auth-remix/readme.md Outdated

+                const isLoggedIn = await session.isLoggedIn();
+                const username = isLoggedIn
+                  ? (await session.client.querySingle<string>(
+                      `select global currentUser.name`

Member

jaclarke Dec 20, 2023

I know this is just an example, but could it be confusing to use a custom global here? since currentUser isn't part of the auth ext schema, but looks like it could be?

Member

jaclarke Dec 20, 2023

Also presumably in this case, checking isLoggedIn first would be redundant, as currentUser would be null if not signed in.

jaclarke reviewed

View reviewed changes

packages/auth-remix/readme.md Outdated Show resolved Hide resolved

jaclarke reviewed

View reviewed changes

packages/auth-remix/readme.md Outdated Show resolved Hide resolved

diksipav force-pushed the add-auth-remix branch from 0422098 to 6b872ec Compare

January 2, 2024 21:23


          Initialise auth-remix project

b108944

diksipav added 17 commits

January 2, 2024 15:27


          Add basic auth flow handlers

cb865fa


          Add oath route handlers

3af6a48


          Add overloads to emailPasswordSignIn method def

7a149b5


          Refactor & reuse handleAction func in server actions


          Fix signout method return type

8cc0dd2


          Refactor & remove headers from auth handler callbacks

1175a19


          Refactor & cleanup server & client files

ddbb400

Remove createUser func, should be in the client app.
Clean package.json.
Update signout func definitions.


          Refactor server file

efac5bf

Use npm cookie library for cookies.
Update callback helper functions' names.
Refactor parseCookies function to use cookies lib.


          Update emailPasswordResendVerificationEmail func declarations

6f839d9


          Use curly braces with if statement


          Enable default exports from client and server

511e190


          Add path=/ to every Set-Cookie header

91daab6


          Add readme

7b3a991


          Transform package to use module node16 with ESM modules

01c9b36


          Update readme, replace isLoggedIn with isSignedIn

f4d0fa0


          Update readme

31a7d89


          Update emailPasswordResendVerificationEmail def & update readme

405c9d0

diksipav force-pushed the add-auth-remix branch from 6b872ec to 405c9d0 Compare

January 2, 2024 21:50


          Update release.yml with remix actions

6a43b76

scotttrinh approved these changes

View reviewed changes

Collaborator

scotttrinh left a comment

Looks good! Thanks for the extra effort on getting the release action working 🙏

scotttrinh mentioned this pull request

Merge latest master and update release for canary #817

Closed

diksipav merged commit df0b7b6 into master

8 checks passed

diksipav deleted the add-auth-remix branch

January 3, 2024 15:55

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet