Skip to content

Commit

Permalink
Add more doc
Browse files Browse the repository at this point in the history
  • Loading branch information
garronej committed May 29, 2024
1 parent e0bc617 commit 27beef6
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 29 deletions.
6 changes: 3 additions & 3 deletions src/components/TodoApp/Todo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { tss } from "tss-react";
import { fr } from "@codegouvfr/react-dsfr";
import { Button } from "@codegouvfr/react-dsfr/Button";
import Checkbox from "@mui/material/Checkbox";
import { useConstCallback } from "tools/useConstCallback";
import { useEvent } from "tools/useEvent";

export type Todo = {
id: string;
Expand All @@ -23,9 +23,9 @@ export const Todo = memo((props: TodoProps) => {
const { className, todo, onToggleTodo, onDeleteTodo } = props;

// NOTE: Make sure it's not stale for when used in the reducer.
// We know it's constant because we also used useListCallbacks() in the parent component
// We know it's constant because we also used useListEvent() in the parent component
// but this component is not supposed to be aware of that.
const onUpdateTodoText = useConstCallback(props.onUpdateTodoText);
const onUpdateTodoText = useEvent(props.onUpdateTodoText);

const [isEditing, setIsEditing] = useReducer((isEditing: boolean, isEditing_new: boolean) => {
if (isEditing_new === isEditing) {
Expand Down
34 changes: 28 additions & 6 deletions src/components/TodoApp/TodoApp.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Todo } from "./Todo";
import { AddTodo } from "./AddTodo";
import { tss } from "tss-react";
import { useListCallbacks } from "tools/useListCallbacks";
import { usePartiallyAppliedEvent } from "tools/usePartiallyAppliedEvent";

type Props = {
className?: string;
Expand All @@ -18,23 +18,45 @@ export function TodoApp(props: Props) {
const { classes, cx } = useStyles();

/*
Example:
```ts
const todoId= "123"
const onUpdateEvent = getOnUpdateTodoText(todoId);
const text = "Hello"
onUpdateEvent(text) // Will call onUpdateTodoText(todoId, text);
```
Why is it useful? Because:
`getOnUpdateTodoText(todoId) === getOnUpdateTodoText(todoId)` // is true
The function reference returned by `getOnUpdateTodoText(todoId)` is stable across re-renders.
If we use this custom hook instead of just doing:
onToggleTodo={()=> onToggleTodo(todo.id)}
onUpdateTodoText={()=> onUpdateTodoText(todo.id, todo.text)}
It is because we want to avoid all <Todo /> to be re-rendered every time this component is re-rendered.
For that we use memo() on the Todo component but we also need to make sure that the references of the callbacks
are stable.
Learn more: https://stackblitz.com/edit/react-ts-fyrwng?file=index.tsx
Hot take: The builtin useCallback() hook should never be used. In any scenario.
It almost never enables to avoid rerender and is very error prone. It shouldn't exist in the first place.
https://stackoverflow.com/questions/65890278/why-cant-usecallback-always-return-the-same-ref
Note: This is the state of the art for React 18. React 19 shuffles the deck with it's pre-compiler.
Note: This is the state of the art for React 18. React 19 shuffles the deck with it's pre-compiler
however there's only so much the compiler will be able to infer. It's important to be able to manually
manage our re-rendering strategy.
*/
const getOnUpdateTodoText = useListCallbacks(([todoId]: [string], [text]: [string]) =>
const getOnUpdateTodoText = usePartiallyAppliedEvent(([todoId]: [string], [text]: [string]) =>
onUpdateTodoText(todoId, text)
);
const getOnToggleTodo = useListCallbacks(([todoId]: [string]) => onToggleTodo(todoId));
const getOnDeleteTodo = useListCallbacks(([todoId]: [string]) => onDeleteTodo(todoId));
const getOnToggleTodo = usePartiallyAppliedEvent(([todoId]: [string]) => onToggleTodo(todoId));
const getOnDeleteTodo = usePartiallyAppliedEvent(([todoId]: [string]) => onDeleteTodo(todoId));

return (
<div className={cx(classes.root, className)}>
Expand Down
7 changes: 5 additions & 2 deletions src/tools/useConstCallback.ts → src/tools/useEvent.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import { useRef, useState } from "react";
import { Parameters } from "tsafe/Parameters";

/** https://stackoverflow.com/questions/65890278/why-cant-usecallback-always-return-the-same-ref */
export function useConstCallback<T extends ((...args: any[]) => unknown) | undefined | null>(
/**
* https://stackoverflow.com/questions/65890278/why-cant-usecallback-always-return-the-same-ref
* https://github.com/reactjs/rfcs/blob/useevent/text/0000-useevent.md
**/
export function useEvent<T extends ((...args: any[]) => unknown) | undefined | null>(
callback: NonNullable<T>
): T {
const callbackRef = useRef<typeof callback>(null as any);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,16 @@ import { useRef, useState } from "react";
import { memoize } from "./memoize";
import { id } from "tsafe/id";

export type CallbackFactory<FactoryArgs extends unknown[], Args extends unknown[], R> = (
export type PartiallyApplierEvent<FactoryArgs extends unknown[], Args extends unknown[], R> = (
...factoryArgs: FactoryArgs
) => (...args: Args) => R;

/**
* https://docs.powerhooks.dev/api-reference/useListCallbacks
*
* const callbackFactory= useListCallbacks(
* ([key]: [string], [params]: [{ foo: number; }]) => {
* ...
* },
* []
* );
*
* WARNING: Factory args should not be of variable length.
*
*/
export function useListCallbacks<FactoryArgs extends unknown[], Args extends unknown[], R = void>(
callback: (...callbackArgs: [FactoryArgs, Args]) => R
): CallbackFactory<FactoryArgs, Args, R> {
type Out = CallbackFactory<FactoryArgs, Args, R>;
export function usePartiallyAppliedEvent<
FactoryArgs extends unknown[],
Args extends unknown[],
R = void
>(callback: (...callbackArgs: [FactoryArgs, Args]) => R): PartiallyApplierEvent<FactoryArgs, Args, R> {
type Out = PartiallyApplierEvent<FactoryArgs, Args, R>;

const callbackRef = useRef<typeof callback>(callback);

Expand Down

0 comments on commit 27beef6

Please sign in to comment.