Skip to content

feat(cloud_firestore): experimental setIndexConfiguration() API #9928

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

Merged
merged 9 commits into from
Nov 16, 2022

Conversation

russellwheatley
Copy link
Member

@russellwheatley russellwheatley commented Nov 14, 2022

Description

This feature allows users to set index configurations on cached data when querying offline. This is aimed at improving query speed when offline.

It follows the Cloud Firestore Definition Index set out here.

All platforms expect a JSON string following the above definition. If JSON string is in the incorrect format, it will throw an error.

I have created an index definition file with the relevant classes that correspond to the above definition to make it easy to construct the correct JSON string.

I have updated the inline documentation, but we don't have control over Firestore documentation on the main website. We need to raise this in the next meeting.

Related Issues

See this discussion: #9913

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.

// Only when back online will this trigger
controller.add(true);
});
test(
Copy link
Member Author

Choose a reason for hiding this comment

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

I updated this file so that web is skipped for each individual e2e test rather than for the entire file hence why it looks like a lot of changes.

skip: kIsWeb,
);

test('setIndexConfiguration()', () async {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the integration test for setIndexConfiguration().

@russellwheatley russellwheatley marked this pull request as ready for review November 15, 2022 11:03
/// The index entries themselves are created asynchronously. You can continue to use queries that
/// require indexing even if the indices are not yet available. Query execution will automatically
/// start using the index once the index entries have been written.
Future<void> setIndexConfiguration({
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we use the experimental flag here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, I think we should 👌

@russellwheatley russellwheatley changed the title feat(cloud_firestore): setIndexConfiguration() API feat(cloud_firestore): experimental setIndexConfiguration() API Nov 16, 2022
@russellwheatley russellwheatley merged commit bf6eda1 into master Nov 16, 2022
@russellwheatley russellwheatley deleted the @russell/index-config branch November 16, 2022 12:35
@firebase firebase locked and limited conversation to collaborators Dec 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants