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

feat(core): Support for react hook #653

Closed
illuminist opened this issue Mar 18, 2019 · 10 comments
Closed

feat(core): Support for react hook #653

illuminist opened this issue Mar 18, 2019 · 10 comments

Comments

@illuminist
Copy link
Contributor

illuminist commented Mar 18, 2019

PR #684

Do you want to request a feature or report a bug?

Feature

What is the current behavior?

There isn't any hook support for react@16.8

What is the expected behavior?

Provide useFirebaseConnect, useFirestoreConnect, etc.

The usage might not be much different from the current implementation. Except, react-redux-firebase and redux-firestore still depend on connect HOC from react-redux and there isn't any official support for useRedux hook. Only closest thing is https://github.com/facebookincubator/redux-react-hook which I believe not production ready yet; which is the main reason I haven't migrate to full hook.

Another reason is recompose is currently being deprecated, which makes HOC kind of deprecated, too. The package author also encourage us to use react hook instead.

So overall, hook has potential to simplify intenal code via useEffect hook as I have done in my unpublish project. However, this feature might not be happen before any official redux hook being publish. But shouldn't we discuss what API we need and how we will use those features?

For my case, I would like to create a custom hook with firebase hook inside to provide necessary data to dumb component. Eg:

const useShop = (shopId) => {
  useFirestoreConnect([{
    collection: 'shops',
    doc: shopId,
  ]})
  const mapState = useCallback(state => state.firestore.data.shops[shopId], [shopId]) 
  const shop = useMappedState(mapState) // usage according to redux-react-hook, equivalent to connect HOC

  return shop
}

const ShopDisplay = ({ shopId }) => {
  const shop = useShop(shopId)
  return (
    <div>
      <h1>{shop.name}</h1>
    </div>
  )
}
@prescottprue
Copy link
Owner

Love the idea, thanks for posting! For now you should be able to create your own custom hooks that call react-redux-firebase internally (since there is nothing preventing usage with hooks).

@peteruithoven
Copy link

Shouldn't this issue be closed now that the PR is merged?

@illuminist
Copy link
Contributor Author

@peteruithoven I still need some more input and developer feedback so I can implement more features into a hook so I think it's might unnecessary to close this issue yet.

@peteruithoven
Copy link

Ah, in that case, let me copy one idea from #703 (comment). My suggestion was to have useFirestoreConnect return selectors to access Redux state. Example:

import React from 'react';
import { useSelector } from 'react-redux'
import { useFirestoreConnect, isLoaded, isEmpty } from 'react-redux-firebase';

export default function List() {
  const { getOrdered, getError } = useFirestoreConnect({
    collection: 'todos',
    where: ['visibility', '==', visibility],
  });
  const todos = useSelector(state => getOrdered(state))
  const error = useSelector(state => getError(state))
  if (!isLoaded(data)) return 'Loading...';
  if (isEmpty(data)) return null;
  if (error) return error.message;
  return (
    <ul>
      {todos.map(todo => (
        <li key={todo.id}>{todo.name}</li>
      ))}
    </ul>
  );
}

@illuminist
Copy link
Contributor Author

Take a note that query as an object need to be memoized in order to use the hook correctly

const query = useMemo(() => ({
  collection: 'todos',
  where: ['visibility', '==', visibility],
}), [visibility])
const { getOrdered, getError } = useFirestoreConnect(query);

@Shreky
Copy link

Shreky commented Jul 5, 2019

Hi,
First, thank you for the efforts!
I just tried to use the useFirebase hook on a typescript based project and it didn't work.
apperantly, the recent version (v3, alpha 12) doesn't have an updated typescript definitions.
Do you plan to add it sometime?

Right now, my workaround is to import react-redux-firebase/src/useFirebase directly.

Thanks.

prescottprue pushed a commit that referenced this issue Jul 13, 2019
* feat(types): add types for hooks including useFirestore, useFirebase, useFirestoreConnect, useFirebaseConnect - #653
* feat(types): improve types for withFirestore and withFirebase
* feat(types): added ExtendedFirebaseInstance and ExtendedFirestoreInstance types
* feat(examples): update typescript example to use hooks
@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

Types added in v3.0.0-alpha.13 release. @Shreky the types for useFirebase are in there so let me know if that works well for you

@Shreky
Copy link

Shreky commented Aug 6, 2019

Hey @prescottprue,
Thank you for the update.
Now useFirebase is available, but appears like it is of type ExtendedFirebaseInstance therefore, I am not able to use auth actions like: logout(). I think the definition should include Auth:
export function useFirebase(): ExtendedFirebaseInstance & Auth & Storage.

@prescottprue
Copy link
Owner

@Shreky Made that change and confirmed it fixed what you were mentioning, I'll try to release soon.

It would probably be nice to combine all of the different action set types into the actual ExtendedFirebaseInstance type itself, but this should work for now

@prescottprue
Copy link
Owner

More updates have been made to hooks recently by @illuminist and myself. Make sure to update from next for the newest updates! Going to close this since they are now supported.

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

No branches or pull requests

4 participants