7

I'm wondering what the recommended best practice is for manipulating and exposing the new React Context.

The easiest way to manipulate context state seems to be to just attach a function to the context that either dispatches (usereducer) or setstate (useState) to change its internal value once called.

export const TodosProvider: React.FC<any> = ({ children }) => {
  const [state, dispatch] = useReducer(reducer, null, init);

  return (
    <Context.Provider
      value={{
        todos: state.todos,
        fetchTodos: async id => {
          const todos = await getTodos(id);
          console.log(id);
          dispatch({ type: "SET_TODOS", payload: todos });
        }
      }}
    >
      {children}
    </Context.Provider>
  );
};

export const Todos = id => {
  const { todos, fetchTodos } = useContext(Context);
  useEffect(() => {
    if (fetchTodos) fetchTodos(id);
  }, [fetchTodos]);
  return (
    <div>
      <pre>{JSON.stringify(todos)}</pre>
    </div>
  );
};

I was however told exposing and using the react context object directly is probably not a good idea, and was told to wrap it inside a hook instead.

export const TodosProvider: React.FC<any> = ({ children }) => {
  const [state, dispatch] = useReducer(reducer, null, init);

  return (
    <Context.Provider
      value={{
        dispatch,
        state
      }}
    >
      {children}
    </Context.Provider>
  );
};

const useTodos = () => {
  const { state, dispatch } = useContext(Context);
  const [actionCreators, setActionCreators] = useState(null);

  useEffect(() => {
    setActionCreators({
      fetchTodos: async id => {
        const todos = await getTodos(id);
        console.log(id);
        dispatch({ type: "SET_TODOS", payload: todos });
      }
    });
  }, []);

  return {
    ...state,
    ...actionCreators
  };
};

export const Todos = ({ id }) => {
  const { todos, fetchTodos } = useTodos();
  useEffect(() => {
    if (fetchTodos && id) fetchTodos(id);
  }, [fetchTodos]);

  return (
    <div>
      <pre>{JSON.stringify(todos)}</pre>
    </div>
  );
};

I have made running code examples for both variants here: https://codesandbox.io/s/mzxrjz0v78?fontsize=14

So now I'm a little confused as to which of the 2 ways is the right way to do it?

Shubham Khatri
  • 211,155
  • 45
  • 305
  • 318
Dac0d3r
  • 1,761
  • 3
  • 33
  • 57

2 Answers2

4

There is absolute no problem with using useContext directly in a component. It however forces the component which has to use the context value to know what context to use.

If you have multiple components in the App where you want to make use of TodoProvider context or you have multiple Contexts within your app , you simplify it a little with a custom hook

Also one more thing that you must consider when using context is that you shouldn't be creating a new object on each render otherwise all components that are using context will re-render even though nothing would have changed. To do that you can make use of useMemo hook

const Context = React.createContext<{ todos: any; fetchTodos: any }>(undefined);

export const TodosProvider: React.FC<any> = ({ children }) => {
  const [state, dispatch] = useReducer(reducer, null, init);
  const context = useMemo(() => {
    return {
      todos: state.todos,
      fetchTodos: async id => {
        const todos = await getTodos(id);
        console.log(id);
        dispatch({ type: "SET_TODOS", payload: todos });
      }
    };
  }, [state.todos, getTodos]);
  return <Context.Provider value={context}>{children}</Context.Provider>;
};

const getTodos = async id => {
  console.log(id);
  const response = await fetch(
    "https://jsonplaceholder.typicode.com/todos/" + id
  );
  return await response.json();
};
export const useTodos = () => {
  const todoContext = useContext(Context);
  return todoContext;
};
export const Todos = ({ id }) => {
  const { todos, fetchTodos } = useTodos();
  useEffect(() => {
    if (fetchTodos) fetchTodos(id);
  }, [id]);
  return (
    <div>
      <pre>{JSON.stringify(todos)}</pre>
    </div>
  );
};

Working demo

EDIT:

Since getTodos is just a function that cannot change, does it make sense to use that as update argument in useMemo?

It makes sense to pass getTodos to dependency array in useMemo if getTodos method is changing and is called within the functional component. Often you would memoize the method using useCallback so that its not created on every render but only if any of its dependency from enclosing scope changes to update the dependency within its lexical scope. Now in such a case you would need to pass it as a parameter to the dependency array.

However in your case, you can omit it.

Also how would you handle an initial effect. Say if you were to call `getTodos´ in useEffect hook when provider mounts? Could you memorize that call as well?

You would simply have an effect within Provider that is called on initial mount

export const TodosProvider: React.FC<any> = ({ children }) => {
  const [state, dispatch] = useReducer(reducer, null, init);
  const context = useMemo(() => {
    return {
      todos: state.todos,
      fetchTodos: async id => {
        const todos = await getTodos(id);
        console.log(id);
        dispatch({ type: "SET_TODOS", payload: todos });
      }
    };
  }, [state.todos]);
  useEffect(() => {
      getTodos();
  }, [])
  return <Context.Provider value={context}>{children}</Context.Provider>;
};
Shubham Khatri
  • 211,155
  • 45
  • 305
  • 318
  • Thanks. 2 questions. Since `getTodos` is just a function that cannot change, does it make sense to use that as update argument in `useMemo`? Also how would you handle an initial effect. Say if you were to call `getTodos´ in useEffect hook when provider mounts? Could you memorize that call as well? – Dac0d3r Apr 27 '19 at 09:26
  • 1
    Updated the answer for your questions – Shubham Khatri Apr 27 '19 at 10:34
0

I don't think there's an official answer, so let's try to use some common sense here. I find perfectly fine to use useContext directly, I don't know who told you not to, perhaps HE/SHE should have pointed for official docs. Why would the React team create that hook if it wasn't supposed to be used? :)

I can understand, however, trying to avoid creating a huge object as the value in the Context.Provider, one that mixes state with functions that manipulate it, possibly with async effects like your example.

However, in your refactor, you introduced a very weird and absolutely unnecessary useState for the action creator that you simply had defined inline in your first approach. It seems to me you were looking for useCallback instead. So, why don't you mix both like this?

  const useTodos = () => {
    const { state, dispatch } = useContext(Context);
    const fetchTodos = useCallback(async id => {
      const todos = await getTodos(id)
      dispatch({ type: 'SAVE_TODOS', payload: todos })
    }, [dispatch])

    return {
      ...state,
      fetchTodos
    };
}

Your calling code doesn't need that weird check to verify that fetchTodos indeed exists.

export const Todos = id => {
  const { todos, fetchTodos } = useContext(Context);
  useEffect(() => {
    fetchTodos()
  }, []);

  return (
    <div>
      <pre>{JSON.stringify(todos)}</pre>
    </div>
  );
};

Finally, unless you actually need to use this todos + fetchTodos combo from more components down the tree from Todos, which you didn't explictly stated in your question, I think using Context is complicating matters when they're not needed. Remove the extra layer of indirection and call useReducer directly in your useTodos.

It may not be the case here, but I find people are mixing a lot of things in their head and turning something simple into something complicated (like Redux = Context + useReducer).

Hope it helps!

CharlieBrown
  • 3,579
  • 21
  • 20