Add react query

[angelocordon]

What type of PR is this? (check all applicable)

• ♻️ Refactor
• ✨ Feature
• 🐛 Bug Fix
• 🎨 Enhancement
• 📝 Documentation Update
• 🔖 Release
• 🚩 Other

Context

This adds ReactQuery to replace our async fetching patterns.

Implementation Details - what was your thought process as you changed the code?

• Made an axios GET request to /resources
• Refactored to use useQuery library
• Wrote tests

React-Query

React Query gives us a better abstraction than using React's useEffect and useState hooks. The initial discussion here was that using state to asynchronously pass data to be rendered seemed a little hacky for the following reasons:

• The data only needs to be fetched once
• The data isn't really a state
• using React Query's abstractions seems to be more readable from the perspective of less mental gymnastics.

The old way:

On mount, set state to loading and data to null, then fetch data, then reset loading state and fill in data state and then render the data state.

The new way:

Component mounts with isLoading state to be true; renders a loading component. Data is fetched, and then data is rendered.
I find this to be significantly easier to read:

const { isLoading, data } = useQuery(['resource', resourceId], getResource);

if (isLoading) { return <p>Loading...</p> }

return (
   // render the elements when `isLoading` is no longer true and pass `data` objects down
)

Utility Functions

This PR also includes utility functions for fetching individual resources. @lpatmo and I thought it would be helpful to pull that functionality out of the component to establish utility patterns. This allows us to better isolate responsibilities and makes unit testing easier. My only concern here is that I am unsure of how the backend API will be handling errors? We are currently throwing errors (though not being handled in the UI), mainly because I think that's what Axios is supposed to be doing?

For additional resources, here are some great, short articles:
https://codeworkshop.dev/blog/2020-04-15-fetching-data-with-react-query-and-suspense/
https://www.robinwieruch.de/axios-jest

Related Tickets & Documents (Optional)

#103
#123

Added tests?

• 👍 yes
• 🙅 no, because they aren't needed
• 🙋 no, because I need help

Added to documentation (readme.md or contributing.md)?

• 📜 readme.md
• 📜 contributing.md
• 🙅 no documentation needed
• 🙋 I'd like someone to help write documentation, and will file a new issue for it

[lpatmo]

Thank you for figuring this out, @angelocordon! Left a few comments/questions, but overall looks good!

[lpatmo]

I think we can do const {isLoading, data, error} here and later something like:

if (error) {
    return <p>{error.detail}</p>
}

This is because the response from the API looks like this when there is an error:

[angelocordon]

@lpatmo - when the API doesn't resolve in DRF, does it return us an error object or does it throw an error? I'm curious how our getResource() function should handle error responses? Right now we're just returning a data object:

frontend/src/utils/queries.js

Lines 6 to 7 in 700010a

	const { data } = await axios.get(`${API_URL}/resources/${id}`);
	return data;

[lpatmo]

Sorry, I should have tried this out before I made the suggestion! DRF throws a 404 error.

We could add try/catch to that function in queries.js, but if we do that the error that is returned is considered "data" and not "error".

Wondering how you feel about doing this instead:

  const { isLoading, data, error } = useQuery(
    ['resource', resourceId],
    getResource
  );
  const classes = useStyles();

  if (error) {
    console.error(error);
    return <p>Sorry, there was an error!</p>;
  }
  if (isLoading) {
    return <p>Loading...</p>;
  }

[angelocordon]

Gotcha, this then tells me that getResource should be returning an error object from Axios then but I think that can be moved to another issue to handle errors a lot more appropriately.

[lpatmo]

Thanks for figuring out this syntax for getting an individual resource ID!

[angelocordon]

I'm not actually sure why we pass down the whole matchProps object to the ResourcePage when only need the guid here 🤔

[lpatmo]

Yeah, can totally do

            <Route
              path="/resources/:guid"
              render={matchProps => (
                <ResourcePage resourceId={matchProps.match.params.guid} />
              )}
            />

and

function ResourcePage({ resourceId }) {
...
ResourcePage.propTypes = {
  resourceId: PropTypes.string,
};

if you want :) I don't have a preference for either.

[lpatmo]

Interesting -- this _key is required by react query and is resources when we call it?

[angelocordon]

yeah, we don't use _key at all here but React Query uses it to id cache responses, I believe.

The unique key you provide is used internally for refetching, caching, deduping related queries.
https://react-query.tanstack.com/docs/guides/queries#query-basics

In our case, how useQuery works, it passes the key and a param to the callback (getResource) so in order to get the id, I had to accommodate for the first arg useQuery passes.

[lpatmo]

Got it; thanks for linking to the docs!

[bengineerdavis]

Looks good, thank you!