diff --git a/lib/index.js b/lib/index.js index 6cd42dce..9d8a3e8a 100644 --- a/lib/index.js +++ b/lib/index.js @@ -24,7 +24,7 @@ export type { } from './modules/auth/types'; export type { default as ConfirmationResult, -} from './modules/auth/ConfirmationResult'; +} from './modules/auth/phone/ConfirmationResult'; export type { default as User } from './modules/auth/User'; /* diff --git a/lib/modules/auth/index.js b/lib/modules/auth/index.js index 4288a7e0..4b2863e8 100644 --- a/lib/modules/auth/index.js +++ b/lib/modules/auth/index.js @@ -8,7 +8,8 @@ import { getAppEventName, SharedEventEmitter } from '../../utils/events'; import { getLogger } from '../../utils/log'; import { getNativeModule } from '../../utils/native'; import INTERNALS from '../../utils/internals'; -import ConfirmationResult from './ConfirmationResult'; +import ConfirmationResult from './phone/ConfirmationResult'; +import PhoneAuthListener from './phone/PhoneAuthListener'; // providers import EmailAuthProvider from './providers/EmailAuthProvider'; @@ -19,8 +20,6 @@ import OAuthProvider from './providers/OAuthProvider'; import TwitterAuthProvider from './providers/TwitterAuthProvider'; import FacebookAuthProvider from './providers/FacebookAuthProvider'; -import PhoneAuthListener from './PhoneAuthListener'; - import type { ActionCodeInfo, ActionCodeSettings, diff --git a/lib/modules/auth/ConfirmationResult.js b/lib/modules/auth/phone/ConfirmationResult.js similarity index 85% rename from lib/modules/auth/ConfirmationResult.js rename to lib/modules/auth/phone/ConfirmationResult.js index fd76e69a..25d701d1 100644 --- a/lib/modules/auth/ConfirmationResult.js +++ b/lib/modules/auth/phone/ConfirmationResult.js @@ -2,9 +2,9 @@ * @flow * ConfirmationResult representation wrapper */ -import { getNativeModule } from '../../utils/native'; -import type Auth from './'; -import type User from './User'; +import { getNativeModule } from '../../../utils/native'; +import type Auth from '../'; +import type User from '../User'; export default class ConfirmationResult { _auth: Auth; diff --git a/lib/modules/auth/PhoneAuthListener.js b/lib/modules/auth/phone/PhoneAuthListener.js similarity index 97% rename from lib/modules/auth/PhoneAuthListener.js rename to lib/modules/auth/phone/PhoneAuthListener.js index d85bcace..68bc78fb 100644 --- a/lib/modules/auth/PhoneAuthListener.js +++ b/lib/modules/auth/phone/PhoneAuthListener.js @@ -1,6 +1,6 @@ // @flow -import INTERNALS from '../../utils/internals'; -import { SharedEventEmitter } from '../../utils/events'; +import INTERNALS from '../../../utils/internals'; +import { SharedEventEmitter } from '../../../utils/events'; import { generatePushID, isFunction, @@ -8,10 +8,10 @@ import { isIOS, isString, nativeToJSError, -} from '../../utils'; -import { getNativeModule } from '../../utils/native'; +} from '../../../utils'; +import { getNativeModule } from '../../../utils/native'; -import type Auth from './'; +import type Auth from '../'; type PhoneAuthSnapshot = { state: 'sent' | 'timeout' | 'verified' | 'error', diff --git a/lib/modules/firestore/DocumentReference.js b/lib/modules/firestore/DocumentReference.js index 91772833..ebce412b 100644 --- a/lib/modules/firestore/DocumentReference.js +++ b/lib/modules/firestore/DocumentReference.js @@ -4,12 +4,11 @@ */ import CollectionReference from './CollectionReference'; import DocumentSnapshot from './DocumentSnapshot'; -import FieldPath from './FieldPath'; -import { mergeFieldPathData } from './utils'; +import { parseUpdateArgs } from './utils'; import { buildNativeMap } from './utils/serialize'; import { getAppEventName, SharedEventEmitter } from '../../utils/events'; import { getLogger } from '../../utils/log'; -import { firestoreAutoId, isFunction, isObject, isString } from '../../utils'; +import { firestoreAutoId, isFunction, isObject } from '../../utils'; import { getNativeModule } from '../../utils/native'; import type Firestore from './'; @@ -50,9 +49,7 @@ export default class DocumentReference { get parent(): CollectionReference { const parentPath = this._documentPath.parent(); - if (!parentPath) { - throw new Error('Invalid document path'); - } + // $FlowExpectedError: parentPath can never be null return new CollectionReference(this._firestore, parentPath); } @@ -232,34 +229,7 @@ export default class DocumentReference { } update(...args: any[]): Promise { - let data = {}; - if (args.length === 1) { - if (!isObject(args[0])) { - throw new Error( - 'DocumentReference.update failed: If using a single argument, it must be an object.' - ); - } - // eslint-disable-next-line prefer-destructuring - data = args[0]; - } else if (args.length % 2 === 1) { - throw new Error( - 'DocumentReference.update failed: Must have either a single object argument, or equal numbers of key/value pairs.' - ); - } else { - for (let i = 0; i < args.length; i += 2) { - const key = args[i]; - const value = args[i + 1]; - if (isString(key)) { - data[key] = value; - } else if (key instanceof FieldPath) { - data = mergeFieldPathData(data, key._segments, value); - } else { - throw new Error( - `DocumentReference.update failed: Argument at index ${i} must be a string or FieldPath` - ); - } - } - } + const data = parseUpdateArgs(args, 'DocumentReference.update'); const nativeData = buildNativeMap(data); return getNativeModule(this._firestore).documentUpdate( this.path, diff --git a/lib/modules/firestore/Path.js b/lib/modules/firestore/Path.js index 5818b874..dd215cb1 100644 --- a/lib/modules/firestore/Path.js +++ b/lib/modules/firestore/Path.js @@ -14,10 +14,7 @@ export default class Path { } get id(): string | null { - if (this._parts.length > 0) { - return this._parts[this._parts.length - 1]; - } - return null; + return this._parts.length > 0 ? this._parts[this._parts.length - 1] : null; } get isDocument(): boolean { @@ -37,11 +34,9 @@ export default class Path { } parent(): Path | null { - if (this._parts.length === 0) { - return null; - } - - return new Path(this._parts.slice(0, this._parts.length - 1)); + return this._parts.length > 0 + ? new Path(this._parts.slice(0, this._parts.length - 1)) + : null; } /** @@ -50,10 +45,6 @@ export default class Path { */ static fromName(name: string): Path { const parts = name.split('/'); - - if (parts.length === 0) { - return new Path([]); - } - return new Path(parts); + return parts.length === 0 ? new Path([]) : new Path(parts); } } diff --git a/lib/modules/firestore/Transaction.js b/lib/modules/firestore/Transaction.js index 71cd1b5e..1fa0ce13 100644 --- a/lib/modules/firestore/Transaction.js +++ b/lib/modules/firestore/Transaction.js @@ -2,15 +2,13 @@ * @flow * Firestore Transaction representation wrapper */ -import { mergeFieldPathData } from './utils'; +import { parseUpdateArgs } from './utils'; import { buildNativeMap } from './utils/serialize'; import type Firestore from './'; import type { TransactionMeta } from './TransactionHandler'; import type DocumentReference from './DocumentReference'; import DocumentSnapshot from './DocumentSnapshot'; -import { isObject, isString } from '../../utils'; -import FieldPath from './FieldPath'; import { getNativeModule } from '../../utils/native'; type Command = { @@ -124,35 +122,7 @@ export default class Transaction { */ update(documentRef: DocumentReference, ...args: Array): Transaction { // todo validate doc ref - let data = {}; - if (args.length === 1) { - if (!isObject(args[0])) { - throw new Error( - 'Transaction.update failed: If using a single data argument, it must be an object.' - ); - } - - [data] = args; - } else if (args.length % 2 === 1) { - throw new Error( - 'Transaction.update failed: Must have either a single object data argument, or equal numbers of data key/value pairs.' - ); - } else { - for (let i = 0; i < args.length; i += 2) { - const key = args[i]; - const value = args[i + 1]; - if (isString(key)) { - data[key] = value; - } else if (key instanceof FieldPath) { - data = mergeFieldPathData(data, key._segments, value); - } else { - throw new Error( - `Transaction.update failed: Argument at index ${i} must be a string or FieldPath` - ); - } - } - } - + const data = parseUpdateArgs(args, 'Transaction.update'); this._commandBuffer.push({ type: 'update', path: documentRef.path, diff --git a/lib/modules/firestore/TransactionHandler.js b/lib/modules/firestore/TransactionHandler.js index 3b4f5761..1f424a80 100644 --- a/lib/modules/firestore/TransactionHandler.js +++ b/lib/modules/firestore/TransactionHandler.js @@ -3,7 +3,6 @@ * Firestore Transaction representation wrapper */ import { getAppEventName, SharedEventEmitter } from '../../utils/events'; -import { getLogger } from '../../utils/log'; import { getNativeModule } from '../../utils/native'; import Transaction from './Transaction'; import type Firestore from './'; @@ -103,9 +102,7 @@ export default class TransactionHandler { * @private */ _remove(id) { - // todo confirm pending arg no longer needed getNativeModule(this._firestore).transactionDispose(id); - // TODO may need delaying to next event loop delete this._pending[id]; } @@ -124,19 +121,17 @@ export default class TransactionHandler { * @private */ _handleTransactionEvent(event: TransactionEvent) { + // eslint-disable-next-line default-case switch (event.type) { case 'update': - return this._handleUpdate(event); + this._handleUpdate(event); + break; case 'error': - return this._handleError(event); + this._handleError(event); + break; case 'complete': - return this._handleComplete(event); - default: - getLogger(this._firestore).warn( - `Unknown transaction event type: '${event.type}'`, - event - ); - return undefined; + this._handleComplete(event); + break; } } diff --git a/lib/modules/firestore/WriteBatch.js b/lib/modules/firestore/WriteBatch.js index fe4f15a9..5f4fd163 100644 --- a/lib/modules/firestore/WriteBatch.js +++ b/lib/modules/firestore/WriteBatch.js @@ -2,10 +2,8 @@ * @flow * WriteBatch representation wrapper */ -import FieldPath from './FieldPath'; -import { mergeFieldPathData } from './utils'; +import { parseUpdateArgs } from './utils'; import { buildNativeMap } from './utils/serialize'; -import { isObject, isString } from '../../utils'; import { getNativeModule } from '../../utils/native'; import type DocumentReference from './DocumentReference'; @@ -66,38 +64,9 @@ export default class WriteBatch { update(docRef: DocumentReference, ...args: any[]): WriteBatch { // TODO: Validation // validate.isDocumentReference('docRef', docRef); - let data = {}; - if (args.length === 1) { - if (!isObject(args[0])) { - throw new Error( - 'WriteBatch.update failed: If using two arguments, the second must be an object.' - ); - } - // eslint-disable-next-line prefer-destructuring - data = args[0]; - } else if (args.length % 2 === 1) { - throw new Error( - 'WriteBatch.update failed: Must have a document reference, followed by either a single object argument, or equal numbers of key/value pairs.' - ); - } else { - for (let i = 0; i < args.length; i += 2) { - const key = args[i]; - const value = args[i + 1]; - if (isString(key)) { - data[key] = value; - } else if (key instanceof FieldPath) { - data = mergeFieldPathData(data, key._segments, value); - } else { - throw new Error( - `WriteBatch.update failed: Argument at index ${i} must be a string or FieldPath` - ); - } - } - } - - const nativeData = buildNativeMap(data); + const data = parseUpdateArgs(args, 'WriteBatch.update'); this._writes.push({ - data: nativeData, + data: buildNativeMap(data), path: docRef.path, type: 'UPDATE', }); diff --git a/lib/modules/firestore/utils/index.js b/lib/modules/firestore/utils/index.js index 8405183b..6aabeeaf 100644 --- a/lib/modules/firestore/utils/index.js +++ b/lib/modules/firestore/utils/index.js @@ -1,6 +1,8 @@ /** * @flow */ +import FieldPath from '../FieldPath'; +import { isObject, isString } from '../../../utils'; const buildFieldPathData = (segments: string[], value: any): Object => { if (segments.length === 1) { @@ -39,3 +41,34 @@ export const mergeFieldPathData = ( [segments[0]]: buildFieldPathData(segments.slice(1), value), }; }; + +export const parseUpdateArgs = (args: any[], methodName: string) => { + let data = {}; + if (args.length === 1) { + if (!isObject(args[0])) { + throw new Error( + `${methodName} failed: If using a single update argument, it must be an object.` + ); + } + [data] = args; + } else if (args.length % 2 === 1) { + throw new Error( + `${methodName} failed: The update arguments must be either a single object argument, or equal numbers of key/value pairs.` + ); + } else { + for (let i = 0; i < args.length; i += 2) { + const key = args[i]; + const value = args[i + 1]; + if (isString(key)) { + data[key] = value; + } else if (key instanceof FieldPath) { + data = mergeFieldPathData(data, key._segments, value); + } else { + throw new Error( + `${methodName} failed: Argument at index ${i} must be a string or FieldPath` + ); + } + } + } + return data; +}; diff --git a/tests/ios/Podfile.lock b/tests/ios/Podfile.lock index 0ff42ead..3af0c2e6 100644 --- a/tests/ios/Podfile.lock +++ b/tests/ios/Podfile.lock @@ -164,7 +164,7 @@ PODS: - React/Core - React/fishhook - React/RCTBlob - - RNFirebase (3.3.1): + - RNFirebase (4.0.0-alpha.1): - React - yoga (0.52.3.React) diff --git a/tests/src/firebase.js b/tests/src/firebase.js index 6b4800de..58580998 100644 --- a/tests/src/firebase.js +++ b/tests/src/firebase.js @@ -4,8 +4,11 @@ import firebase from 'firebase'; import RNfirebase from './../firebase'; import DatabaseContents from './tests/support/DatabaseContents'; -// RNfirebase.database.enableLogging(true); -// RNfirebase.firestore.enableLogging(true); +// Verify logging works +RNfirebase.database.enableLogging(true); +RNfirebase.database.enableLogging(false); +RNfirebase.firestore.enableLogging(true); +RNfirebase.firestore.enableLogging(false); // RNfirebase.utils().logLevel = 'debug'; // RNfirebase.utils().logLevel = 'info'; diff --git a/tests/src/tests/firestore/documentReferenceTests.js b/tests/src/tests/firestore/documentReferenceTests.js index bd14e3d8..417c6166 100644 --- a/tests/src/tests/firestore/documentReferenceTests.js +++ b/tests/src/tests/firestore/documentReferenceTests.js @@ -37,6 +37,17 @@ function documentReferenceTests({ describe, it, context, firebase }) { const collection = document.collection('pages'); collection.id.should.equal('pages'); }); + + it('should error if invalid collection path supplied', () => { + (() => { + firebase.native + .firestore() + .doc('documents/doc1') + .collection('pages/page1'); + }).should.throw( + 'Argument "collectionPath" must point to a collection.' + ); + }); }); context('delete()', () => { @@ -586,12 +597,12 @@ function documentReferenceTests({ describe, it, context, firebase }) { (() => { docRef.update('error'); }).should.throw( - 'DocumentReference.update failed: If using a single argument, it must be an object.' + 'DocumentReference.update failed: If using a single update argument, it must be an object.' ); (() => { docRef.update('error1', 'error2', 'error3'); }).should.throw( - 'DocumentReference.update failed: Must have either a single object argument, or equal numbers of key/value pairs.' + 'DocumentReference.update failed: The update arguments must be either a single object argument, or equal numbers of key/value pairs.' ); (() => { docRef.update(0, 'error'); diff --git a/tests/src/tests/firestore/firestoreTests.js b/tests/src/tests/firestore/firestoreTests.js index 94b0c91b..1445069a 100644 --- a/tests/src/tests/firestore/firestoreTests.js +++ b/tests/src/tests/firestore/firestoreTests.js @@ -111,12 +111,12 @@ function firestoreTests({ describe, it, context, fcontext, firebase }) { (() => { batch.update(ref, 'error'); }).should.throw( - 'WriteBatch.update failed: If using two arguments, the second must be an object.' + 'WriteBatch.update failed: If using a single update argument, it must be an object.' ); (() => { batch.update(ref, 'error1', 'error2', 'error3'); }).should.throw( - 'WriteBatch.update failed: Must have a document reference, followed by either a single object argument, or equal numbers of key/value pairs.' + 'WriteBatch.update failed: The update arguments must be either a single object argument, or equal numbers of key/value pairs.' ); (() => { batch.update(ref, 0, 'error');