Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue with reserved prop names inside firebaseConnect #613

Merged
merged 9 commits into from
Jan 16, 2019
69 changes: 47 additions & 22 deletions src/firebaseConnect.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ import { watchEvents, unWatchEvents } from './actions/query'
import { getEventsFromInput, createCallable, wrapDisplayName } from './utils'
import ReactReduxFirebaseContext from './ReactReduxFirebaseContext'

// Reserved props that should not be passed into a firebaseConnect wrapped
// component. Will throw an error if they are.
const RESERVED_PROPS = ['_firebaseRef', '_dispatch']

/**
* @name createFirebaseConnect
* @description Function that creates a Higher Order Component that
Expand Down Expand Up @@ -39,27 +43,27 @@ export const createFirebaseConnect = (storeKey = 'store') => (
prevData = null

componentDidMount() {
const { firebase, dispatch } = this.props
const { _firebaseRef, _dispatch } = this.props

// Allow function to be passed
const inputAsFunc = createCallable(dataOrFn)
this.prevData = inputAsFunc(this.props, this.props)

const { ref, helpers, storage, database, auth } = firebase
const { ref, helpers, storage, database, auth } = _firebaseRef
this.firebase = { ref, storage, database, auth, ...helpers }

this._firebaseEvents = getEventsFromInput(this.prevData)

watchEvents(firebase, dispatch, this._firebaseEvents)
watchEvents(_firebaseRef, _dispatch, this._firebaseEvents)
}

componentWillUnmount() {
const { firebase, dispatch } = this.props
unWatchEvents(firebase, dispatch, this._firebaseEvents)
const { _firebaseRef, _dispatch } = this.props
unWatchEvents(_firebaseRef, _dispatch, this._firebaseEvents)
}

componentWillReceiveProps(np) {
const { firebase, dispatch } = this.props
const { _firebaseRef, _dispatch } = this.props
const inputAsFunc = createCallable(dataOrFn)
const data = inputAsFunc(np, this.store)

Expand All @@ -71,15 +75,19 @@ export const createFirebaseConnect = (storeKey = 'store') => (
this.prevData = data
// UnWatch all current events
unWatchEvents(
firebase,
dispatch,
_firebaseRef,
_dispatch,
getEventsFromInput(itemsToUnsubscribe)
)
// Get watch events from new data
this._firebaseEvents = getEventsFromInput(data)

// Watch new events
watchEvents(firebase, dispatch, getEventsFromInput(itemsToSubscribe))
watchEvents(
_firebaseRef,
_dispatch,
getEventsFromInput(itemsToSubscribe)
)
}
}

Expand All @@ -89,23 +97,40 @@ export const createFirebaseConnect = (storeKey = 'store') => (
}

FirebaseConnectWrapped.propTypes = {
dispatch: PropTypes.func.isRequired,
firebase: PropTypes.object.isRequired
_dispatch: PropTypes.func.isRequired,
_firebaseRef: PropTypes.object.isRequired
}

const HoistedComp = hoistStatics(FirebaseConnectWrapped, WrappedComponent)

const FirebaseConnect = props => (
<ReactReduxFirebaseContext.Consumer>
{firebase => (
<HoistedComp
firebase={firebase}
dispatch={firebase.dispatch}
{...props}
/>
)}
</ReactReduxFirebaseContext.Consumer>
)
const FirebaseConnect = props => {
// Check that reserved props are not supplied to a FirebaseConnected
// component and if they are, throw an error so the developer can rectify
// this issue.
const clashes = Object.keys(props).filter(
k => !!RESERVED_PROPS.find(r => r === k)
)

if (clashes) {
theashguy marked this conversation as resolved.
Show resolved Hide resolved
throw new Error(
`Supplied prop/s "${clashes.join(
'", "'
)}" are reserved for internal firebaseConnect() useage.`
theashguy marked this conversation as resolved.
Show resolved Hide resolved
)
}

return (
<ReactReduxFirebaseContext.Consumer>
{_firebaseRef => (
<HoistedComp
_firebaseRef={_firebaseRef}
_dispatch={_firebaseRef.dispatch}
{...props}
/>
)}
</ReactReduxFirebaseContext.Consumer>
)
}

FirebaseConnect.displayName = wrapDisplayName(
WrappedComponent,
Expand Down
21 changes: 18 additions & 3 deletions test/unit/firebaseConnect.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const getFirebaseWatchers = store => {
return { ...store.firebase._.watchers }
}

const createContainer = () => {
const createContainer = additionalWrappedProps => {
const store = storeWithFirebase()
const WrappedContainer = firebaseConnect(props => {
const itemsToSubscribe =
Expand All @@ -33,12 +33,12 @@ const createContainer = () => {

const tree = TestUtils.renderIntoDocument(
<ProviderMock store={store}>
<WrappedContainer pass="through" />
<WrappedContainer pass="through" {...additionalWrappedProps} />
</ProviderMock>
)

return {
container: TestUtils.findRenderedComponentWithType(tree, WrappedContainer),
// container: TestUtils.findRenderedComponentWithType(tree, WrappedContainer),
parent: TestUtils.findRenderedComponentWithType(tree, ProviderMock),
store
}
Expand Down Expand Up @@ -106,6 +106,21 @@ describe('firebaseConnect', () => {
expect(values(getFirebaseWatchers(store))).to.eql([1])
})

it('throws an exception if passed a prop that clashes with a reserved param', () => {
let exceptions = []

try {
createContainer({
_firebaseRef: '__SECRET_INTERNALS',
_dispatch: '__SECRET_INTERNALS'
})
} catch (e) {
exceptions.push(e)
}

expect(exceptions.length).to.equal(1)
})

describe.skip('sets displayName static as ', () => {
/* eslint-disable no-template-curly-in-string */
describe('FirebaseConnect(${WrappedComponentName}) for', () => {
Expand Down