Add mergeProps argument (#20)

* Add minor optimization by checking `exists()` before calling `val()`

* Add `mergeProps` argument to `connect()`

* Change argument order to ownProps, subscriptionProps and actionProps

* Fix "Unknown prop `firebaseApp` on <div> tag" warning

* Pass firebaseProps to mergeProps instead of actionProps and subscriptionProps

* Clarify "actions" term in README

* Revert adding exists() to snapshot.val() check

* Only merge actions from firebaseProps before merging

* Improve tests

Author: simenbrekken

README.md

@@ -38,7 +38,7 @@ export default connect((props, ref) => ({
 
 ## Usage
 
-### `connect([mapFirebaseToProps])`
+### `connect([mapFirebaseToProps], [mergeProps])`
 
 Connects a React component to a Firebase App reference.
 
@@ -48,10 +48,15 @@ It does not modify the component class passed to it. Instead, it *returns* a new
 
 * [`mapFirebaseToProps(props, ref, firebaseApp): subscriptions`] \(*Object or Function*): Its result, or the argument itself must be a plain object. Each value must either be a path to a location in your database, a query object or a function. If you omit it, the default implementation just passes `firebaseApp` as a prop to your component.
 
+
+* [`mergeProps(ownProps, firebaseProps): props`] \(*Function*): If specified, it is passed the parent `props` and current subscription state merged with the result of `mapFirebaseToProps()`. The plain object you return from it will be passed as props to the wrapped component. If you omit it, `Object.assign({}, ownProps, firebaseProps)` is used by default.
+
 #### Returns
 
 A React component class that passes subscriptions and actions as props to your component according to the specified options.
 
+> Note: "actions" are any function values returned by `mapFirebaseToProps()` which are typically used to modify data in Firebase.
+
 ##### Static Properties
 
 * `WrappedComponent` *(Component)*: The original component class passed to `connect()`.

src/connect.js

@@ -6,10 +6,9 @@ import 'firebase/database'
 import { firebaseAppShape } from './PropTypes'
 import { applyMethods, getDisplayName } from './utils'
 
-const mergeProps = (actionProps, subscriptionProps, ownProps) => ({
+const defaultMergeProps = (ownProps, firebaseProps) => ({
   ...ownProps,
-  ...actionProps,
-  ...subscriptionProps,
+  ...firebaseProps,
 })
 
 const mapSubscriptionsToQueries = subscriptions => (
@@ -21,7 +20,7 @@ const mapSubscriptionsToQueries = subscriptions => (
 
 const defaultMapFirebaseToProps = (props, ref, firebaseApp) => ({ firebaseApp })
 
-export default (mapFirebaseToProps = defaultMapFirebaseToProps) => {
+export default (mapFirebaseToProps = defaultMapFirebaseToProps, mergeProps = defaultMergeProps) => {
   const mapFirebase = (
     isFunction(mapFirebaseToProps) ? mapFirebaseToProps : () => mapFirebaseToProps
   )
@@ -132,7 +131,7 @@ export default (mapFirebaseToProps = defaultMapFirebaseToProps) => {
         const firebaseProps = mapFirebase(this.props, this.ref, this.firebaseApp)
         const actionProps = pickBy(firebaseProps, isFunction)
         const subscriptionProps = this.state.subscriptionsState
-        const props = mergeProps(actionProps, subscriptionProps, this.props)
+        const props = mergeProps(this.props, { ...actionProps, ...subscriptionProps })
 
         return createElement(WrappedComponent, props)
       }

src/tests/connect-test.js

@@ -9,14 +9,14 @@ import { findRenderedComponentWithType, renderIntoDocument } from 'react-addons-
 import connect from '../connect'
 import { createMockApp, createMockSnapshot } from './helpers'
 
-const renderStub = (mapFirebaseToProps, firebaseApp, props) => {
+const renderStub = ({ mapFirebaseToProps, mergeProps, firebaseApp }, props) => {
   class Passthrough extends Component { // eslint-disable-line react/prefer-stateless-function
     render() {
       return <div />
     }
   }
 
-  const WrappedComponent = connect(mapFirebaseToProps)(Passthrough)
+  const WrappedComponent = connect(mapFirebaseToProps, mergeProps)(Passthrough)
   const container = renderIntoDocument(<WrappedComponent {...props} firebaseApp={firebaseApp} />)
   const stub = findRenderedComponentWithType(container, Passthrough)
 
@@ -33,7 +33,7 @@ test('Should throw if no initialized Firebase app instance was found', assert =>
   // Default app instance
   assert.doesNotThrow(() => {
     const defaultApp = firebase.initializeApp({})
-    const WrappedComponent = connect()('div')
+    const WrappedComponent = connect()(() => <div />)
     const container = renderIntoDocument(<WrappedComponent />)
     const stub = findRenderedComponentWithType(container, WrappedComponent)
 
@@ -66,16 +66,59 @@ test('Should subscribe to a single path', assert => {
     },
     on: (event, callback) => {
       assert.equal(event, 'value')
-      callback(createMockSnapshot('foo value'))
+      callback(createMockSnapshot({ bar: 'bar' }))
     },
   }
 
   const mapFirebaseToProps = () => ({ foo: 'foo' })
   const firebaseApp = createMockApp(mockDatabase)
-  const { state, props } = renderStub(mapFirebaseToProps, firebaseApp)
+  const { state, props } = renderStub({ mapFirebaseToProps, firebaseApp })
 
-  assert.deepEqual(state, { foo: 'foo value' })
-  assert.equal(props.foo, 'foo value')
+  assert.deepEqual(state, { foo: { bar: 'bar' } })
+  assert.deepEqual(props.foo, { bar: 'bar' })
+  assert.end()
+})
+
+test('Should return null if a subscribed path does not exist', assert => {
+  const mockDatabase = {
+    ref: path => {
+      assert.equal(path, 'foo')
+
+      return mockDatabase
+    },
+    on: (event, callback) => {
+      assert.equal(event, 'value')
+      callback(createMockSnapshot(null))
+    },
+  }
+
+  const mapFirebaseToProps = () => ({ foo: 'foo' })
+  const firebaseApp = createMockApp(mockDatabase)
+  const { state, props } = renderStub({ mapFirebaseToProps, firebaseApp })
+
+  assert.deepEqual(state, { foo: null })
+  assert.equal(props.foo, null)
+  assert.end()
+})
+
+test('Should not pass unresolved subscriptions from result of mapFirebaseToProps', assert => {
+  const mockDatabase = {
+    ref: path => {
+      assert.equal(path, 'foo')
+
+      return mockDatabase
+    },
+    on: event => {
+      assert.equal(event, 'value')
+    },
+  }
+
+  const mapFirebaseToProps = () => ({ foo: 'foo' })
+  const firebaseApp = createMockApp(mockDatabase)
+  const first = renderStub({ mapFirebaseToProps, firebaseApp })
+
+  assert.equal(first.state, null)
+  assert.equal(first.props.foo, undefined)
   assert.end()
 })
 
@@ -112,7 +155,7 @@ test('Should subscribe to a query', assert => {
   })
 
   const firebaseApp = createMockApp(mockDatabase)
-  const { state, props } = renderStub(mapFirebaseToProps, firebaseApp)
+  const { state, props } = renderStub({ mapFirebaseToProps, firebaseApp })
 
   assert.deepEqual(state, { bar: 'bar value' })
   assert.equal(props.bar, 'bar value')
@@ -126,7 +169,7 @@ test('Should not subscribe to functions', assert => {
   })
 
   const firebaseApp = createMockApp()
-  const { state, props } = renderStub(mapFirebaseToProps, firebaseApp)
+  const { state, props } = renderStub({ mapFirebaseToProps, firebaseApp })
 
   assert.deepEqual(state, { foo: 'foo value' })
   assert.equal(props.foo, 'foo value')
@@ -152,7 +195,7 @@ test('Should unsubscribe when component unmounts', assert => {
 
   const mapFirebaseToProps = () => ({ baz: 'baz' })
   const firebaseApp = createMockApp(mockDatabase)
-  const { container } = renderStub(mapFirebaseToProps, firebaseApp)
+  const { container } = renderStub({ mapFirebaseToProps, firebaseApp })
 
   assert.notEqual(container.listeners.baz, undefined)
   unmountComponentAtNode(findDOMNode(container).parentNode)
@@ -171,31 +214,43 @@ test('Should pass props, ref and firebaseApp to mapFirebaseToProps', assert => {
   }
 
   const firebaseApp = createMockApp()
-  const { props } = renderStub(mapFirebaseToProps, firebaseApp, { foo: 'foo prop' })
+  const { props } = renderStub({ mapFirebaseToProps, firebaseApp }, { foo: 'foo prop' })
 
   assert.equal(props.foo, 'foo value')
   assert.end()
 })
 
 test('Should update subscriptions when props change', assert => {
   const mapFirebaseToProps = props => ({ foo: props.foo, bar: props.bar })
-
   const firebaseApp = createMockApp()
-  const initial = renderStub(mapFirebaseToProps, firebaseApp, { foo: 'foo' })
+  const stubOptions = { mapFirebaseToProps, firebaseApp }
+
+  const initial = renderStub(stubOptions, { foo: 'foo' })
   assert.equal(initial.props.foo, 'foo value')
   assert.equal(initial.props.bar, undefined)
 
-  const added = renderStub(mapFirebaseToProps, firebaseApp, { foo: 'foo', bar: 'bar' })
+  const added = renderStub(stubOptions, { foo: 'foo', bar: 'bar' })
   assert.equal(added.props.foo, 'foo value')
   assert.equal(added.props.bar, 'bar value')
 
-  const changed = renderStub(mapFirebaseToProps, firebaseApp, { foo: 'foo', bar: 'baz' })
+  const changed = renderStub(stubOptions, { foo: 'foo', bar: 'baz' })
   assert.equal(changed.props.foo, 'foo value')
   assert.equal(changed.props.bar, 'baz value')
 
-  const removed = renderStub(mapFirebaseToProps, firebaseApp, { bar: 'baz' })
+  const removed = renderStub(stubOptions, { bar: 'baz' })
   assert.equal(removed.props.foo, undefined)
   assert.equal(removed.props.bar, 'baz value')
 
   assert.end()
 })
+
+test('Should use custom mergeProps function if provided', assert => {
+  const mapFirebaseToProps = props => ({ foo: props.foo })
+  const mergeProps = () => ({ bar: 'bar merge props' })
+
+  const firebaseApp = createMockApp()
+  const { props } = renderStub({ mapFirebaseToProps, mergeProps, firebaseApp }, { foo: 'foo prop' })
+
+  assert.deepEqual(props, { bar: 'bar merge props' })
+  assert.end()
+})

src/tests/helpers.js

@@ -1,6 +1,6 @@
-export const createMockSnapshot = (val, ...otherProps) => ({
+export const createMockSnapshot = (value, ...otherProps) => ({
   ...otherProps,
-  val: () => val,
+  val: () => value,
 })
 
 const defaultDatabaseProps = {