RFC: Cells

[samwightt]

Background

Currently the useQuery hook is used to load data from the server into components. useQuery works great, but rendering different UI on the basis of a query's state can be a bit of a pain / involve a lot of boilerplate.

For an example of this, let's use a component that renders out a list of posts from the server. We want to display different components for the loading, failure, empty, and success cases. We want these states to only be applied to this current component: we're okay with having multiple spinners or error messages in the app. This might look something like this:

import listPostsQuery from 'app/posts/queries/listPosts';
import PostComponent from './PostComponent';
import Spinner from './Spinner';
import ErrorDIsplay from './ErrorDisplay'

function ListPostsUI(_props) {
  const [posts] = useQuery(listPostsQuery);

  if (posts.length === 0)
    return <div>
      <h1>We couldn't find any posts :(</h1>
      <p>Get started by clicking the "New Post" button in the upper right hand corner.</p>
    </div>

  return <div>
    {posts.map(x => <PostComponent post={x} />)}
  </div>
}

export function ListPosts(props) {
  return <Suspense fallback={<Spinner />}>
    <ErrorBoundary fallback={(error) <ErrorDisplay error={error} />}>
      <ListPostsUI {...props} />
    </ErrorBoundary>
  </Suspense>
}

We wrap the main component with a Suspense and ErrorBoundary, and then export that new component to get the loading states displayed.

This is a pattern I've found myself repeating a lot in my Blitz code. It results in a lot of boilerplate when I just want to write a hacky component that manages its own loading and error states.

Cells

To help mitigate this boilerplate, I'd like to propose a Cells API. This would be implemented in mostly the same way as the RedwoodJS Cells API. A cell is a module that can have a few different exports:

• query: The query to call.
• Loading A component that should render when the query is loading.
• Empty: A component that should render when the query is null or an empty array.
• Failure: A component that should render if there's an error.
• Success: A component that should render if the data has loaded.

Webpack would combine these exports into a single component, writing the boilerplate code above automatically for you. Any file ending in .cell.tsx would be assumed to be a cell by Webpack and would be compiled as such.

Example

Here's an example of what the above would look like rewritten as a cell:

import listPostsQuery from 'app/posts/queries/listPosts';
import PostComponent from './PostComponent';
import Spinner from './Spinner';
import ErrorDIsplay from './ErrorDisplay'

export const query = listPostsQuery();

export const Loading = Spinner;
export const Failure = ErrorDisplay;
export const Empty = () => (
  <div>
    <h1>We couldn't find any posts :(</h1>
     <p>Get started by clicking the "New Post" button in the upper right hand corner.</p>
  </div>
)

export const Success = ({ data: posts }) => {
    return <div>
    {posts.map(x => <PostComponent post={x} />)}
  </div>
}

The different states are separated and the boilerplate code is gone. It's easier to read, and the loading state stays closer to the component's definition, instead of having to hoist it further up the tree.

What are y'all's thoughts on something like this?

[Skn0tt]

Is this something that we could do in Userland? Maybe with an API like this:

export default Cell({
  query: listPostsQuery,
  loading: Spinner,
  failure: ErrorDisplay,
  PostList({ data: posts }) { ... }
})

Cell() would then return a standard Blitz Page.

[codytooker]

This API looks really good. However I don't think it should return a BlitzPage because a single page could have multiple Cells in it. Seems like it should just return a React.ReactNode or something similar

[AndriusBartulis]

Another question to consider is should this be encouraged by Blitz or not? Do you think this Cell approach is actually prefered and nicer? Should it be recommended in the docs? Or... do we need a new section in Docs along the lines of "Common Patterns" where we can list various useful solutions to common problems that might not actually ship with the framework by default?

I think a key to Blitz succeeding is making it opinionated enough to make it easy for users to learn the framework and know how they should solve common scenarios like this without having to figure it out themselves.

[Skn0tt]

it should just return a React.ReactNode

agreed!

[Skn0tt]

@AndriusBartulis you have a very valid point, wanted to bring that up too. I spun it out to its own thread: #2583 (comment)

[codytooker]

@Skn0tt using your example above as a starting place I made a few adjustments. Wanting to be able to use this as single use components and not an entire page also wants me to be allowed to pass additional props to the component.

type DataComponent<Q extends QueryFn> = React.ComponentType<{ data: PromiseReturnType<Q> }>

interface InnerCellProps<Q extends QueryFn> {
  query: Q
  Component: DataComponent<Q>
}

const InnerCell = <Q extends QueryFn>({ query, Component, props }: InnerCellProps<Q>) => {
  const [data] = useQuery(query, undefined)
  return React.createElement(Component, { data, ...props })
}

const Cell = <Q extends QueryFn>(
  query: Q,
  Component: DataComponent<Q>,
  {
    failure,
    loading,
  }: {
    failure: React.ReactElement | null
    loading: NonNullable<React.ReactNode>
  }
): React.ComponentType => {
  return (props) => (
    <ErrorBoundary fallback={failure}>
      <React.Suspense fallback={loading}>
        <InnerCell query={query} Component={Component} props={props} />
      </React.Suspense>
    </ErrorBoundary>
  )
}

export default Cell

However my Typescript skills aren't all that great yet so I havn't been able to figure out how to type these props correctly.

[Skn0tt]

Spinning out @AndriusBartulis point to declutter:

Another question to consider is should this be encouraged by Blitz or not? Do you think this Cell approach is actually prefered and nicer? Should it be recommended in the docs? Or... do we need a new section in Docs along the lines of "Common Patterns" where we can list various useful solutions to common problems that might not actually ship with the framework by default?

I think a key to Blitz succeeding is making it opinionated enough to make it easy for users to learn the framework and know how they should solve common scenarios like this without having to figure it out themselves.

[Skn0tt]

I think that the Cell approach makes it easy to keep everything data-fetching-related in a single place, and it's well-suited for something like a dashboard where cells can resolve one-by-one. It also forces you to remember that there is indeed a loading and an error state, something that can easily be forgotten in development (where loading is fast and errors are typically ignored / expected).

Having said that, personally I haven't yet felt the need for a Cell component. Loading states are handled by my Layouts, and Failure states are fine with one central handler I think (e.g. in _app.tsx, which also deals with the Sentry integration).

I'd prefer to not ship a cell component directly in Blitz core, but test this out as user-land code first and then see if it's really helpful. If it is, we can either include it in docs (as you proposed), spin it out to a community-maintained library, or include it in core. If it is not, we have one thing less to maintain ^^

Curious about your thoughts on this, and the whole RFC @flybayer!

[AndriusBartulis]

@Skn0tt good point about handling errors and loading states at a higher level. So far, it's quite rare for me to handle loading and errors per component. I could end up with ten spinners on my dashboard. Of course there are valid use cases for that but that makes cells a little less useful in every situation and also it raises further questions, such as how would cells work with Suspense, etc.

Keeping it user-land for now makes a lot of sense I think.

[flybayer]

I'm a bit skeptical of the cell concept being better than our current approach of higher level error and suspense boundaries. And I don't really ever see myself using it.

But I'm just one of many, and if people want it, then we should support it. Starting a userland library is a good idea.

Also the idea of a "common patterns" doc section is good, and we could link to the userland library.

[samwightt]

Definitely agree that I don't think that this should be the default way of doing things. I think that the Suspense and ErrorBoundary way of doing things is a good default, but it'd be nice to have another option.

But yeah, doing this as a userland library would be super cool and probably the better way to implement things! The nice part about Blitz is that's super easy to do 😁

[flybayer]

Pinging @peterp — you said you had thoughts on this?

[peterp]

Sorry - Crazy times! Chiming in below.

[peterp]

@samwightt's implementation is exactly how we think of it in RedwoodJS; with the exception of a few things:

We have guard functions that sandwich query named beforeQuery and afterQuery. We use beforeQuery to modify the data or to provide additional request settings (headers, caching, etc), and afterQuery to modify the data before it's passed to Success. In my experience they're rarely used.

We also include a mechanism for "refetching:" If your client library invalidates the cache and refetches the data we pass an additional prop into success named updating that can be used to illustrate some sort of network activity.

---

On a personal note I enjoy the way that Cells let me think about my components in insolation: The shape of the "loader", the failure state, what they're fetching, etc.

[samwightt]

On a personal note I enjoy the way that Cells let me think about my components in insolation: The shape of the "loader", the failure state, what they're fetching, etc.

Yep exactly this! I have ADHD, so I like to keep my JS code simple and easy to reason about because it's harder for me to store a lot of stuff in my working memory. Having error and loading states located in one place just makes it easier for me to find things.

[MrLeebo]

I'm worried that making data fetching easy to compartmentalize into Cell components is going to lead to network waterfall issues. Maybe you have a Cell that is a parent of a Cell that is a parent of another Cell. Each child can't fetch data until the previous Cell's data has arrived, creating the network waterfall effect. It exaggerates the impact of the most time-consuming aspect of fetching data, the time it takes to send stuff across the wire.

This seems like one of those design issues that you can sweep under the rug at first because the queries are "fast enough," until the day they aren't. And on that day you have to undo all of that component isolation that a pattern like this creates.

Scoping Cells as BlitzPages resolves the waterfall issue, but at that cost of having something that seems generally worse than server-side rendering (except by having a better error handling story out of the box), but page-like Cells don't have a way to receive query parameters unless they're in the query string.

[samwightt]

I don't really see how that relates to this, as this can still very easily happen with nested useQuery uses. It seems like network waterfall issues are just something you as an application developer have to avoid.