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.

Sign up for GitHub

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

Jump to bottom

Implement useEmulator for Database #3904

Merged
merged 17 commits into from
Oct 20, 2020
Merged

Implement useEmulator for Database #3904

merged 17 commits into from
Oct 20, 2020

Conversation

samtstern
Copy link
Contributor

@samtstern samtstern commented Oct 7, 2020 •
edited
Loading

Discussion

This PR is part of a group: #3904 #3906 #3909

In order for useEmulator() to work the Repo has to be changed or replaced. In Android this was easier because the database instance lazily creates the Repo (via ensureRepo()) but in JS the Repo creates the database instance.

In this change I move the repo_ and root_ fields to have getters/setters so that we can track their usage and allow useEmulator() to replace the Repo only if no database operations have been performed yet.

Android implementation:
firebase/firebase-android-sdk#1802

This follows Proposal 2 at:
http://go/firebase-emulator-connection-api

Testing

  • Added new unit tests

API Changes

  • Approved, see above

@changeset-bot
Copy link

changeset-bot bot commented Oct 7, 2020 •
edited
Loading

🦋 Changeset detected

Latest commit: e965bbd

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

This PR includes changesets to release 8 packages
Name Type
firebase Minor
@firebase/database Minor
@firebase/database-types Minor
@firebase/rules-unit-testing 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

@samtstern samtstern requested a review from davideast October 7, 2020 18:25
@google-oss-bot
Copy link
Contributor

google-oss-bot commented Oct 7, 2020 •
edited
Loading

Binary Size Report

Affected SDKs

  • @firebase/analytics

    Type Base (f5d122a) Head (1614086) Diff
    esm2017 18.6 kB 18.6 kB +69 B (+0.4%)
    main 23.8 kB 23.8 kB +37 B (+0.2%)
    module 23.3 kB 23.3 kB +37 B (+0.2%)

  • @firebase/app

    Type Base (f5d122a) Head (1614086) Diff
    browser 11.1 kB 11.1 kB +2 B (+0.0%)
    esm2017 9.46 kB 9.48 kB +22 B (+0.2%)
    lite-esm2017 7.75 kB 7.77 kB +20 B (+0.3%)
    module 11.0 kB 11.0 kB +2 B (+0.0%)

  • @firebase/auth

    Type Base (f5d122a) Head (1614086) Diff
    browser 177 kB 179 kB +2.05 kB (+1.2%)
    main 177 kB 179 kB +2.05 kB (+1.2%)
    module 177 kB 179 kB +2.05 kB (+1.2%)

  • @firebase/component

    Type Base (f5d122a) Head (1614086) Diff
    esm2017 3.98 kB 3.99 kB +8 B (+0.2%)

  • @firebase/database

    Type Base (f5d122a) Head (1614086) Diff
    browser 270 kB 271 kB +1.04 kB (+0.4%)
    esm2017 236 kB 238 kB +1.14 kB (+0.5%)
    main 271 kB 272 kB +1.04 kB (+0.4%)
    module 269 kB 270 kB +1.04 kB (+0.4%)

  • @firebase/firestore

    Type Base (f5d122a) Head (1614086) Diff
    browser 249 kB 249 kB +11 B (+0.0%)
    esm2017 197 kB 198 kB +1.22 kB (+0.6%)
    main 484 kB 484 kB +532 B (+0.1%)
    module 247 kB 247 kB +69 B (+0.0%)
    react-native 197 kB 198 kB +1.22 kB (+0.6%)

  • @firebase/firestore/exp

    Type Base (f5d122a) Head (1614086) Diff
    browser 189 kB 190 kB +1.01 kB (+0.5%)
    main 477 kB 478 kB +1.04 kB (+0.2%)
    module 189 kB 190 kB +1.01 kB (+0.5%)
    react-native 189 kB 190 kB +1.01 kB (+0.5%)

  • @firebase/firestore/lite

    Type Base (f5d122a) Head (1614086) Diff
    browser 63.4 kB 63.5 kB +94 B (+0.1%)
    main 140 kB 140 kB -91 B (-0.1%)
    module 63.4 kB 63.5 kB +94 B (+0.1%)
    react-native 63.6 kB 63.7 kB +94 B (+0.1%)

  • @firebase/firestore/memory

    Type Base (f5d122a) Head (1614086) Diff
    browser 187 kB 186 kB -594 B (-0.3%)
    esm2017 147 kB 148 kB +398 B (+0.3%)
    main 357 kB 356 kB -835 B (-0.2%)
    module 185 kB 184 kB -508 B (-0.3%)
    react-native 147 kB 148 kB +395 B (+0.3%)

  • @firebase/functions

    Type Base (f5d122a) Head (1614086) Diff
    browser 10.0 kB 10.1 kB +95 B (+0.9%)
    esm2017 7.60 kB 7.69 kB +90 B (+1.2%)
    main 10.2 kB 10.3 kB +95 B (+0.9%)
    module 9.78 kB 9.87 kB +95 B (+1.0%)

  • @firebase/installations

    Type Base (f5d122a) Head (1614086) Diff
    esm2017 16.5 kB 16.6 kB +55 B (+0.3%)
    main 22.1 kB 22.1 kB +33 B (+0.1%)
    module 21.5 kB 21.6 kB +33 B (+0.2%)

  • @firebase/logger

    Type Base (f5d122a) Head (1614086) Diff
    esm2017 3.25 kB 3.25 kB +6 B (+0.2%)

  • @firebase/messaging

    Type Base (f5d122a) Head (1614086) Diff
    esm2017 25.9 kB 26.0 kB +20 B (+0.1%)

  • @firebase/performance

    Type Base (f5d122a) Head (1614086) Diff
    esm2017 25.8 kB 25.8 kB +66 B (+0.3%)

  • @firebase/remote-config

    Type Base (f5d122a) Head (1614086) Diff
    browser 22.8 kB 22.8 kB +26 B (+0.1%)
    esm2017 17.4 kB 17.4 kB +50 B (+0.3%)
    main 22.8 kB 22.8 kB +26 B (+0.1%)
    module 22.4 kB 22.4 kB +26 B (+0.1%)

  • @firebase/rules-unit-testing

    Type Base (f5d122a) Head (1614086) Diff
    main 7.84 kB 7.86 kB +18 B (+0.2%)

  • @firebase/storage

    Type Base (f5d122a) Head (1614086) Diff
    esm2017 54.9 kB 55.0 kB +78 B (+0.1%)

  • @firebase/util

    Type Base (f5d122a) Head (1614086) Diff
    browser 21.1 kB 21.2 kB +92 B (+0.4%)
    esm2017 18.8 kB 18.9 kB +146 B (+0.8%)
    main 21.2 kB 21.3 kB +92 B (+0.4%)
    module 20.1 kB 20.1 kB +76 B (+0.4%)

  • @firebase/webchannel-wrapper

    Type Base (f5d122a) Head (1614086) Diff
    esm2017 39.4 kB 39.3 kB -30 B (-0.1%)
    main 41.0 kB 46.7 kB +5.66 kB (+13.8%)
    module 40.6 kB 40.6 kB -32 B (-0.1%)

  • firebase

    Click to show 15 binary size changes.
    Type Base (f5d122a) Head (1614086) Diff
    firebase-analytics.js 35.8 kB 35.7 kB -99 B (-0.3%)
    firebase-app.js 20.1 kB 20.0 kB -166 B (-0.8%)
    firebase-auth.js 174 kB 176 kB +2.01 kB (+1.2%)
    firebase-database.js 190 kB 190 kB +834 B (+0.4%)
    firebase-firestore.js 286 kB 286 kB -335 B (-0.1%)
    firebase-firestore.memory.js 226 kB 225 kB -897 B (-0.4%)
    firebase-functions.js 10.1 kB 9.99 kB -91 B (-0.9%)
    firebase-installations.js 19.1 kB 19.0 kB -135 B (-0.7%)
    firebase-messaging.js 40.9 kB 40.7 kB -135 B (-0.3%)
    firebase-performance-standalone.es2017.js 71.8 kB 71.7 kB -109 B (-0.2%)
    firebase-performance-standalone.js 48.2 kB 48.1 kB -135 B (-0.3%)
    firebase-performance.js 38.6 kB 38.5 kB -135 B (-0.3%)
    firebase-remote-config.js 36.9 kB 36.8 kB -110 B (-0.3%)
    firebase-storage.js 39.7 kB 39.6 kB -168 B (-0.4%)
    firebase.js 829 kB 832 kB +3.00 kB (+0.4%)

Test Logs

@google-oss-bot

This comment has been minimized.

Sign in to view
This was referenced Oct 8, 2020
Merged
Merged
@schmidt-sebastian schmidt-sebastian self-assigned this Oct 14, 2020
schmidt-sebastian
schmidt-sebastian reviewed Oct 14, 2020
View reviewed changes
Copy link
Contributor

@schmidt-sebastian schmidt-sebastian left a comment

Choose a reason for hiding this comment

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

Do you have thoughts on this: https://gist.github.com/schmidt-sebastian/b84902bf402a887ee2b3bc6591a51068

This brings this repo a little close in line with Android (or does it - but it seems simpler).

packages/database/src/api/Database.ts Outdated Show resolved Hide resolved
packages/firebase/index.d.ts Outdated Show resolved Hide resolved
@samtstern
Copy link
Contributor Author

Quoting @schmidt-sebastian from chat:

Reserving seems like a good idea, but I am not sure how complex this would be - but I am sure that it will be more complex than your initial commit
Let's go back to that with some minor tweaks:
s/instanceUsed/started
and can you use the same applyEmulatorSettings naming and logic as you did on Android?
so no clone/delete but rather a mutable Repo class

@samtstern samtstern mentioned this pull request Oct 19, 2020
Closed
@schmidt-sebastian
Copy link
Contributor

@Feiyang1 - Looks like we need more approvals.

@schmidt-sebastian schmidt-sebastian merged commit ef33328 into master Oct 20, 2020
@google-oss-bot google-oss-bot mentioned this pull request Oct 22, 2020
Merged
@schmidt-sebastian schmidt-sebastian deleted the ss-use-emulator-database branch November 9, 2020 22:36
@firebase firebase locked and limited conversation to collaborators Nov 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@schmidt-sebastian schmidt-sebastian schmidt-sebastian approved these changes

@Feiyang1 Feiyang1 Feiyang1 approved these changes

@jsdt jsdt Awaiting requested review from jsdt

@davideast davideast Awaiting requested review from davideast

Assignees

@schmidt-sebastian schmidt-sebastian

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@samtstern @google-oss-bot @schmidt-sebastian @Feiyang1