data-provider / core

Async data provider agnostic about data origins
https://data-provider.javierbrea.com
Apache License 2.0
56 stars 1 forks source link

Add `loaded` property to state #81

Closed javierbrea closed 4 years ago

javierbrea commented 4 years ago

The problem

Currently data-provider has three state properties: data, loading and error. In some scenarios you need to know if the data has been loaded any time, not only if its being loaded right now. This is because you want to show the loading indicator the first time the data is being loaded, but not next times the data is being loaded again because the cache was invalidated, as it may result in undesirables screen "flickerings" in case the component is "alive" while the cache is cleaned.

This problem is described in the data-provider react tutorial, and it is proposed to be solved using a double condition before rendering the loading indicator:

Solution proposed in the data-provider documentation:

import React from "react";
import { useData, useLoading } from "@data-provider/react";

import { todosFiltered, updateTodo } from "../data/todos";
import TodoList from "../components/TodoList";

const FilteredTodoList = ({ showCompleted }) => {
  const todosProvider = todosFiltered.query({ completed: showCompleted });
  const todos = useData(todosProvider);
  const loading = useLoading(todosProvider);

  if (loading && !todos) {
    return <div>Loading...</div>;
  }
  return (
    <TodoList todos={todos} onTodoClick={updateTodo} />
  );
};

export default FilteredTodoList;

Note the expression (loading && !todos) that we are using to handle the loading state. Data Provider informs to us when the todos are being loadded, but we don't want to show the "loading..." indicator every time data is loading. It will be shown only the first time the data is being loaded, while todos collection is empty. Rest of times, the current todos state will be shown until the todos are fetched again, and then replaced. In this way we avoid unwanted flickerings in the UI.

Problem of this solution

Apart that this solution is too much "verbose", as this is a very common scenario, and you'll need to repeat the double condition in a lot of components, it will only work when the data is not empty. If the data returned by the selector or provider is empty, it will result in rendering the loading indicator every time the cache is cleaned, so it will result in undesired flickerings in the UI.

New Proposed solution

To add a new property loaded to the state, which would be set as true the first time the data is loaded, and never modified again (until the "resetState" method is called)

This property should be exposed also by ui bindings addons. For example, in the case of @data-provider/react, a new hook useLoaded could be exposed. A fourth property loaded could also be returned by the current useDataProvider hook.

javierbrea commented 4 years ago

@Palen, the other day we talked about some points related to this topic. I would really appreciate some feedback before implementing this.