Skip to content

Move Datastore to ComponentProvider #3417

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 2 commits into from
Jul 17, 2020
Merged

Conversation

schmidt-sebastian
Copy link
Contributor

This is the first step in my two step plan to split up the ComponentProvider into one that provides only the components that are needed to reach from cache and into one that provides the RemoteStore and other components needed for connectivity.

@changeset-bot
Copy link

changeset-bot bot commented Jul 16, 2020

💥 No Changeset

Latest commit: e77b203

Merging this PR will not cause any packages to be released. If these changes should not cause updates to packages in this repo, this is fine 🙂

If these changes should be published to npm, 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 Jul 16, 2020

Binary Size Report

Affected SDKs

  • @firebase/analytics

    Type Base (6d5b4cc) Head (aade2b3) Diff
    esm2017 8.68 kB 9.66 kB +975 B (+11.2%)
    main 9.69 kB 10.9 kB +1.20 kB (+12.3%)
    module 9.37 kB 10.6 kB +1.22 kB (+13.0%)
  • @firebase/firestore

    Type Base (6d5b4cc) Head (aade2b3) Diff
    browser 247 kB 247 kB +170 B (+0.1%)
    esm2017 194 kB 194 kB +90 B (+0.0%)
    main 495 kB 495 kB +409 B (+0.1%)
    module 245 kB 245 kB +170 B (+0.1%)
    react-native 194 kB 194 kB +90 B (+0.0%)
  • @firebase/firestore/exp

    Type Base (6d5b4cc) Head (aade2b3) Diff
    main 400 kB 401 kB +899 B (+0.2%)
  • @firebase/firestore/lite

    Type Base (6d5b4cc) Head (aade2b3) Diff
    main 124 kB 125 kB +694 B (+0.6%)
  • @firebase/firestore/memory

    Type Base (6d5b4cc) Head (aade2b3) Diff
    browser 185 kB 185 kB +164 B (+0.1%)
    esm2017 145 kB 145 kB +84 B (+0.1%)
    main 364 kB 364 kB +407 B (+0.1%)
    module 183 kB 183 kB +164 B (+0.1%)
    react-native 145 kB 145 kB +84 B (+0.1%)
  • @firebase/util

    Type Base (6d5b4cc) Head (aade2b3) Diff
    browser 19.6 kB 20.5 kB +921 B (+4.7%)
    esm2017 17.5 kB 18.3 kB +786 B (+4.5%)
    main 19.6 kB 20.5 kB +921 B (+4.7%)
    module 18.7 kB 19.5 kB +832 B (+4.5%)
  • firebase

    Type Base (6d5b4cc) Head (aade2b3) Diff
    firebase-analytics.js 26.6 kB 28.0 kB +1.37 kB (+5.1%)
    firebase-firestore.js 285 kB 285 kB +168 B (+0.1%)
    firebase-firestore.memory.js 224 kB 224 kB +162 B (+0.1%)
    firebase.js 819 kB 820 kB +1.54 kB (+0.2%)

Test Logs

const serializer = newSerializer(databaseInfo.databaseId);
const datastore = newDatastore(conn, credentialsProvider, serializer);
const datastore = newDatastore(credentialsProvider, serializer);
await datastore.start(conn);
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, how does this work with await? start doesn't seem to be async.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"await" returns the value that is being awaited even if it is not a Promise, so it should work. That being said, we should still drop it here. Thanks for catching.

@schmidt-sebastian schmidt-sebastian merged commit a875bbe into master Jul 17, 2020
@schmidt-sebastian schmidt-sebastian deleted the mrschmidt/movedatastore branch July 17, 2020 01:40
@firebase firebase locked and limited conversation to collaborators Aug 17, 2020
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.

3 participants