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: Release enablePersistentCacheIndexAutoCreation() and related functions #7599

Merged
merged 12 commits into from
Sep 5, 2023

Conversation

dconeybe
Copy link
Contributor

Expose the functions and types for using the Firestore SDK's local persistent cache index auto-creation feature, and deprecate the functions for manually creating these indexes.

The following functions and types are made available by this PR:

  • PersistentCacheIndexManager
  • getPersistentCacheIndexManager()
  • enablePersistentCacheIndexAutoCreation()
  • disablePersistentCacheIndexAutoCreation()
  • deleteAllPersistentCacheIndexes()

This feature was implemented (but hidden) in the following PRs: #7542 and #7587. See those PRs and the public reference documentation for full details on this feature.

@dconeybe dconeybe self-assigned this Aug 30, 2023
@changeset-bot
Copy link

changeset-bot bot commented Aug 30, 2023

🦋 Changeset detected

Latest commit: 73bfb9b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@firebase/firestore Minor
firebase Minor
@firebase/firestore-compat Patch

Not sure what this means? Click here to learn what changesets are.

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

@github-actions
Copy link
Contributor

github-actions bot commented Aug 30, 2023

Changeset File Check ✅

  • No modified packages are missing from the changeset file.
  • No changeset formatting errors detected.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Aug 30, 2023

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Aug 30, 2023

@cherylEnkidu cherylEnkidu requested a review from markarndt August 31, 2023 18:12
@dconeybe dconeybe marked this pull request as ready for review September 1, 2023 04:44
@dconeybe
Copy link
Contributor Author

dconeybe commented Sep 1, 2023

@markarndt Please review this PR for tech writer approval.

@dconeybe
Copy link
Contributor Author

dconeybe commented Sep 1, 2023

@markarndt For context, the equivalent PR in the android sdk, which you reviewed, is firebase/firebase-android-sdk#5256.

Copy link
Collaborator

@markarndt markarndt left a comment

Choose a reason for hiding this comment

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

A few edits.

@@ -13,6 +13,11 @@ https://github.com/firebase/firebase-js-sdk
> This API is provided as a preview for developers and may change based on feedback that we receive. Do not use this API in a production environment.
>

> Warning: This API is now obsolete.
>
> Instead of creating cache indexes manually, consider using `enablePersistentCacheIndexAutoCreation()` to let SDK decide whether to create cache indexes for queries running locally.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Edit:

"Instead of creating cache indexes manually, consider using enablePersistentCacheIndexAutoCreation() to let the SDK decide whether to create cache indexes for queries running locally."

Copy link
Collaborator

Choose a reason for hiding this comment

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

And line 16 in a couple other files below.

| <b>function(indexManager...)</b> |
| [deleteAllPersistentCacheIndexes(indexManager)](./firestore_.md#deleteallpersistentcacheindexes) | Removes all persistent cache indexes.<!-- -->Please note this function will also deletes indexes generated by <code>setIndexConfiguration()</code>, which is deprecated. |
| [disablePersistentCacheIndexAutoCreation(indexManager)](./firestore_.md#disablepersistentcacheindexautocreation) | Stops creating persistent cache indexes automatically for local query execution. The indexes which have been created by calling <code>enablePersistentCacheIndexAutoCreation()</code> still take effect. |
| [enablePersistentCacheIndexAutoCreation(indexManager)](./firestore_.md#enablepersistentcacheindexautocreation) | Enables SDK to create persistent cache indexes automatically for local query execution when SDK believes cache indexes can help improves performance.<!-- -->This feature is disabled by default. |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Edit:

"enablePersistentCacheIndexAutoCreation(indexManager) | Enables the SDK to create persistent cache indexes automatically for local query execution when the SDK believes cache indexes can help improves performance.This feature is disabled by default."

@@ -130,6 +135,7 @@ https://github.com/firebase/firebase-js-sdk
| [FirestoreError](./firestore_.firestoreerror.md#firestoreerror_class) | An error returned by a Firestore operation. |
| [GeoPoint](./firestore_.geopoint.md#geopoint_class) | An immutable object representing a geographic location in Firestore. The location is represented as latitude/longitude pair.<!-- -->Latitude values are in the range of \[-90, 90\]. Longitude values are in the range of \[-180, 180\]. |
| [LoadBundleTask](./firestore_.loadbundletask.md#loadbundletask_class) | Represents the task of loading a Firestore bundle. It provides progress of bundle loading, as well as task completion and error events.<!-- -->The API is compatible with <code>Promise&lt;LoadBundleTaskProgress&gt;</code>. |
| [PersistentCacheIndexManager](./firestore_.persistentcacheindexmanager.md#persistentcacheindexmanager_class) | A <code>PersistentCacheIndexManager</code> which you can config persistent cache indexes used for local query execution.<!-- -->To use, call <code>getPersistentCacheIndexManager()</code> to get an instance. |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Edits:

PersistentCacheIndexManager | A PersistentCacheIndexManager for configuring persistent cache indexes used for local query execution.To use, call getPersistentCacheIndexManager() to get an instance.

@@ -659,6 +687,11 @@ If the transaction completed successfully or was explicitly aborted (the `update
> This API is provided as a preview for developers and may change based on feedback that we receive. Do not use this API in a production environment.
>

> Warning: This API is now obsolete.
>
> Instead of creating cache indexes manually, consider using `enablePersistentCacheIndexAutoCreation()` to let SDK decide whether to create cache indexes for queries running locally.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Edit:

"Instead of creating cache indexes manually, consider using enablePersistentCacheIndexAutoCreation() to let the SDK decide whether to create cache indexes for queries running locally."

Copy link
Collaborator

Choose a reason for hiding this comment

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

And below....


## enablePersistentCacheIndexAutoCreation()

Enables SDK to create persistent cache indexes automatically for local query execution when SDK believes cache indexes can help improves performance.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Edit:

"Enables the SDK to create persistent cache indexes automatically for local query execution when the SDK believes cache indexes can help improves performance."

https://github.com/firebase/firebase-js-sdk
{% endcomment %}

# PersistentCacheIndexManager class
Copy link
Collaborator

Choose a reason for hiding this comment

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

Edit (I don't know if these are generated files, so edits to source may make these .md edits unnecessary.

"A PersistentCacheIndexManager for configuring persistent cache indexes used for local query execution."

…believes' -> 'when the SDK believes' and 'can help improves performance' -> 'can help improve performance'
@dconeybe dconeybe requested a review from markarndt September 5, 2023 17:59
@dconeybe
Copy link
Contributor Author

dconeybe commented Sep 5, 2023

@markarndt PTAL. I believe I have addressed your feedback.

Copy link
Contributor

@cherylEnkidu cherylEnkidu 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 fbd8e0e into master Sep 5, 2023
41 checks passed
@dconeybe dconeybe deleted the dconeybe/ClientSideIndexAutoCreationExpose branch September 5, 2023 19:57
@google-oss-bot google-oss-bot mentioned this pull request Sep 12, 2023
@firebase firebase locked and limited conversation to collaborators Oct 6, 2023
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.

5 participants