auth-helpers icon indicating copy to clipboard operation
auth-helpers copied to clipboard

chore: next 12.2.x middleware upgrade

Open boredland opened this issue 3 years ago • 10 comments

closes https://github.com/supabase-community/auth-helpers/issues/168 closes https://github.com/supabase-community/auth-helpers/issues/144 likely closes https://github.com/supabase-community/auth-helpers/issues/166

my general idea for the middleware part would be, that we'll be able to define a list of route guards, each may have a list of path-matchers (similar to the stock matcher configuration) and each may have the options we were defining before with the nested routes.

regarding the matchers I have yet to find a nice approach on how to replicate the behavior of the stock matchers. So, right now this is meant as a draft to get some feedback if that API would be sth. we could live & work with.

boredland avatar Jul 10 '22 11:07 boredland

edit: I guess nextjs processes paths extremely similar to url-pattern, so I'll just add that for now. Better ideas (preferrably using next utils) welcome!

boredland avatar Jul 10 '22 11:07 boredland

This would be really nice if it could be merged, since I can't get my app to work with the latest version of NextJS.

jonathanwilke avatar Jul 20 '22 08:07 jonathanwilke

@boredland is this ready to go? I've had a look over the code and it looks good to me.

silentworks avatar Jul 26 '22 00:07 silentworks

@boredland is this ready to go? I've had a look over the code and it looks good to me.

I'd say so yes. Added an example app so you can try it out...

boredland avatar Jul 26 '22 13:07 boredland

Hey @silentworks, did you found the time to test the example app on this PR? If yes or if that's not necessary, I think @boredland can then proceed to merge it!

413n avatar Jul 27 '22 17:07 413n

I think @boredland can then proceed to merge it!

I think he can't, because "At least 1 approving review is required by reviewers with write access".

boredland avatar Jul 28 '22 07:07 boredland

Yeah I meant that if @silentworks approves it then you can merge it!

413n avatar Jul 28 '22 07:07 413n

Please run pnpm build:nextjs after making these changes from the root directory. Also run the example app to see that signing in and signing out is working. Currently the url isn't redirecting properly when I tested this and the hash with the access_token is in the URL when I tried signing in.

silentworks avatar Jul 28 '22 19:07 silentworks

Currently the url isn't redirecting properly when I tested this and the hash with the access_token is in the URL when I tried signing in.

While you're right, I am not so sure this is connected to this PR, as this shouldn't be connected to middleware, should it?

boredland avatar Jul 28 '22 21:07 boredland

Currently the url isn't redirecting properly when I tested this and the hash with the access_token is in the URL when I tried signing in.

While you're right, I am not so sure this is connected to this PR, as this shouldn't be connected to middleware, should it?

I think this is related to the PR somehow, not sure which if it's a part of the NextJS 12.2 change or something else. When I test the main branch this issue doesn't exist at all.

silentworks avatar Jul 29 '22 13:07 silentworks

@boredland thanks so much for this, and sorry for moving so slowly here 🙈

Two things. We're working on a new approach on the next branch which incorporates the changes of supabase-js V2 RC. In the meantime we'll release https://github.com/supabase/auth-helpers/pull/227 as v 0.2.8 to bridge the gap until we can release the next changes.

Sorry again that this didn't work out, but we really appreciate your contribution and helping us nudge in the right direction! 💚

thorwebdev avatar Sep 15 '22 05:09 thorwebdev