fix(database, firestore): avoid id field name conflict (vuejs#1472)

Author: tlserver

src/database/utils.ts

@@ -15,15 +15,21 @@ export function createRecordFromDatabaseSnapshot(
 
   const value: unknown = snapshot.val()
   return isObject(value)
-    ? (Object.defineProperty(value, 'id', {
-        // allow destructuring without interfering without using the `id` property
-        value: snapshot.key,
+    ? (Object.defineProperties(value, {
+        // allow destructuring without interfering without using the `.key` property
+        '.key': { value: snapshot.key },
+        '.priority': { value: snapshot.priority },
+        '.ref': { value: snapshot.ref },
+        '.size': { value: snapshot.size },
       }) as VueDatabaseDocumentData<unknown>)
     : {
         // if the value is a primitive we can just return a regular object, it's easier to debug
         // @ts-expect-error: $value doesn't exist
         $value: value,
-        id: snapshot.key,
+        '.key': snapshot.key,
+        '.priority': snapshot.priority,
+        '.ref': snapshot.ref,
+        '.size': snapshot.size,
       }
 }
 
@@ -43,7 +49,7 @@ export function indexForKey(
   key: string | null | number
 ): number {
   for (let i = 0; i < array.length; i++) {
-    if (array[i].id === key) return i
+    if (array[i]['.key'] === key) return i
   }
 
   return -1
@@ -58,9 +64,25 @@ export type VueDatabaseDocumentData<T = unknown> =
   | null
   | (T & {
       /**
-       * id of the document
+       * The key (last part of the path) of the location of this DataSnapshot.The last token in a Database location is
+       * considered its key. For example, "ada" is the key for the /users/ada/ node. Accessing the key on any
+       * DataSnapshot will return the key for the location that generated it. However, accessing the key on the root URL
+       * of a Database will return null.
        */
-      readonly id: string
+      readonly '.key': string
+      /**
+       * The priority value of the data in this DataSnapshot.Applications need not use priority but can order
+       * collections by ordinary properties.
+       */
+      readonly '.priority': string
+      /**
+       * The location of this document data.
+       */
+      readonly '.ref': string
+      /**
+       * The number of child properties of this document data.
+       */
+      readonly '.size': string
     })
 
 /**

src/firestore/useFirestoreRef.ts

@@ -229,7 +229,15 @@ export type VueFirestoreDocumentData<T = DocumentData> =
       /**
        * id of the document
        */
-      readonly id: string
+      readonly '.id': string
+      /**
+       * Metadata about the DocumentSnapshot, including information about its source and local modifications.
+       */
+      readonly '.metadata': string
+      /**
+       * The DocumentReference for the document
+       */
+      readonly '.ref': string
     })
 
 export type VueFirestoreQueryData<T = DocumentData> = Array<

src/firestore/utils.ts

@@ -24,17 +24,17 @@ export const firestoreDefaultConverter: FirestoreDataConverter<VueFirestoreDocum
       return data as DocumentData
     },
     fromFirestore(snapshot, options) {
-      return snapshot.exists()
-        ? (Object.defineProperties(snapshot.data(options)!, {
-            id: { value: snapshot.id },
-            // TODO: check if worth adding or should be through an option
-            // It could also be an example in the docs about converters
-            // $meta: {
-            //   value: snapshot.metadata,
-            // },
-            // $ref: { get: () => snapshot.ref },
-          }) as VueFirestoreDocumentData)
-        : null
+      if (snapshot.exists()) {
+        const data = snapshot.data(options)!
+        Object.defineProperties(data, {
+          '.id': { value: snapshot.id },
+          '.metadata': { value: snapshot.metadata },
+          '.ref': { value: snapshot.ref },
+        })
+        return data as VueFirestoreDocumentData
+      } else {
+        return null
+      }
     },
   }
 

tests/database/list.spec.ts

@@ -97,9 +97,9 @@ describe('Database lists', () => {
     await promise.value
 
     expect(wrapper.vm.list).toMatchObject([
-      { $value: 'a', id: expect.any(String) },
-      { $value: 'b', id: expect.any(String) },
-      { $value: 'c', id: expect.any(String) },
+      { $value: 'a', '.key': expect.any(String) },
+      { $value: 'b', '.key': expect.any(String) },
+      { $value: 'c', '.key': expect.any(String) },
     ])
   })
 
@@ -169,8 +169,8 @@ describe('Database lists', () => {
 
     const a = await push(listRef, { name: 'a' })
     expect(wrapper.vm.list).toHaveLength(1)
-    expect(data.value[0].id).toBeTypeOf('string')
-    expect(data.value[0].id).toEqual(a.key)
+    expect(data.value[0]['.key']).toBeTypeOf('string')
+    expect(data.value[0]['.key']).toEqual(a.key)
   })
 
   it('unbinds when the component is unmounted', async () => {
@@ -397,7 +397,7 @@ describe('Database lists', () => {
       useDatabaseList(databaseRef(db, 'todos'))
     )
     expectType<string | undefined>(
-      useDatabaseList(databaseRef(db, 'todos')).value?.[0]?.id
+      useDatabaseList(databaseRef(db, 'todos')).value?.[0]?.['.key']
     )
     expectType<Ref<VueDatabaseQueryData<number>>>(
       useDatabaseList<number>(databaseRef(db, 'todos'))

tests/database/objects.spec.ts

@@ -147,7 +147,7 @@ describe('Database objects', () => {
 
     expect(wrapper.vm.item).toMatchObject({
       $value: 24,
-      id: itemRef.key,
+      '.key': itemRef.key,
     })
   })
 
@@ -294,7 +294,7 @@ describe('Database objects', () => {
       useDatabaseObject<{ name: string }>(databaseRef(db, 'todo'))
     )
     expectType<undefined | string>(
-      useDatabaseObject(databaseRef(db, 'todo')).value?.id
+      useDatabaseObject(databaseRef(db, 'todo')).value?.['.key']
     )
     expectType<Ref<number | null | undefined>>(
       useDatabaseObject<number>(databaseRef(db, 'todo'))

tests/firestore/collection.spec.ts

@@ -229,8 +229,8 @@ describe(
 
       const a = await addDoc(listRef, { name: 'a' })
       expect(wrapper.vm.list).toHaveLength(1)
-      expect(data.value[0].id).toBeTypeOf('string')
-      expect(data.value[0].id).toEqual(a.id)
+      expect(data.value[0]['.id']).toBeTypeOf('string')
+      expect(data.value[0]['.id']).toEqual(a.id)
     })
 
     it('unbinds when the component is unbound', async () => {
@@ -472,13 +472,14 @@ describe(
 
       // Adds the id
       // FIXME: this one is any but the test passes
-      expectType<string>(useCollection(collection(db, 'todos')).value[0].id)
+      expectType<string>(useCollection(collection(db, 'todos')).value[0]['.id'])
       expectType<string>(
-        useCollection<TodoI>(collection(db, 'todos')).value[0].id
+        useCollection<TodoI>(collection(db, 'todos')).value[0]['.id']
       )
       expectType<string>(
-        useCollection<unknown>(collection(db, 'todos')).value[0].id
+        useCollection<unknown>(collection(db, 'todos')).value[0]['.id']
       )
+      // @ts-expect-error: no id with custom converter
       useCollection(
         collection(db, 'todos').withConverter<TodoI, DocumentData>({
           fromFirestore: (snapshot) => {
@@ -487,18 +488,17 @@ describe(
           },
           toFirestore: (todo) => todo,
         })
-        // @ts-expect-error: no id with custom converter
-      ).value[0].id
+      ).value[0]['.id']
 
       expectType<Ref<TodoI[]>>(useCollection<TodoI>(collection(db, 'todos')))
       expectType<Ref<TodoI[]>>(
         useCollection<TodoI>(collection(db, 'todos')).data
       )
       expectType<string>(
-        useCollection<TodoI>(collection(db, 'todos')).value.at(0)!.id
+        useCollection<TodoI>(collection(db, 'todos')).value.at(0)!['.id']
       )
       expectType<string>(
-        useCollection<TodoI>(collection(db, 'todos')).data.value.at(0)!.id
+        useCollection<TodoI>(collection(db, 'todos')).data.value.at(0)!['.id']
       )
       // @ts-expect-error: wrong type
       expectType<Ref<string[]>>(useCollection<TodoI>(collection(db, 'todos')))
@@ -513,7 +513,7 @@ describe(
       expectType<Ref<number[]>>(useCollection(refWithConverter))
       expectType<Ref<number[]>>(useCollection(refWithConverter).data)
       // @ts-expect-error: no id with converter
-      expectType<Ref<number[]>>(useCollection(refWithConverter).data.value.id)
+      expectType<Ref<number[]>>(useCollection(refWithConverter).data.value['.id'])
       // @ts-expect-error
       expectType<Ref<string[]>>(useCollection(refWithConverter))
     })

tests/firestore/document.spec.ts

@@ -140,7 +140,7 @@ describe(
       const { itemRef, data } = factory()
 
       await setDoc(itemRef, { name: 'a' })
-      expect(data.value!.id).toBe(itemRef.id)
+      expect(data.value!['.id']).toBe(itemRef.id)
     })
 
     it('sets pending while loading', async () => {
@@ -347,12 +347,13 @@ describe(
 
       // Adds the id
       // FIXME: this one is any but the test passes
-      expectType<string>(useDocument(doc(db, 'todos', '1')).value?.id)
-      expectType<string>(useDocument<TodoI>(doc(db, 'todos', '1')).value!.id)
+      expectType<string>(useDocument(doc(db, 'todos', '1')).value?.['.id'])
+      expectType<string>(useDocument<TodoI>(doc(db, 'todos', '1')).value!['.id'])
       expectType<_Nullable<TodoI>>(
         useDocument<TodoI>(doc(db, 'todos', '1')).value
       )
-      expectType<string>(useDocument<unknown>(doc(db, 'todos', '1')).value!.id)
+      expectType<string>(useDocument<unknown>(doc(db, 'todos', '1')).value!['.id'])
+      // @ts-expect-error: no id with custom converter
       useDocument(
         doc(db, 'todos').withConverter<TodoI, DocumentData>({
           fromFirestore: (snapshot) => {
@@ -361,8 +362,7 @@ describe(
           },
           toFirestore: (todo) => todo,
         })
-        // @ts-expect-error: no id with custom converter
-      ).value?.id
+      ).value?.['.id']
 
       expectType<Ref<number | null | undefined>>(useDocument<number>(itemRef))
       expectType<Ref<number | null | undefined>>(
@@ -386,7 +386,7 @@ describe(
       // @ts-expect-error: string is not assignable to number
       expectType<Ref<string>>(useDocument(refWithConverter))
       // @ts-expect-error: no id when a custom converter is used
-      useDocument(refWithConverter).value.id
+      useDocument(refWithConverter).value['.id']
 
       // destructuring
       expectType<Ref<DocumentData | null | undefined>>(

tests/firestore/refs-in-documents.spec.ts

@@ -120,8 +120,8 @@ describe('Firestore refs in documents', async () => {
     await promise.value
     // NOTE: why does toEqual fail here?
     expect(data.value).toMatchObject({
-      id: docRef.id,
-      a: { name: 'a', id: aRef.id },
+      '.id': docRef.id,
+      a: { name: 'a', '.id': aRef.id },
     })
   })
 

tests/firestore/utils.spec.ts

@@ -30,12 +30,12 @@ describe('Firestore and Database utils', () => {
 
   it('createSnapshot adds an id', async () => {
     const snapshot = await addDocToCollection()
-    expect(snapshot.data()?.id).not.toBeFalsy()
+    expect(snapshot.data()?.['.id']).not.toBeFalsy()
   })
 
   it('id is not enumerable', async () => {
     const snapshot = await addDocToCollection()
-    expect(Object.keys(snapshot.data() ?? {}).includes('id')).toBe(false)
+    expect(Object.keys(snapshot.data() ?? {}).includes('.id')).toBe(false)
   })
 
   it('contains all the data', async () => {