elersong / fireorm24

ORM for Firebase Firestore 🔥 updated for 2024
MIT License
1 stars 0 forks source link

Enable strictNullChecks for Better Type Safety #35

Open elersong opened 3 months ago

elersong commented 3 months ago

Description

The findById method in BaseFirestoreRepository is expected to return Promise<T | null>. However, the generated TypeScript declaration file shows Promise<T>, losing the benefit of type safety. This issue is likely due to strictNullChecks being set to false. Enabling strictNullChecks and fixing the resulting errors will improve type safety throughout the library.

Steps to Reproduce

  1. Check the findById method in BaseFirestoreRepository.
  2. Observe that it is expected to return Promise<T | null>.
  3. Build the library and check the generated declaration file.
  4. Notice that the return type is Promise<T> instead of Promise<T | null>.

Expected Behavior

The findById method should correctly reflect Promise<T | null> in the generated declaration file, ensuring proper type safety.

Actual Behavior

The findById method's return type is Promise<T> in the generated declaration file, losing the benefit of type safety.

Acceptance Criteria

Additional Context

Proposed API Changes

  1. Enable strictNullChecks:

    • Update the tsconfig.json file to enable strictNullChecks.
    {
     "compilerOptions": {
       "strictNullChecks": true,
       // Other compiler options...
     }
    }
  2. Fix Resulting Errors:

    • Address the errors that occur when enabling strictNullChecks to ensure the library builds successfully.
    // Example fix for the findById method
    async findById(id: string): Promise<T | null> {
     return this.firestoreColRef
       .doc(id)
       .get()
       .then(d => (d.exists ? this.extractTFromDocSnap(d) : null));
    }
  3. Validate Return Types:

    • Ensure the correct return types are reflected in the generated declaration files.
    // Check the generated declaration file
    declare class BaseFirestoreRepository<T> {
     findById(id: string): Promise<T | null>;
    }
  4. Unit Tests:

    • Add unit tests to validate the correct return types and behavior of methods.
    test('findById should return null when document does not exist', async () => {
     const repo = getRepository(MyEntity);
     const result = await repo.findById('nonexistent-id');
     expect(result).toBeNull();
    });
    
    test('findById should return entity when document exists', async () => {
     const repo = getRepository(MyEntity);
     const entity = new MyEntity();
     entity.id = 'existent-id';
     await repo.create(entity);
    
     const result = await repo.findById('existent-id');
     expect(result).toEqual(entity);
    });

Example Implementation

class MyEntity {
  id: string;
}

@Collection("myEntities")
class MyEntityRepository extends BaseFirestoreRepository<MyEntity> {}

const repo = getRepository(MyEntity);

const entity = new MyEntity();
entity.id = "existent-id";
await repo.create(entity);

const result = await repo.findById("existent-id");
console.log(result); // Output: MyEntity instance or null

Original Issue