Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Drop RR requirement and simplify library a lot #299

Merged
merged 11 commits into from
Nov 24, 2024
Merged

Conversation

sergiodxa
Copy link
Owner

@sergiodxa sergiodxa commented Sep 20, 2024

Change how the Authenticator and Strategy classes work to drop requirement of a Remix session storage at the authenticator level.

This simplifies many things in the library, since strategies can now define the cookie/session storage to store the intermediate state they may need (like OAuth state and PKCE values) or nothing if there's no reason to use one, then instead of Remix Auth handling how the user data is stored in the application session, that depends on the app.

Because of that the logout method was removed, all the sessionKey, etc. options were removed, the Strategy failure and success methods were removed and the successRedirect and failureRedirect methods were removed too.

The authenticate method will now return the user data returned by the strategy verify function, or it will throw. Here it depends on the strategy as it may only throw errors or in case of OAuth2 based strategies it may need to throw a redirect.

This also brings another important change, as Remix Auth itself doesn't depends on react-router or any @remix-run/* package, this means if a strategy doesn't depend on them this will work everywhere you can use Request and Response objects.

Only possible Remix/Rr requirement will be how to handle throw responses.

@sergiodxa sergiodxa self-assigned this Sep 20, 2024
@sergiodxa sergiodxa added enhancement New feature or request breaking change A change introducing a breaking change labels Sep 20, 2024
@stephen776
Copy link

stephen776 commented Nov 22, 2024

Hey @sergiodxa - can we expect this to get merged? I am migrating to RR7 and remix-auth is one of the last hurdles. The peer dep on @remix-run/react is causing issues for me using pnpm

Happy to provide more info to assist.

@stephen776
Copy link

pnpm why react-router-dom gives me this:

image

and I am hitting runtime errors from things trying to import from react-router-dom

@sergiodxa sergiodxa merged commit 89ad79d into main Nov 24, 2024
5 checks passed
@sergiodxa sergiodxa deleted the simplify-library branch November 24, 2024 06:42
@sergiodxa sergiodxa changed the title Use a Cookie object instead of SessionStorage Drop RR requirement and simplify library a lot Nov 24, 2024
sergiodxa added a commit to sergiodxa/remix-auth-form that referenced this pull request Nov 27, 2024
Updated strategy to use [remix-auth
v4](sergiodxa/remix-auth#299)

---------

Co-authored-by: Sergio Xalambrí <hello@sergiodxa.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
breaking change A change introducing a breaking change enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants