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

Firestore: component_provider.ts: cleanup terminate() functions #7906

Merged

Conversation

dconeybe
Copy link
Contributor

@dconeybe dconeybe commented Dec 24, 2023

This PR makes a few tiny code improvements to the terminate() functions in component_provider.ts:

  1. Simplify some if blocks checking for and object being null with more concise null chaining (i.e. the ?. operator).
  2. Remove await from function calls that simply return void, which make the await superfluous and potentially confusing.
  3. Explicitly call indexBackfillerScheduler?.stop(), just like how gcScheduler?.stop() is called. This doesn't really affect anything because the scheduled job is cancelled by virtue of the AsyncQueue being shut down, but it's probably best not to rely on that implementation detail of the scheduler.

@dconeybe dconeybe self-assigned this Dec 24, 2023
Copy link

changeset-bot bot commented Dec 24, 2023

⚠️ No Changeset found

Latest commit: efedb67

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Dec 24, 2023

Size Report 1

Affected Products

  • @firebase/firestore

    TypeBase (537d399)Merge (9bdee51)Diff
    browser375 kB375 kB+83 B (+0.0%)
    esm5360 kB360 kB+20 B (+0.0%)
    main577 kB577 kB+106 B (+0.0%)
    module375 kB375 kB+83 B (+0.0%)
    react-native375 kB375 kB+83 B (+0.0%)
  • bundle

    TypeBase (537d399)Merge (9bdee51)Diff
    firestore (Persistence)302 kB303 kB+83 B (+0.0%)
    firestore (Query Cursors)238 kB238 kB+83 B (+0.0%)
    firestore (Query)236 kB236 kB+83 B (+0.0%)
    firestore (Read data once)224 kB224 kB+83 B (+0.0%)
    firestore (Read Write w Persistence)321 kB321 kB+83 B (+0.0%)
    firestore (Realtime updates)226 kB226 kB+83 B (+0.0%)
    firestore (Transaction)204 kB204 kB+83 B (+0.0%)
    firestore (Write data)204 kB204 kB+83 B (+0.0%)
  • firebase

    TypeBase (537d399)Merge (9bdee51)Diff
    firebase-compat.js779 kB779 kB+81 B (+0.0%)
    firebase-firestore-compat.js341 kB341 kB+81 B (+0.0%)
    firebase-firestore.js434 kB434 kB+83 B (+0.0%)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/mUOksNSd4s.html

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Dec 24, 2023

Size Analysis Report 1

Affected Products

  • @firebase/firestore

    • addDoc

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size123 kB123 kB+83 B (+0.1%)
      size-with-ext-deps194 kB194 kB+83 B (+0.0%)
    • deleteAllPersistentCacheIndexes

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size87.2 kB87.3 kB+73 B (+0.1%)
      size-with-ext-deps157 kB157 kB+73 B (+0.0%)
    • deleteDoc

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size114 kB114 kB+83 B (+0.1%)
      size-with-ext-deps185 kB185 kB+83 B (+0.0%)
    • disableNetwork

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size103 kB103 kB+83 B (+0.1%)
      size-with-ext-deps173 kB173 kB+83 B (+0.0%)
    • disablePersistentCacheIndexAutoCreation

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size87.2 kB87.2 kB+73 B (+0.1%)
      size-with-ext-deps157 kB157 kB+73 B (+0.0%)
    • enableIndexedDbPersistence

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size186 kB186 kB+83 B (+0.0%)
      size-with-ext-deps258 kB258 kB+83 B (+0.0%)
    • enableMultiTabIndexedDbPersistence

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size222 kB222 kB+83 B (+0.0%)
      size-with-ext-deps294 kB294 kB+83 B (+0.0%)
    • enableNetwork

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size103 kB103 kB+83 B (+0.1%)
      size-with-ext-deps173 kB173 kB+83 B (+0.0%)
    • enablePersistentCacheIndexAutoCreation

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size87.2 kB87.2 kB+73 B (+0.1%)
      size-with-ext-deps157 kB157 kB+73 B (+0.0%)
    • executeWrite

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size113 kB113 kB+83 B (+0.1%)
      size-with-ext-deps184 kB184 kB+83 B (+0.0%)
    • getAggregateFromServer

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size110 kB110 kB+83 B (+0.1%)
      size-with-ext-deps180 kB180 kB+83 B (+0.0%)
    • getCountFromServer

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size110 kB110 kB+83 B (+0.1%)
      size-with-ext-deps181 kB181 kB+83 B (+0.0%)
    • getDoc

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size143 kB143 kB+83 B (+0.1%)
      size-with-ext-deps214 kB214 kB+83 B (+0.0%)
    • getDocFromCache

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size94.6 kB94.7 kB+73 B (+0.1%)
      size-with-ext-deps165 kB165 kB+73 B (+0.0%)
    • getDocFromServer

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size143 kB143 kB+83 B (+0.1%)
      size-with-ext-deps214 kB214 kB+83 B (+0.0%)
    • getDocs

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size145 kB145 kB+83 B (+0.1%)
      size-with-ext-deps216 kB216 kB+83 B (+0.0%)
    • getDocsFromCache

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size102 kB102 kB+73 B (+0.1%)
      size-with-ext-deps172 kB173 kB+73 B (+0.0%)
    • getDocsFromServer

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size145 kB145 kB+83 B (+0.1%)
      size-with-ext-deps215 kB215 kB+83 B (+0.0%)
    • loadBundle

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size112 kB112 kB+83 B (+0.1%)
      size-with-ext-deps183 kB183 kB+83 B (+0.0%)
    • memoryEagerGarbageCollector

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size85.2 kB85.3 kB+73 B (+0.1%)
      size-with-ext-deps155 kB155 kB+73 B (+0.0%)
    • memoryLocalCache

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size99.7 kB99.8 kB+83 B (+0.1%)
      size-with-ext-deps170 kB170 kB+83 B (+0.0%)
    • memoryLruGarbageCollector

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size91.7 kB91.8 kB+73 B (+0.1%)
      size-with-ext-deps162 kB162 kB+73 B (+0.0%)
    • namedQuery

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size88.1 kB88.2 kB+73 B (+0.1%)
      size-with-ext-deps158 kB158 kB+73 B (+0.0%)
    • onSnapshot

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size145 kB146 kB+83 B (+0.1%)
      size-with-ext-deps216 kB216 kB+83 B (+0.0%)
    • onSnapshotsInSync

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size135 kB135 kB+83 B (+0.1%)
      size-with-ext-deps206 kB206 kB+83 B (+0.0%)
    • persistentLocalCache

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size183 kB183 kB+83 B (+0.0%)
      size-with-ext-deps255 kB255 kB+83 B (+0.0%)
    • persistentMultipleTabManager

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size219 kB219 kB+83 B (+0.0%)
      size-with-ext-deps291 kB291 kB+83 B (+0.0%)
    • persistentSingleTabManager

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size183 kB183 kB+83 B (+0.0%)
      size-with-ext-deps255 kB255 kB+83 B (+0.0%)
    • runTransaction

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size123 kB123 kB+83 B (+0.1%)
      size-with-ext-deps194 kB194 kB+83 B (+0.0%)
    • setDoc

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size122 kB123 kB+83 B (+0.1%)
      size-with-ext-deps193 kB193 kB+83 B (+0.0%)
    • setIndexConfiguration

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size90.7 kB90.8 kB+73 B (+0.1%)
      size-with-ext-deps161 kB161 kB+73 B (+0.0%)
    • updateDoc

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size123 kB123 kB+83 B (+0.1%)
      size-with-ext-deps194 kB194 kB+83 B (+0.0%)
    • waitForPendingWrites

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size103 kB103 kB+83 B (+0.1%)
      size-with-ext-deps174 kB174 kB+83 B (+0.0%)
    • writeBatch

      Size

      TypeBase (537d399)Merge (9bdee51)Diff
      size125 kB125 kB+83 B (+0.1%)
      size-with-ext-deps196 kB196 kB+83 B (+0.0%)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/VTS6OMJdKf.html

Copy link
Contributor

@milaGGL milaGGL left a comment

Choose a reason for hiding this comment

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

LGTM

@dconeybe dconeybe merged commit 97fabd4 into master Jan 2, 2024
51 of 54 checks passed
@dconeybe dconeybe deleted the dconeybe/ComponentProviderCleanupTerminateFunctions branch January 2, 2024 20:30
@firebase firebase locked and limited conversation to collaborators Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants