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

feat: provide default storage when persistSession is false or localStorage is not supported #774

Merged
merged 1 commit into from
Sep 19, 2023

Conversation

hf
Copy link
Contributor

@hf hf commented Aug 31, 2023

Provides a default memory-based, per client, storage interface when the persistSession is set to false (which previously used a similar approach) or when the storage option is not set and the current environment is not a browser or has no localStorage registered on globalThis.

With this PR, the warning that was often emitted in server-side environments is gone, as those environments now gracefully and safely fall back to using per-client memory storage.

@hf hf force-pushed the hf/default-memory-storage branch 2 times, most recently from d0d2ff1 to 94ba7b6 Compare September 1, 2023 08:53
@@ -1756,11 +1748,7 @@ export default class GoTrueClient {
private async _removeSession() {
this._debug('#_removeSession()')

if (this.persistSession) {
await removeItemAsync(this.storage, this.storageKey)
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah okay was slow to figure it out so calling it out explicitly for future ref feel free to correct me if my understanding is off.

Looks like we no longer need to check persistSession since there's a default inMemorySession when persistSession is set to false. Consequently there's no need to check for the persistSession before calling removeItemAsync since it will now default to the in memory session

Copy link
Contributor

@J0 J0 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for putting this together!

@azlekov

This comment was marked as outdated.

@hf
Copy link
Contributor Author

hf commented Sep 19, 2023

Tested this with Supabase Dashboard. Seems to be working!

@hf hf merged commit 9324fa5 into master Sep 19, 2023
2 checks passed
@hf hf deleted the hf/default-memory-storage branch September 19, 2023 16:37
@github-actions
Copy link
Contributor

🎉 This PR is included in version 2.53.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants