Skip to content

Conversation

christian-byrne
Copy link
Contributor

@christian-byrne christian-byrne commented Sep 17, 2025

Currently, we set persistence method in the auth store setup. This creates pattern of using the default on init (indexed DB) up until the firebase store is initialized and setPersistence is called. For devices that don't support indexed DB or have the connection aggresively terminated or cleared, like Safari, this can create problems with maintaing auth persistence.

Fix by setting persistence method in the initialization in main.ts

┆Issue is synchronized with this Notion page by Unito

@christian-byrne christian-byrne requested a review from a team as a code owner September 17, 2025 01:11
@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Sep 17, 2025
Copy link

github-actions bot commented Sep 17, 2025

🎭 Playwright Test Results

All tests passed!

⏰ Completed at: 09/17/2025, 02:41:59 AM UTC

📈 Summary

  • Total Tests: 450
  • Passed: 421 ✅
  • Failed: 0
  • Flaky: 0
  • Skipped: 29 ⏭️

📊 Test Reports by Browser

  • chromium: View Report • ✅ 414 / ❌ 0 / ⚠️ 0 / ⏭️ 29
  • chromium-2x: View Report • ✅ 2 / ❌ 0 / ⚠️ 0 / ⏭️ 0
  • chromium-0.5x: View Report • ✅ 1 / ❌ 0 / ⚠️ 0 / ⏭️ 0
  • mobile-chrome: View Report • ✅ 4 / ❌ 0 / ⚠️ 0 / ⏭️ 0

🎉 Click on the links above to view detailed test results for each browser configuration.

DrJKL
DrJKL previously approved these changes Sep 17, 2025
Copy link
Contributor

@DrJKL DrJKL left a comment

Choose a reason for hiding this comment

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

🔥

@DrJKL
Copy link
Contributor

DrJKL commented Sep 17, 2025

Change detector tests detected changes.

…B issues

- Use VueFireAuthWithDependencies to set localStorage-first persistence order
- Prevents "Connection to Indexed Database server lost" errors on iOS Safari
- Remove obsolete persistence test since configuration moved to main.ts

Fixes CLOUD-FRONTEND-STAGING-2J

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@christian-byrne christian-byrne requested a review from a team as a code owner September 17, 2025 02:31
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Sep 17, 2025
@DrJKL DrJKL self-assigned this Sep 17, 2025
@christian-byrne christian-byrne merged commit ea4e57b into main Sep 18, 2025
25 checks passed
@christian-byrne christian-byrne deleted the fix/safari-mobile-indexed-db branch September 18, 2025 18:18
Myestery pushed a commit that referenced this pull request Sep 19, 2025
Currently, we set persistence method in the auth store setup. This
creates pattern of using the default on init (indexed DB) up until the
firebase store is initialized and `setPersistence` is called. For
devices that don't support indexed DB or have the connection aggresively
terminated or cleared, like
[Safari](https://comfy-org.sentry.io/issues/6879071102/?project=4509681221369857&query=is%3Aunresolved&referrer=issue-stream),
this can create problems with maintaing auth persistence.

Fix by setting persistence method in the initialization in main.ts

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5614-Move-VueFire-persistence-configuration-to-initialization-2716d73d3650817480e0c8feb1f37b9a)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Claude <[email protected]>
christian-byrne added a commit that referenced this pull request Sep 23, 2025
christian-byrne added a commit that referenced this pull request Sep 23, 2025
…) (#5729)

## Summary
This reverts PR #5614 which moved VueFire persistence configuration to
initialization.

## Reason for Revert

It breaks Google SSO login with error:

```
useErrorHandling.ts:12 FirebaseError: Firebase: Error (auth/argument-error).
    at createErrorInternal (index-c92d61ad.js:506:41)
    at _assert (index-c92d61ad.js:512:15)
    at _withDefaultResolver (index-c92d61ad.js:9237:5)
    at signInWithPopup (index-c92d61ad.js:9457:30)
    at executeAuthAction.createCustomer (firebaseAuthStore.ts:263:25)
    at executeAuthAction (firebaseAuthStore.ts:223:28)
    at Proxy.loginWithGoogle (firebaseAuthStore.ts:262:5)
    at Proxy.wrappedAction (pinia.mjs:1405:26)
    at useFirebaseAuthActions.ts:104:28
    at Object.signInWithGoogle (useErrorHandling.ts:39:22)
```

## Changes
- Reverts commit ea4e57b "Move VueFire persistence configuration to
initialization (#5614)"
- Restores previous Firebase auth persistence behavior

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5729-Revert-Move-VueFire-persistence-configuration-to-initialization-5614-2776d73d3650814c9b80d9c67c852874)
by [Unito](https://www.unito.io)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:auth size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants