fix(firebase_auth): pass `Persistence` value to `FirebaseAuth.instanceFor(app: app, persistence: persistence)` for setting persistence on Web platform #9138

russellwheatley · 2022-07-18T14:46:35Z

Description

"persistence" behaviour has changed on the web platform. This PR fixes "persistence" so it locally persists data as it was before web v9 JS SDK update.

This PR also allows a persistence value to be passed on initialising Auth instance FirebaseAuth.instanceFor(app: app, persistence: persistence).

Related Issues

fixes #9117

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]).
This will ensure a smooth and quick review process. Updating the pubspec.yaml and changelogs is not required.

I read the Contributor Guide and followed the process outlined there for submitting PRs.
My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
All existing and new tests are passing.
I updated/added relevant documentation (doc comments with ///).
The analyzer (melos run analyze) does not report any problems on my PR.
I read and followed the Flutter Style Guide.
I signed the CLA.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

Yes, this is a breaking change.
No, this is not a breaking change.

…base Auth Web initialiser

….dart

russellwheatley

A couple of doc updates I'll make.

packages/firebase_auth/firebase_auth/lib/src/firebase_auth.dart

...ase_auth_platform_interface/lib/src/platform_interface/platform_interface_firebase_auth.dart

Ehesp · 2022-07-21T16:35:58Z

packages/firebase_auth/firebase_auth/lib/src/firebase_auth.dart

-  /// Auth state for applications that are shared by other users or have
-  /// sensitive data.
-  ///
-  /// This is only supported on web based platforms.
  Future<void> setPersistence(Persistence persistence) async {


Can we add the Deprecated annotation here so developers will get notified in their IDE?

Also, do we need to breaking change this straight away? Is it not possible to deprecate but just call the new method instead, and then eventually we can remove the method in a later release.

@Ehesp this is useless now Firebase.instance.setPersistence() because the value needs to be passed before Firebase.instance is ever called. We could just deprecate it and keep the implementation but it does nothing. We needed a new API (FirebaseAuth.persistenceType()) to pass the value on the very first call of initializeAuth() for web. We felt it made sense to simply remove underlying implementation and throw a deprecation exception so users adopt the new strategy.

I've updated the PR description, but just to be clear, setPersistence() is not useless as long as persistence is used to initialize the auth instance here

…c/platform_interface/platform_interface_firebase_auth.dart

…terfire into @russell/auth-9117

Salakar · 2022-07-22T09:11:04Z

What about having .instanceFor(app: app, persistence: persistence) since its tied directly to making a new auth instance, not sure about a new static api

…ence()

…terfire into @russell/auth-9117 � Conflicts: � docs/auth/start.md � packages/firebase_auth/firebase_auth/lib/src/firebase_auth.dart

russellwheatley

Will update this doc change after code review.

docs/auth/start.md

packages/firebase_auth/firebase_auth/lib/src/firebase_auth.dart

…terfire into @russell/auth-9117

packages/firebase_auth/firebase_auth/lib/src/firebase_auth.dart

packages/firebase_auth/firebase_auth_web/lib/firebase_auth_web.dart

packages/firebase_auth/firebase_auth/lib/src/firebase_auth.dart

docs/auth/start.md

russellwheatley added 2 commits July 18, 2022 15:42

fix(firebase_auth): created static method to pass persistence to Fire…

e1d60c6

…base Auth Web initialiser

remove print

8bdf2a4

darshankawar mentioned this pull request Jul 19, 2022

🐛 [firebase_auth] Firebase Auth Web loses persistence after upgrading to the latest version #9089

Closed

anchann mentioned this pull request Jul 19, 2022

Flutter Web Firebase Auth User Logged Out On Page Refresh #9117

Closed

chore(auth): move persistence to PI otherwise tests fail importing web

67755ab

russellwheatley changed the title ~~fix(firebase_auth): created static method FirebaseAuth.persistenceType() for setting persistence on Web platform~~ fix(firebase_auth)!: created static method FirebaseAuth.persistenceType() for setting persistence on Web platform Jul 19, 2022

russellwheatley added 4 commits July 19, 2022 14:14

chore(auth): deprecate setPersistence and create deprecated_exception…

6ef6a51

….dart

chore(auth): export deprecated exception

4439af0

tes(auth): update setPersistence tests

0fcbe0b

Merge branch 'master' into @russell/auth-9117

75caf9d

russellwheatley added type: bug Something isn't working plugin: auth platform: web Issues / PRs which are specifically for web. labels Jul 19, 2022

russellwheatley marked this pull request as ready for review July 19, 2022 15:28

russellwheatley commented Jul 19, 2022

View reviewed changes

packages/firebase_auth/firebase_auth/lib/src/firebase_auth.dart Outdated Show resolved Hide resolved

...ase_auth_platform_interface/lib/src/platform_interface/platform_interface_firebase_auth.dart Outdated Show resolved Hide resolved

Lyokone approved these changes Jul 19, 2022

View reviewed changes

Ehesp reviewed Jul 21, 2022

View reviewed changes

This comment was marked as off-topic.

Sign in to view

russellwheatley added 7 commits July 22, 2022 08:42

Update packages/firebase_auth/firebase_auth/lib/src/firebase_auth.dart

755ab71

Update packages/firebase_auth/firebase_auth_platform_interface/lib/sr…

3b1048e

…c/platform_interface/platform_interface_firebase_auth.dart

chore(auth): add deprecated annotation

5870d2d

Merge branch 'master' into @russell/auth-9117

d8d3183

Merge branch '@russell/auth-9117' of github.com:FirebaseExtended/flut…

8669864

…terfire into @russell/auth-9117

format

c8cce0a

test(auth): rm deprecated API test

4ce4726

russellwheatley added 2 commits July 22, 2022 11:28

chore(auth): rm static method and use instanceFor() for persistence

2e187f7

docs(auth): update docs and exception message

66cefed

russellwheatley requested a review from Ehesp July 22, 2022 11:57

russellwheatley added 3 commits July 25, 2022 12:15

chore(auth): make persistence per auth instance. reinstate setPersist…

3f17803

…ence()

Merge branch '@russell/auth-9117' of github.com:FirebaseExtended/flut…

ed8a069

…terfire into @russell/auth-9117 � Conflicts: � docs/auth/start.md � packages/firebase_auth/firebase_auth/lib/src/firebase_auth.dart

docs(auth): make persistence docs clear

57429c3

docs(auth): make persistence docs clear

9775e59

russellwheatley added type: missing-feature A feature that is supported on the underlying Firebase SDK but has not been exposed to Dart API. and removed type: bug Something isn't working labels Jul 25, 2022

Merge branch 'master' into @russell/auth-9117

916be94

russellwheatley commented Jul 25, 2022

View reviewed changes

docs/auth/start.md Outdated Show resolved Hide resolved

russellwheatley requested a review from Salakar July 25, 2022 13:39

dmytro-audibene mentioned this pull request Jul 25, 2022

🐛 [firebase_auth] Authenticated user isn't being restored after page refresh on flutter web #9219

Closed

darshankawar mentioned this pull request Jul 26, 2022

🐛 [firebase_auth_web] Cannot read properties of undefined (reading 'auth') #9228

Closed

Lyokone reviewed Jul 26, 2022

View reviewed changes

packages/firebase_auth/firebase_auth/lib/src/firebase_auth.dart Outdated Show resolved Hide resolved

russellwheatley added 3 commits July 27, 2022 08:06

chore(auth): rm clearance of _persistence instance member

b8a2867

Merge branch '@russell/auth-9117' of github.com:FirebaseExtended/flut…

b28389d

…terfire into @russell/auth-9117

Update docs/auth/start.md

5b2d205

Salakar reviewed Jul 27, 2022

View reviewed changes

packages/firebase_auth/firebase_auth/lib/src/firebase_auth.dart Outdated Show resolved Hide resolved

Salakar reviewed Jul 27, 2022

View reviewed changes

packages/firebase_auth/firebase_auth_web/lib/firebase_auth_web.dart Outdated Show resolved Hide resolved

Salakar reviewed Jul 27, 2022

View reviewed changes

packages/firebase_auth/firebase_auth/lib/src/firebase_auth.dart Show resolved Hide resolved

Salakar reviewed Jul 27, 2022

View reviewed changes

docs/auth/start.md Outdated Show resolved Hide resolved

Apply suggestions from code review

a1254aa

Salakar approved these changes Jul 27, 2022

View reviewed changes

russellwheatley mentioned this pull request Jul 27, 2022

chore(release): prepare for release #9235

Closed

10 tasks

russellwheatley merged commit ae7ebaf into master Jul 27, 2022

russellwheatley deleted the @russell/auth-9117 branch July 27, 2022 13:01

OutdatedGuy mentioned this pull request Jul 28, 2022

🐛 [firebase_auth] Previous auth state lost after firebase_auth: ^3.6.0 #9243

Closed

nilsreichardt mentioned this pull request Jul 31, 2022

fix(firebase_auth): add persistence parameter for delegateFor. FirebaseExtended/flutterfire_desktop#100

Merged

russellwheatley mentioned this pull request Aug 18, 2022

(analytics) Add the option to set default config on Web #9263

Closed

firebase locked and limited conversation to collaborators Aug 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(firebase_auth): pass `Persistence` value to `FirebaseAuth.instanceFor(app: app, persistence: persistence)` for setting persistence on Web platform #9138

fix(firebase_auth): pass `Persistence` value to `FirebaseAuth.instanceFor(app: app, persistence: persistence)` for setting persistence on Web platform #9138

russellwheatley commented Jul 18, 2022 •

edited

russellwheatley left a comment •

edited

Ehesp Jul 21, 2022

Ehesp Jul 21, 2022

russellwheatley Jul 22, 2022

russellwheatley Jul 25, 2022

This comment was marked as off-topic.

Salakar commented Jul 22, 2022

russellwheatley left a comment

fix(firebase_auth): pass Persistence value to FirebaseAuth.instanceFor(app: app, persistence: persistence) for setting persistence on Web platform #9138

fix(firebase_auth): pass Persistence value to FirebaseAuth.instanceFor(app: app, persistence: persistence) for setting persistence on Web platform #9138

Conversation

russellwheatley commented Jul 18, 2022 • edited

Description

Related Issues

Checklist

Breaking Change

russellwheatley left a comment • edited

Choose a reason for hiding this comment

Ehesp Jul 21, 2022

Choose a reason for hiding this comment

Ehesp Jul 21, 2022

Choose a reason for hiding this comment

russellwheatley Jul 22, 2022

Choose a reason for hiding this comment

russellwheatley Jul 25, 2022

Choose a reason for hiding this comment

This comment was marked as off-topic.

Salakar commented Jul 22, 2022

russellwheatley left a comment

Choose a reason for hiding this comment

fix(firebase_auth): pass `Persistence` value to `FirebaseAuth.instanceFor(app: app, persistence: persistence)` for setting persistence on Web platform #9138

fix(firebase_auth): pass `Persistence` value to `FirebaseAuth.instanceFor(app: app, persistence: persistence)` for setting persistence on Web platform #9138

russellwheatley commented Jul 18, 2022 •

edited

russellwheatley left a comment •

edited