zetkin / lyra

3 stars 3 forks source link

Refactor project fetching into a helper function shared with the API #96

Closed henrycatalinismith closed 5 months ago

henrycatalinismith commented 5 months ago

This is a follow-up to the Slack conversation about data fetching, where we discussed the trade-offs of handling data fetching directly in server components, handling it client-side via REST API calls from the browser, and various middle-ground options between those two extremes.

Currently we've reached a point where we'd like to keep having a REST API. Although it's true that Next.js has added a lot of features that reduce the immediate, urgent need for a REST API, such as server components and server actions, we feel we'd like to build one anyway because of the interoperability benefits they bring later, when integrating with other systems.

Where we arrived at was that it'd be nice to take advantage of Next's great server rendering capabilities, but to have the server components delegate the data fetching work to the REST API endpoints. This pull request takes a step in that direction by extracting the home page's data fetching code into a helper function and then updating the REST API's /projects endpoint to use that helper function.

This aligns the REST API endpoint's response schema with the actual needs of the app feature that uses it, and puts us within touching distance of the approach discussed in Slack.

UI API
Screenshot 2024-06-11 at 20 12 27 Screenshot 2024-06-11 at 20 12 12

I haven't gone 100% of the way on this. The home page's server component is still fetching its own data directly, bypassing the REST API. There are two reasons for this.

  1. It wasn't immediately clear to me what was the correct way to formulate a fetch() call from a Next.js application server to itself, and I didn't easily find examples to learn from when searching.
  2. I think it's actually worth pausing in this intermediate state here and asking ourselves if we'll be improving something in a way that matters by spending time replacing a call to await getProjects() here with a call to await fetch(), as compared to prioritising forward progress on app functionality that translators can use to build Zetkin itself more efficiently.