-
Notifications
You must be signed in to change notification settings - Fork 154
Fix clientOptions issues and cookie problems
#463
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
Conversation
This allows you to disable cookies, and customize certain options that supabase ssr doesn't allow changing.
This allows you to change the default cookie name that supabase stores cookies under.
✅ Deploy Preview for n3-supabase canceled.
|
|
Thanks a lot for this PR and how you've documented it @Jordan-Ellis! I've just updated the documentation and merge the main branch but from what I see, it looks all good to me to merge it. What I propose is to merge it and I'll use the continuous release during a week to test it on my project (and ensure there is no regression). Can you also do it yourself, I suspect you're disabling Next week if everything seems good, we can release it and publish the new version. WDYT? |
|
@Jordan-Ellis Version |
|
Awesome! I'll do a thorough test this weekend. |
|
Looking forward to this release, any updates? As it stands, the docs don't match the latest release version https://supabase.nuxtjs.org/get-started#usessrcookies |
|
I just tested it again using the local demo project and everything seems to work. I also I used the new version in my production app with no issues. Although intended behavior, changing the option |
|
Just released it ✨ https://github.com/nuxt-modules/supabase/releases/tag/v1.5.0 |
The following issues are all more or less related to the same set of problems. This pull request aims to add options that solve all of these problems, and make the module more flexible and consistent!
createBrowserClientissuesCurrently, the
createBrowserClientoverrides multiple client options, causing multiple issues:You can't set
detectSessionInUrlto false in the client options (this is caused from the override). In my static site, I need to handle the session manually.Issue: Setting persistSession: false still persists auth session (this is caused from the override)
Issue: The module doesn't recognize supabase.clientOptions.auth.flowType key (this is caused from the override)
Issue: Detecting session in URL is not working when sending "Password recovery" email manually from Supabase dashboard (supabase ssr prevents using the implicit flow)
Issue: Static Hosting: Client side only rendering gives error when signing in with email (this would be solved if you use
createClientinstead ofcreateBrowserClient.)Issue: Non-cookie token storage (there's currently not a way to disable use of cookies)
In certain environments you might not be able to set a cookie, so the local storage behavior would be preferred.
Although a hack, the following code also doesn't work because the client options are overwritten:
Override storage to use local storage
It seems based on these comments that supabase ssr wants to keep these methods overwritten:
Cookie issues
Along with some of the issues above which related to cookies, there are multiple inconsistent behaviors of the cookie options:
cookieNamein the config, however this only applies to the redirect cookie.-redirect-pathto it.cookieOptionson the other hand is used for the redirect cookie, and is passed to the supabase ssr clients.cookieOptionsare used for the redirect cookie.cookieNameis misleading, as it's not the name of the cookie, but the prefix.Changes made
Added a
useSsrCookiesoption to the config which if enabled, uses thecreateBrowserClientmethod to construct the client. By default this is true, so the behavior will not change when updating the module.If disabled, the module will use the
createClientmethod directly. This allows you to completely customize the client options, and store the session info in local storage. See the docs changes for a more detailed explanation.This indirectly solves most issues related to the inability to customize the client options, or disable cookie storage.
Renamed
cookieNametocookiePrefixto more accurately reflect what it does. The prefix also now applies to the supabase cookies, allowing for total customization of all cookies when usingserverSupabaseClientanduseSupabaseClient.Renamed
cookieRedirecttosaveRedirectToCookieto better reflect what it does.Added a
useSupabaseCookieRedirectcomposable that handles getting and setting the cookie redirect value. This abstracts out the cookie retrieval so you don't have to hard code the name. It also allows for manually setting the redirect path if needed. See the docs for more details.Example of using the composable
before:after:
Updated the docs to thoroughly explain in more detail how to use these options and the caveats to the issues described. Hopefully this can solve, or at least explain a lot of peoples issues!
Does this break anything?
It shouldn't. Everything is opt in, and both
cookieNameandcookiePrefixhave been deprecated, so existing setups will not be affected and will behave as before.Other solutions considered
Show more
This change doesn't fix the problem where you can't set certain options like `detectSessionInUrl` with `useSsrCookies` enabled. It does however, let you disable cookies letting you fully customize it.The only way to fix it directly is to remove the
createBrowserClientmethod entirely and implement a custom version of it, however the source code for it is very large, and making a change to it could break things if thecreateServerClientmethod is changed.Since
@supabase/ssris the official package for ssr, it makes sense to not touch it. It's also technically still in beta, so it's possible that this specific issue of config overrides could be fixed in the future.I'd appreciate any feedback on the changes! I would love to get this merged as I love this module!
Thanks!