react-redux-firebase icon indicating copy to clipboard operation
react-redux-firebase copied to clipboard

feat(query): easier access to firestoreConnect errors in components

Open peteruithoven opened this issue 5 years ago • 9 comments

Feature request: Easier access to errors from firestoreConnect (reading data) in components.

What is the current behavior? When a error arises when requesting data the error information is stored in the Redux state at firestore.errors.byQuery.[query id]. A query id could simply be the name of a collection like todos, but when specifying a limit it becomes todos?limit=6 making it hard to read out and forward to a component.

What is the expected behavior? I'd love an easier way to pass that those errors as a prop to a component.

I figured out a work around using storeAs:

import React from 'react';
import { connect } from 'react-redux';
import { firestoreConnect, isLoaded, isEmpty } from 'react-redux-firebase';
import { compose } from 'redux';

function List({ todos, error }) {
  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>
  );
}

const QUERY_ID = 'todos';
export default compose(
  firestoreConnect([
    {
      collection: 'todos',
      storeAs: QUERY_ID,
    },
  ]),
  connect(state => ({
    [QUERY_ID]: state.firestore.ordered[QUERY_ID],
    error: state.firestore.errors.byQuery[QUERY_ID],
  }))
)(List);

I'm using:

  • firebase: 5.7.0
  • react-redux-firebase: 2.2.6
  • redux-firestore: 0.8.0

peteruithoven avatar May 17 '19 20:05 peteruithoven

redux-firestore has internal utility function called getQueryName which is being used for determining and creating query name from query object input into firestoreConnect.

It hasn't been officially exported, so to use this you can import it by

import { getQueryName } from 'redux-firestore/es/utils/query'

Example usage:

const makeQuery(visibility) => ({
  collection: 'todos',
  where: ['visibility', '==', visibility],
})
export default compose(
  firestoreConnect(({visibility}) => [makeQuery(visibility)]),
  connect((state, {visibility}) => ({
    todos: state.firestore.ordered.todos,
    error: state.firestore.errors.byQuery[getQueryName(makeQuery(visibility))],
  }))
)(List);

I think it's a good idea to export getQueryName or some way to return a query name from the connector itself.

illuminist avatar May 20 '19 07:05 illuminist

Thanks for the reply! Making getQueryName seems like a great improvement.

I see you've created the new React hooks, like useFirestoreConnect(), maybe those can make things easier. We probably don't want to retrieve the error directly from useFirestoreConnect() since, like the data it should probably always be connected to that data in redux. But maybe it could return the query name it's connected to? One downside is that it might be confused with the path to use to retrieve data, which can be different (example: data is retrieved from state.firestore.ordered.todos while the error is at state.firestore.errors.byQuery.todos?visibility=true).

That would result in something like:

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

export default function List() {
  const { queryName } = useFirestoreConnect({
    collection: 'todos',
    where: ['visibility', '==', visibility],
  });
  const todos = useSelector(state => state.firestore.ordered.todos)
  const error = useSelector(state => state.firestore.errors.byQuery[queryName])
  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>
  );
}

Another approach might be to (also) return redux paths from useFirestoreConnect, but that requires functions. I don't have enough experience with hooks to know if this is a good idea.

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>
  );
}

For comparison, using getQueryName util, which only seems slightly harder:

import { useSelector } from 'react-redux'
import { useFirestoreConnect, isLoaded, isEmpty } from 'react-redux-firebase';
import { getQueryName } from 'redux-firestore/es/utils/query'

export default function List({ visibility }) {
  const query = {
    collection: 'todos',
    where: ['visibility', '==', visibility],
  };
  useFirestoreConnect(query);
  const todos = useSelector(state => state.firestore.ordered.todos)
  const error = useSelector(state => state.firestore.errors.byQuery[getQueryName(query)])
  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>
  );
}

peteruithoven avatar May 20 '19 09:05 peteruithoven

Thank you! I like the second idea where a hook returns state selectors for its own query. That should make life much easier for everyone.

I've been thinking about making pagination easier which I planned to take the same approach, which should include { readNext, readPrevious, readAppend }, but I still need to research and refactor redux-firestore internal to be able to do that first.

illuminist avatar May 20 '19 09:05 illuminist

Love all of this discussion and input! We may want to expose getQueryName at the top level

prescottprue avatar May 31 '19 03:05 prescottprue

I'd like to capture any FirebaseError: Missing or insufficient permissions. when using useFirestoreConnect. Is this currently possible? My use case is "trying to access documents you shouldn't". I'd like to capture this "403 - Forbidden" scenario somehow and redirect the user.

mariotacke avatar Feb 01 '20 04:02 mariotacke

@peteruithoven I really like the idea of hooks returning functions to help load from state - this will be particularly useful for v1 of redux-firestore since it stores things in state under the query path.

@mariotacke did you check to see if that is written to state at all. The above discussion is about loading those errors out of state in the easiest way

prescottprue avatar Feb 03 '20 20:02 prescottprue

@prescottprue, yeah I've noticed there is an error object on the state that records these things. I used that one. Thanks for your help!

mariotacke avatar Feb 04 '20 02:02 mariotacke

@prescottprue More input from me, it'd be better if we have a helper to create selector function instead.

For example

const { errorSelector, dataSelector, orderedSelector } = makeFirestoreSelector(query)

So the accessing to state isn't exclusive to hooks and should be able to be used in firestoreConnect and redux connect HOC. And it even has potential to combine data and ordered state selector to denormalize data if we're going to change ordered state to store only id.

illuminist avatar Feb 04 '20 10:02 illuminist

@illuminist That is a really great point and I like the proposed API - thanks for sharing!

prescottprue avatar Feb 04 '20 20:02 prescottprue