Add clearPersistence(), separate functionality out from shutdown() #1712

thebrianchen · 2019-04-19T21:07:20Z

No description provided.

…schmidt

schmidt-sebastian

Some drive-by comments, but looks good :)

packages/firestore/src/api/database.ts

packages/firestore/src/local/indexeddb_persistence.ts

packages/firestore/test/integration/api/database.test.ts

packages/firestore/src/api/database.ts

packages/firestore/src/local/indexeddb_persistence.ts

packages/firestore-types/index.d.ts

thebrianchen

also removed boolean flags from shutdown()

packages/firestore/test/integration/api/database.test.ts

packages/firestore/src/core/firestore_client.ts

thebrianchen

thanks for the help sebastian :)

packages/firestore-types/index.d.ts

packages/firestore/src/api/database.ts

wilhuff · 2019-04-22T17:14:48Z

packages/firestore-types/index.d.ts

@@ -216,6 +216,13 @@ export class FirebaseFirestore {
   */
  app: any;

+  /**
+   * Clears the persistence cache. This can only be called when the client is not running.


A few thoughts on this comment:

In the enablePersistence comment above we say we "enable persistent storage". Let's describe this comparably.

We should also define "not running" concretely: see how enablePersistence defines the before state, but also after deleting the app.

We also need to adjust the comment for enablePersistence(), since clearPersistence is allowed.

You should indicate that we'll throw an exception if the client is still running

You should indicate under which conditions the promise can be rejected.

(And again, all in a separate PR.)

packages/firestore/src/api/database.ts

packages/firestore/src/core/firestore_client.ts

packages/firestore/src/local/indexeddb_persistence.ts

packages/firestore/test/integration/api/database.test.ts

packages/firestore/test/integration/util/helpers.ts

packages/firestore/test/unit/local/local_store.test.ts

thebrianchen

Resolved @wilhuff comments

packages/firestore/test/integration/util/helpers.ts

wilhuff

LGTM

A few final nits. Make sure Travis is green before merging.

packages/firestore/src/api/database.ts

packages/firestore/test/integration/util/helpers.ts

wilhuff · 2019-04-23T03:20:22Z

Good to go!

Brian Chen added 7 commits April 17, 2019 21:40

init no tests

b947d92

init

b805895

merge

a69820c

[AUTOMATED]: Prettier Code Styling

ae22244

integration test in

7fed562

code comment updates

49bdebf

Merge branch 'master' of github.com:firebase/firebase-js-sdk into bc/…

dcda4f5

…schmidt

thebrianchen added the api: firestore label Apr 19, 2019

typo fix

c1edc39

thebrianchen requested review from mikelehen, wilhuff and schmidt-sebastian April 19, 2019 21:09

add await in shutdown

269480d

schmidt-sebastian reviewed Apr 19, 2019

View reviewed changes

packages/firestore-types/index.d.ts Outdated Show resolved Hide resolved

Brian Chen added 4 commits April 19, 2019 15:28

pass in only persistenceKey

8c7d9d6

removing deleteData flag from shutdown()

fdd6f1d

remove purgePersistenceWithDataLoss flag from shutdown()

c777796

[AUTOMATED]: Prettier Code Styling

9a2b683

thebrianchen requested review from Feiyang1, hiranya911 and hsubox76 as code owners April 19, 2019 23:17

thebrianchen commented Apr 19, 2019

View reviewed changes

Brian Chen added 2 commits April 19, 2019 16:21

removed binds

2e114e1

[AUTOMATED]: Prettier Code Styling

406f535

schmidt-sebastian reviewed Apr 19, 2019

View reviewed changes

packages/firestore/test/integration/api/database.test.ts Outdated Show resolved Hide resolved

packages/firestore/test/integration/api/database.test.ts Outdated Show resolved Hide resolved

packages/firestore/src/core/firestore_client.ts Outdated Show resolved Hide resolved

split test and add error message

75066cd

thebrianchen commented Apr 19, 2019

View reviewed changes

thebrianchen assigned mikelehen Apr 19, 2019

thebrianchen requested a review from schmidt-sebastian April 19, 2019 23:44

Brian Chen added 2 commits April 19, 2019 16:45

update error message

89fa8b1

typo in error message

963f9f6

wilhuff suggested changes Apr 22, 2019

View reviewed changes

wilhuff assigned thebrianchen and unassigned mikelehen Apr 22, 2019

Brian Chen added 2 commits April 22, 2019 11:01

removing index files

68f582e

resolving gil's comments

5834b2a

thebrianchen commented Apr 22, 2019

View reviewed changes

thebrianchen assigned wilhuff Apr 22, 2019

Brian Chen added 2 commits April 22, 2019 15:19

remove restartfirestore

2e85591

remove unused imports

b2e1dfe

thebrianchen requested review from wilhuff and removed request for hiranya911, Feiyang1 and hsubox76 April 22, 2019 22:40

Brian Chen added 2 commits April 22, 2019 17:02

fixed firestore error, but getting serializer error

4064359

[AUTOMATED]: Prettier Code Styling

c9a252a

mikelehen reviewed Apr 23, 2019

View reviewed changes

packages/firestore/test/integration/util/helpers.ts Outdated Show resolved Hide resolved

packages/firestore/test/integration/util/helpers.ts Outdated Show resolved Hide resolved

thebrianchen removed the request for review from schmidt-sebastian April 23, 2019 00:18

Brian Chen added 2 commits April 22, 2019 17:19

resolve mike's comments

b2cb0c0

alphabetize imports

5659d92

wilhuff approved these changes Apr 23, 2019

View reviewed changes

packages/firestore/src/api/database.ts Outdated Show resolved Hide resolved

packages/firestore/test/integration/util/helpers.ts Outdated Show resolved Hide resolved

gil nits resolved

a621e4d

thebrianchen changed the title ~~clearPersistence() first pass~~ Add clearPersistence(), separate functionality out from shutdown() Apr 23, 2019

thebrianchen merged commit b35ae72 into master Apr 23, 2019

thebrianchen deleted the bc/schmidt branch April 23, 2019 16:41

firebase locked and limited conversation to collaborators Oct 11, 2019

Add clearPersistence(), separate functionality out from shutdown() #1712

Add clearPersistence(), separate functionality out from shutdown() #1712

Uh oh!

Conversation

thebrianchen commented Apr 19, 2019

Uh oh!

schmidt-sebastian left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thebrianchen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thebrianchen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

wilhuff Apr 22, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thebrianchen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

wilhuff left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

wilhuff commented Apr 23, 2019

Uh oh!

Uh oh!