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

Implement useEmulator for Firestore #3909

Merged
merged 9 commits into from
Oct 16, 2020
Merged

Conversation

samtstern
Copy link
Contributor

@samtstern samtstern commented Oct 8, 2020

Discussion

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

Note: I did not feel comfortable adding this to firestore-exp because I couldn't quite understand the internals and it seems like we'll need to amend the API proposal to add something like EmulatorSettings since it looks like there is no longer a global settings object. Hoping @schmidt-sebastian can do that in a follow-up PR?

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 8, 2020

🦋 Changeset detected

Latest commit: 3332d63

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

This PR includes changesets to release 9 packages
Name Type
firebase Minor
@firebase/firestore Minor
@firebase/firestore-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

@github-actions
Copy link
Contributor

github-actions bot commented Oct 8, 2020

Changeset File Check ✅

No modified packages are missing from the changeset file.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Oct 8, 2020

Binary Size Report

Affected SDKs

  • @firebase/firestore

    Type Base (8939aec) Head (e47a7da) Diff
    browser 249 kB 249 kB +335 B (+0.1%)
    esm2017 197 kB 198 kB +317 B (+0.2%)
    main 484 kB 484 kB +420 B (+0.1%)
    module 246 kB 247 kB +335 B (+0.1%)
    react-native 197 kB 198 kB +317 B (+0.2%)
  • @firebase/firestore/memory

    Type Base (8939aec) Head (e47a7da) Diff
    browser 186 kB 186 kB +335 B (+0.2%)
    esm2017 147 kB 148 kB +317 B (+0.2%)
    main 356 kB 356 kB +420 B (+0.1%)
    module 184 kB 184 kB +335 B (+0.2%)
    react-native 148 kB 148 kB +317 B (+0.2%)
  • firebase

    Type Base (8939aec) Head (e47a7da) Diff
    firebase-firestore.js 286 kB 286 kB +340 B (+0.1%)
    firebase-firestore.memory.js 225 kB 225 kB +340 B (+0.2%)
    firebase.js 830 kB 831 kB +340 B (+0.0%)

Test Logs

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Oct 8, 2020

Size Analysis Report

Affected Products

No changes between base commit (8939aec) and head commit (e47a7da).

Test Logs

@schmidt-sebastian schmidt-sebastian self-assigned this Oct 14, 2020
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.

Basically LGTM. I think we should merge the two error checks that verify that the settings cannot be changed after instantiation.

packages/firestore/exp/test/shim.ts Show resolved Hide resolved
packages/firestore/src/api/database.ts Outdated Show resolved Hide resolved
@samtstern samtstern merged commit 79b0493 into master Oct 16, 2020
@google-oss-bot google-oss-bot mentioned this pull request Oct 22, 2020
@firebase firebase locked and limited conversation to collaborators Nov 16, 2020
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.

None yet

4 participants