-
Notifications
You must be signed in to change notification settings - Fork 390
Allow Firestore to auto-generate typings, separate internal vs external APIs #986
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but I don't see why the code changes in firestore-internal was needed. Can we revert them and just make it a simple file rename?
src/firestore/firestore-internal.ts
Outdated
|
||
import * as validator from '../utils/validator'; | ||
import * as utils from '../utils/index'; | ||
|
||
export function getFirestoreOptions(app: FirebaseApp): Settings { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does this file contain code changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, it shouldn't have any changes. I had originally put some code in firestore.ts
, but after realizing it wasn't necessary and adding it back in a different order polluted the diff. Fixed.
Thanks for the feedback. Also, for visibility, the Firestore service will require manual re-exporting in the barrel export even past this project because of the previous issue we ran into when prototyping this in #948 with the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. LGTM 👍
Context
The goal is to bridge the gap between today's Admin SDK (manually curated d.ts files, nested namespaces) and a modularized SDK that has auto-generated typings.
This is the eighth milestone from:
go/firebase-node-auto-typing
.Goals for this PR
Prerequisites
None.
Test Plan