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

Uncaught Error: Supplied prop/s "firebase", "dispatch" are reserved for internal firebaseConnect() usage. #700

Closed
khmaies5 opened this issue May 14, 2019 · 12 comments

Comments

@khmaies5
Copy link

Uncaught Error: Supplied prop/s "firebase", "dispatch" are reserved for internal firebaseConnect() usage.

this happens when i use react-redux-firebase v3 inside a component and then call that component inside other component where i also use react-redux-firebase

store.js

import { createBrowserHistory } from 'history'
import { createStore, compose, applyMiddleware } from 'redux';
import { routerMiddleware } from 'connected-react-router';
import { reactReduxFirebase, firebaseReducer } from 'react-redux-firebase'
import createRootReducer  from './core/reducers/index';

export const history = createBrowserHistory();

export default function configureStore (initialState) {
  const createStoreWithMiddleware = compose(
    typeof window === 'object' && typeof window.devToolsExtension !== 'undefined' ? () => window.__REDUX_DEVTOOLS_EXTENSION__ : f => f,
    applyMiddleware(
      routerMiddleware(history) // for dispatching history actions
    )
  )(createStore)
  const store = createStoreWithMiddleware(createRootReducer(history))

  if (module.hot) {
    // Enable Webpack hot module replacement for reducers
    module.hot.accept('./core/reducers/index', () => {
      const nextRootReducer = require('./core/reducers/index')
      store.replaceReducer(nextRootReducer)
    })
  }

  return store
}

reducers

import { combineReducers } from 'redux';
import { firebaseStateReducer as firebase } from 'react-redux-firebase'
import { connectRouter } from 'connected-react-router';
import mainReducer from './mainReducer';

export default (history) => combineReducers({
  
  firebase,
  router: connectRouter(history),
  mainReducer
  
});

Originally posted by @khmaies5 in #638 (comment)

@illuminist
Copy link
Contributor

Duplicate #675

@prescottprue
Copy link
Owner

Yeah, this is definitely an issue, thanks for pointing out the duplicate @illuminist

@iamthefox
Copy link

This issue is caused by #613

Idea was if your firebaseConnect() was added after connect() it was overriding firebase data that was connected via firebaseReducer. I don't think this solution is working and I feel like that PR could be reverted as it does not really solve that issue.

@khmaies5
Copy link
Author

@iamthefox yes it does not fix this problem, i was willing to investigate it more but i didn't find the time for that

@iamthefox
Copy link

hey @prescottprue, is there any particular reason why props order changed, and "firebase" is the first one in "next" branch (https://github.com/prescottprue/react-redux-firebase/blob/next/src/firebaseConnect.js#L120) and last one in master (https://github.com/prescottprue/react-redux-firebase/blob/master/src/firebaseConnect.js#L97)

I moved it back to where it was in 2.x branch and it fixed the original issue I raised in #613

@khmaies5
Copy link
Author

@iamthefox
to understand you well
you chnaged this:

return (
      <ReactReduxFirebaseContext.Consumer>
        {_internalFirebase => (
          <HoistedComp
            firebase={_internalFirebase}
            dispatch={_internalFirebase.dispatch}
            {...props}
          />
        )}
      </ReactReduxFirebaseContext.Consumer>
    )

to this ?

return (
      <ReactReduxFirebaseContext.Consumer>
        {_internalFirebase => (
          <HoistedComp
            dispatch={_internalFirebase.dispatch}
            {...props}
            firebase={_internalFirebase}
          />
        )}
      </ReactReduxFirebaseContext.Consumer>
    )

@iamthefox
Copy link

https://github.com/iamthefox/react-redux-firebase/blob/next-patched/src/firebaseConnect.js#L103

prescottprue pushed a commit that referenced this issue Jul 13, 2019
* fix(firestoreConnect): add error for passing of reserved props "firebase" and "firestore" when using firestoreConnect
@prescottprue prescottprue mentioned this issue Jul 13, 2019
3 tasks
prescottprue added a commit that referenced this issue Jul 13, 2019
* feat(types): add types for hooks including `useFirestore`, `useFirebase`, `useFirestoreConnect`, `useFirebaseConnect` - #653
* fix(types): make scopes optional in `credentials` - #671
* fix(docs): remove v2 auth ready docs from v3 docs - #676
* fix(providers): prevent unwanted re-initialization when parent component updates - #725 - @illuminist
* fix(profile): pass `updateProfile` options to action - #701 - @cruzdanilo 
* fix(firebaseConnect): remove `dispatch` from reserved props list - #675, #700
* feat(types): improve types for `withFirestore` and `withFirebase`
* feat(types): added `ExtendedFirebaseInstance` and `ExtendedFirestoreInstance` types
* feat(examples): update typescript example to use hooks
* fix(firestoreConnect): add error for passing of reserved props `firebase` and `firestore` when using `firestoreConnect`
* feat(types): add typing for firebase queries used in `firebaseConnect` and `useFirebaseConnect`
* fix(types): fix spelling of initialize in types
* feat(docs): add api docs pages for context providers and new hooks api 
* feat(docs): automatically find files for doc generation instead of having a constant that needs updating
* feat(core): add uglify and lodash plugins to webpack build to shrink bundle build size
@prescottprue
Copy link
Owner

Proposed solution implemented in v3.0.0-alpha.13 release. Reach out if it isn't working as expected and we can reopen. Thanks to all for helping test out the next version.

@iamthefox
Copy link

iamthefox commented Jul 16, 2019

hey @prescottprue You moved firebase property so it is no longer overridden by outer component's firebase prop. (Ref: 47e41da#diff-10defd74a58e7f8397c9c53cf8d5ee99R114)

So there is no longer need to check for clashing props at all. It is safe to remove them completely.

They are no longer required for firebaseConnect or firestoreConnect.

EDIT: this issue was only partially resolved. To fully resolve it you want to remove check that me and Ash added originally.

@prescottprue
Copy link
Owner

Gotcha, makes total sense. I modified it to check for firebase in firebaseConnect and firebase and firestore in firestoreConnect, but yeah feels unnecessary.

@designifyer
Copy link

Hey guys,
I'm still getting this error when I try to use both firebaseConnect() and firestoreConnect(). If I use only one per component it works fine. I'm using "react-redux-firebase": "^3.0.0-alpha.13"

@prescottprue
Copy link
Owner

@designifyer the plan moving forward is for use to remove prop checking all together, so that should fix this issue. Thanks for reporting

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants