Skip to content

Commit

Permalink
fix(firestore): skip ref extraction in non pojo
Browse files Browse the repository at this point in the history
Fix #1257
  • Loading branch information
posva committed Nov 29, 2022
1 parent ed59873 commit cc01b84
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 3 deletions.
5 changes: 3 additions & 2 deletions src/firestore/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
FirestoreDataConverter,
Timestamp,
} from 'firebase/firestore'
import { isObject, isDocumentRef, TODO } from '../shared'
import { isObject, isDocumentRef, TODO, isPOJO } from '../shared'
import { VueFirestoreDocumentData } from '.'

export type FirestoreReference = Query | DocumentReference | CollectionReference
Expand Down Expand Up @@ -42,7 +42,7 @@ export function extractRefs(
oldDoc: DocumentData | void,
subs: Record<string, { path: string; data: () => DocumentData | null }>
): [DocumentData, Record<string, DocumentReference>] {
if (!isObject(doc)) return [doc, {}]
if (!isPOJO(doc)) return [doc, {}]

const dataAndRefs: [DocumentData, Record<string, DocumentReference>] = [
{},
Expand Down Expand Up @@ -115,6 +115,7 @@ export function extractRefs(
refs,
])
} else if (isObject(ref)) {
// dive into nested refs
data[key] = {}
recursiveExtract(ref, oldDoc[key], path + key + '.', [data[key], refs])
} else {
Expand Down
10 changes: 10 additions & 0 deletions src/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,16 @@ export function isObject(o: unknown): o is Record<any, unknown> {
return !!o && typeof o === 'object'
}

const ObjectPrototype = Object.prototype
/**
* Check if an object is a plain js object. Differently from `isObject()`, this excludes class instances.
*
* @param obj - object to check
*/
export function isPOJO(obj: unknown): obj is Record<any, unknown> {
return isObject(obj) && Object.getPrototypeOf(obj) === ObjectPrototype
}

/**
* Checks if a variable is a Firestore Document Reference
* @param o
Expand Down
36 changes: 35 additions & 1 deletion tests/firestore/document.spec.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import { mount } from '@vue/test-utils'
import { describe, expect, it } from 'vitest'
import {
addDoc,
doc as originalDoc,
DocumentData,
DocumentReference,
FirestoreError,
} from 'firebase/firestore'
import { expectType, setupFirestoreRefs, tds, firestore } from '../utils'
import { nextTick, ref, shallowRef, unref, type Ref } from 'vue'
import { _MaybeRef, _Nullable } from '../../src/shared'
import { isPOJO, _MaybeRef, _Nullable } from '../../src/shared'
import {
useDocument,
VueFirestoreDocumentData,
Expand Down Expand Up @@ -147,6 +148,39 @@ describe(
expect(error.value).toBeUndefined()
})

it('can use a custom converter', async () => {
class MyName {
private _name: string
constructor(name: string) {
this._name = name
}

get name() {
return this._name
}

set name(_newName: string) {
// do nothing
}
}
const itemRef = doc().withConverter<MyName>({
toFirestore: (data) => ({ name: data.name }),
fromFirestore: (snap) => new MyName(snap.get('name')),
})
await setDoc(itemRef, new MyName('a'))

const { wrapper, data, promise } = factory({ ref: itemRef })

await promise.value

expect(wrapper.vm.item).toHaveProperty('name', 'a')
expect(isPOJO(wrapper.vm.item)).toBe(false)

// should respect the setter
wrapper.vm.item!.name = 'b'
expect(wrapper.vm.item).toHaveProperty('name', 'a')
})

describe('reset option', () => {
it('resets the value when specified', async () => {
const { wrapper, itemRef, data } = factory({
Expand Down

0 comments on commit cc01b84

Please sign in to comment.