-
Notifications
You must be signed in to change notification settings - Fork 994
Description
Operating System
All operating systems (Linux, macOS, Windows)
Environment (if applicable)
Node.js 20.12.2 (as specified in repository) Affects both browser and Node.js environments
Firebase SDK Version
Latest (main branch / v12.7.0)
Firebase SDK Product(s)
Firestore
Project Tooling
TypeScript, Yarn v1.22.22
Detailed Problem Description
The error messages in packages/firestore/src/lite-api/reference.ts use the outdated v8 naming convention "FirebaseFirestore" instead of the v9 modular API name "Firestore".
Location
- Line 516-517 in
collection()function - Line 654-655 in
doc()function
What I was trying to achieve
While reviewing the codebase, I noticed inconsistency between the type names used in error messages and the actual type names in the v9 modular API.
What actually happened
The error messages still reference the old v8 name "FirebaseFirestore":
Expected first argument to collection() to be a CollectionReference, a DocumentReference or FirebaseFirestore
Expected behavior
The error messages should use the correct v9 type name "Firestore":
Expected first argument to collection() to be a CollectionReference, a DocumentReference or Firestore
Evidence of inconsistency
- The type is imported as
Firestore(line 40):import { Firestore } from './database'; - All function signatures use
firestore: Firestore - JSDoc comments consistently refer to it as
Firestore - The class definition is
export class Firestore(packages/firestore/src/lite-api/database.ts:68)
Impact
- Confuses developers using the v9 modular API
- Creates inconsistency between error messages and actual type names
- Makes error messages less helpful for debugging
Steps and code to reproduce issue
This is a static code issue (incorrect naming in error messages), but here's how to observe it:
Steps to verify the issue:
- Clone the repository:
git clone https://github.com/firebase/firebase-js-sdk.git
cd firebase-js-sdk- Check the error messages:
grep -n "FirebaseFirestore" packages/firestore/src/lite-api/reference.tsOutput:
516: 'Expected first argument to collection() to be a CollectionReference, ' +
517: 'a DocumentReference or FirebaseFirestore'
654: 'Expected first argument to doc() to be a CollectionReference, ' +
655: 'a DocumentReference or Firestore'
- Compare with actual type name:
grep -n "import { Firestore }" packages/firestore/src/lite-api/reference.tsOutput:
40:import { Firestore } from './database';
Code that would trigger this error (for context):
import { collection } from 'firebase/firestore';
// Passing undefined or invalid first argument
collection(undefined, 'users');
// Error: Expected first argument to collection() to be a CollectionReference,
// a DocumentReference or FirebaseFirestore
// ^^^^^^^^^^^^^^^^^ <- Should say "Firestore"Proposed fix
Replace "FirebaseFirestore" with "Firestore" in both locations (lines 517 and 655).
I'd be happy to submit a PR to fix this if the maintainers agree this should be changed.