firebase / firebase/firebase-js-sdk

fix(firestore): Error messages reference 'FirebaseFirestore' instead of 'Firestore'

Open
#9,434 1 comment 0 reactions 1 assignee Claimed by @wu-hui View on GitHub
api: firestore needs-attention question
Dominant language
TypeScript
Stars
5.1k
Forks
1k
Avg merge
2d 21h
Merged PRs (30d)
37

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
1. The type is imported as `Firestore` (line 40): `import { Firestore } from './database';`
2. All function signatures use `firestore: Firestore`
3. JSDoc comments consistently refer to it as `Firestore`
4. 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:

1. Clone the repository:
```bash
git clone https://github.com/firebase/firebase-js-sdk.git
cd firebase-js-sdk
```

2. Check the error messages:
```bash
grep -n "FirebaseFirestore" packages/firestore/src/lite-api/reference.ts
```

**Output:**
```
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'
```

3. Compare with actual type name:
```bash
grep -n "import { Firestore }" packages/firestore/src/lite-api/reference.ts
```

**Output:**
```
40:import { Firestore } from './database';
```

### Code that would trigger this error (for context):
```typescript
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.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.