Skip to content

Commit

Permalink
feat: warn wrong usage useDocument(), ...
Browse files Browse the repository at this point in the history
  • Loading branch information
posva committed Nov 28, 2022
1 parent d96a506 commit 098c16c
Show file tree
Hide file tree
Showing 8 changed files with 211 additions and 18 deletions.
14 changes: 11 additions & 3 deletions src/database/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
} from 'vue-demi'
import { DatabaseReference, getDatabase, Query } from 'firebase/database'
import {
checkWrittenTarget,
isSSR,
noop,
OperationsType,
Expand Down Expand Up @@ -57,15 +58,22 @@ export function _useDatabaseRef(
) {
let unbind!: UnbindWithReset
const options = Object.assign({}, databaseOptionsDefaults, localOptions)
const initialSourceValue = unref(reference)
const data = options.target || ref<unknown | null>()

// dev only warning
if (process.env.NODE_ENV !== 'production') {
// is the target a ref that has already been passed to useDocument() and therefore can't be extended anymore
if (options.target && checkWrittenTarget(data, 'useObject()/useList()')) {
return data
}
}

// During SSR, we should only get data once
if (isSSR()) {
options.once = true
}

const initialSourceValue = unref(reference)

const data = options.target || ref<unknown | null>()
// set the initial value from SSR even if the ref comes from outside
data.value = getInitialValue(initialSourceValue, options.ssrKey, data.value)

Expand Down
14 changes: 13 additions & 1 deletion src/firestore/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
} from 'vue-demi'
import { useFirebaseApp } from '../app'
import {
checkWrittenTarget,
isDocumentRef,
isSSR,
noop,
Expand Down Expand Up @@ -72,12 +73,23 @@ export function _useFirestoreRef(
let unbind: UnbindWithReset = noop
const options = Object.assign({}, firestoreOptionsDefaults, localOptions)
const initialSourceValue = unref(docOrCollectionRef)
const data = options.target || ref<unknown | null>()

// dev only warning
if (process.env.NODE_ENV !== 'production') {
// is the target a ref that has already been passed to useDocument() and therefore can't be extended anymore
if (
options.target &&
checkWrittenTarget(data, 'useDocument()/useCollection()')
) {
return data
}
}

if (isSSR()) {
options.once = true
}

const data = options.target || ref<unknown | null>()
// set the initial value from SSR even if the ref comes from outside
data.value = getInitialValue(initialSourceValue, options.ssrKey, data.value)

Expand Down
19 changes: 19 additions & 0 deletions src/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -274,3 +274,22 @@ export interface _ResolveRejectFn {
export function isSSR(): boolean {
return !!(getCurrentInstance() && inject(ssrContextKey, null))
}

/**
* Checks and warns if a data ref has already bee overwritten by useDocument() and others.
*
* @internal
*/
export function checkWrittenTarget(
data: Ref<unknown>,
fnName: string
): boolean {
if (Object.getOwnPropertyDescriptor(data, 'data')?.get?.() === data) {
console.warn(`[VueFire] the passed "options.target" is already the returned value of "${fnName}". If you want to subscribe to a different data source, pass a reactive variable to "${fnName}" instead:
https://vuefire.vuejs.org/guide/realtime-data.html#declarative-realtime-data
This will FAIL in production.`)
return true
}

return false
}
12 changes: 12 additions & 0 deletions tests/database/list.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@ import {
ref as _databaseRef,
} from 'firebase/database'
import { _MaybeRef } from '../../src/shared'
import { mockWarn } from '../vitest-mock-warn'

describe('Database lists', () => {
const { databaseRef, push, set, remove, update } = setupDatabaseRefs()
mockWarn()

function factory<T = unknown>({
options,
Expand Down Expand Up @@ -74,6 +76,16 @@ describe('Database lists', () => {
expect(wrapper.vm.list).toHaveLength(2)
})

it('warns if target is the result of useDocument', async () => {
const target = ref()
const { data, listRef } = factory({ options: { target } })

expect(data).toBe(target)

expect(() => useList(listRef, { target })).not.toThrow()
expect(/FAIL/).toHaveBeenWarned()
})

it('add items to the list', async () => {
const { wrapper, data, listRef } = factory()

Expand Down
12 changes: 12 additions & 0 deletions tests/database/objects.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@ import { expectType, tds, setupDatabaseRefs, database } from '../utils'
import { computed, nextTick, ref, shallowRef, unref, type Ref } from 'vue'
import { DatabaseReference, ref as _databaseRef } from 'firebase/database'
import { _MaybeRef, _Nullable } from '../../src/shared'
import { mockWarn } from '../vitest-mock-warn'

describe('Database objects', () => {
const { databaseRef, set, update, remove } = setupDatabaseRefs()
mockWarn()

function factory<T = unknown>({
options,
Expand Down Expand Up @@ -56,6 +58,16 @@ describe('Database objects', () => {
expect(wrapper.vm.item).toMatchObject({ name: 'b' })
})

it('warns if target is the result of another composable', async () => {
const target = ref()
const { data, itemRef } = factory({ options: { target } })

expect(data).toBe(target)

expect(() => useObject(itemRef, { target })).not.toThrow()
expect(/FAIL/).toHaveBeenWarned()
})

// TODO: right now this creates an object with a .value property equal to null
it.todo('stays null if it does not exist', async () => {
const { wrapper, itemRef } = factory()
Expand Down
26 changes: 13 additions & 13 deletions tests/firestore/collection.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,16 @@ import {
VueFirestoreQueryData,
} from '../../src'
import { _MaybeRef, _Nullable } from '../../src/shared'
import { mockWarn } from '../vitest-mock-warn'

describe(
'Firestore collections',
() => {
const { collection, query, addDoc, setDoc, updateDoc, deleteDoc } =
setupFirestoreRefs()

mockWarn()

function factory<T = DocumentData>({
options,
ref = collection(),
Expand Down Expand Up @@ -92,19 +95,6 @@ describe(
}
}

function sortedList<
A extends Array<Record<any, unknown>>,
K extends keyof A[any]
>(list: A, key: K) {
return list.slice().sort((a, b) => {
const aVal = a[key]
const bVal = b[key]
return typeof aVal === 'string' && typeof bVal === 'string'
? aVal.localeCompare(bVal)
: 0
})
}

it('add items to the collection', async () => {
const { wrapper, listRef } = factory<{ name: string }>()

Expand All @@ -117,6 +107,16 @@ describe(
expect(wrapper.vm.list).toContainEqual({ name: 'c' })
})

it('warns if target is the result of useDocument', async () => {
const target = ref()
const { data, listRef } = factory({ options: { target } })

expect(data).toBe(target)

expect(() => useCollection(listRef, { target })).not.toThrow()
expect(/FAIL/).toHaveBeenWarned()
})

it('delete items from the collection', async () => {
const { wrapper, listRef } = factory<{ name: string }>()

Expand Down
14 changes: 13 additions & 1 deletion tests/firestore/document.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,21 @@ import {
FirestoreError,
} from 'firebase/firestore'
import { expectType, setupFirestoreRefs, tds, firestore } from '../utils'
import { nextTick, shallowRef, unref, type Ref } from 'vue'
import { nextTick, ref, shallowRef, unref, type Ref } from 'vue'
import { _MaybeRef } from '../../src/shared'
import {
useDocument,
VueFirestoreDocumentData,
UseDocumentOptions,
_RefFirestore,
} from '../../src'
import { mockWarn } from '../vitest-mock-warn'

describe(
'Firestore documents',
() => {
const { doc, deleteDoc, setDoc, updateDoc } = setupFirestoreRefs()
mockWarn()

function factory<T = DocumentData>({
options,
Expand Down Expand Up @@ -65,6 +67,16 @@ describe(
expect(data.value).toEqual({ name: 'b' })
})

it('warns if target is the result of useDocument', async () => {
const target = ref()
const { data, itemRef } = factory({ options: { target } })

expect(data).toBe(target)

expect(() => useDocument(itemRef, { target })).not.toThrow()
expect(/FAIL/).toHaveBeenWarned()
})

it('fetches once', async () => {
const itemRef = doc<{ name: string }>()
await setDoc(itemRef, { name: 'a' })
Expand Down
118 changes: 118 additions & 0 deletions tests/vitest-mock-warn.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
// https://github.com/posva/jest-mock-warn/blob/master/src/index.js

import { afterEach, beforeEach, expect, SpyInstance, vi } from 'vitest'

export function mockWarn() {
expect.extend({
toHaveBeenWarned(received: string | RegExp) {
asserted.set(received.toString(), received)
const passed = warn.mock.calls.some((args) =>
typeof received === 'string'
? args[0].indexOf(received) > -1
: received.test(args[0])
)
if (passed) {
return {
pass: true,
message: () => `expected "${received}" not to have been warned.`,
}
} else {
const msgs = warn.mock.calls.map((args) => args[0]).join('\n - ')
return {
pass: false,
message: () =>
`expected "${received}" to have been warned.\n\nActual messages:\n\n - ${msgs}`,
}
}
},

toHaveBeenWarnedLast(received: string | RegExp) {
asserted.set(received.toString(), received)
const lastCall = warn.mock.calls[warn.mock.calls.length - 1][0]
const passed =
typeof received === 'string'
? lastCall.indexOf(received) > -1
: received.test(lastCall)
if (passed) {
return {
pass: true,
message: () => `expected "${received}" not to have been warned last.`,
}
} else {
const msgs = warn.mock.calls.map((args) => args[0]).join('\n - ')
return {
pass: false,
message: () =>
`expected "${received}" to have been warned last.\n\nActual messages:\n\n - ${msgs}`,
}
}
},

toHaveBeenWarnedTimes(received: string | RegExp, n: number) {
asserted.set(received.toString(), received)
let found = 0
warn.mock.calls.forEach((args) => {
const isFound =
typeof received === 'string'
? args[0].indexOf(received) > -1
: received.test(args[0])
if (isFound) {
found++
}
})

if (found === n) {
return {
pass: true,
message: () =>
`expected "${received}" to have been warned ${n} times.`,
}
} else {
return {
pass: false,
message: () =>
`expected "${received}" to have been warned ${n} times but got ${found}.`,
}
}
},
})

let warn: SpyInstance
const asserted = new Map<string, string | RegExp>()

beforeEach(() => {
asserted.clear()
warn = vi.spyOn(console, 'warn')
warn.mockImplementation(() => {})
})

afterEach(() => {
const assertedArray = Array.from(asserted)
const nonAssertedWarnings = warn.mock.calls
.map((args) => args[0])
.filter((received) => {
return !assertedArray.some(([key, assertedMsg]) => {
return typeof assertedMsg === 'string'
? received.indexOf(assertedMsg) > -1
: assertedMsg.test(received)
})
})
warn.mockRestore()
if (nonAssertedWarnings.length) {
nonAssertedWarnings.forEach((warning) => {
console.warn(warning)
})
throw new Error(`test case threw unexpected warnings.`)
}
})
}

declare global {
namespace Vi {
interface JestAssertion<T = any> {
toHaveBeenWarned(): void
toHaveBeenWarnedLast(): void
toHaveBeenWarnedTimes(n: number): void
}
}
}

0 comments on commit 098c16c

Please sign in to comment.