From 27beef67c19b99c5a4c92ba187acfc8e50d5fbbd Mon Sep 17 00:00:00 2001 From: Joseph Garrone Date: Wed, 29 May 2024 16:48:34 +0200 Subject: [PATCH 1/2] Add more doc --- src/components/TodoApp/Todo.tsx | 6 ++-- src/components/TodoApp/TodoApp.tsx | 34 +++++++++++++++---- .../{useConstCallback.ts => useEvent.ts} | 7 ++-- ...llbacks.ts => usePartiallyAppliedEvent.ts} | 25 ++++---------- 4 files changed, 43 insertions(+), 29 deletions(-) rename src/tools/{useConstCallback.ts => useEvent.ts} (58%) rename src/tools/{useListCallbacks.ts => usePartiallyAppliedEvent.ts} (61%) diff --git a/src/components/TodoApp/Todo.tsx b/src/components/TodoApp/Todo.tsx index 09f768e..b3c71f6 100644 --- a/src/components/TodoApp/Todo.tsx +++ b/src/components/TodoApp/Todo.tsx @@ -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; @@ -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) { diff --git a/src/components/TodoApp/TodoApp.tsx b/src/components/TodoApp/TodoApp.tsx index 4efbd53..2286e2e 100644 --- a/src/components/TodoApp/TodoApp.tsx +++ b/src/components/TodoApp/TodoApp.tsx @@ -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; @@ -18,8 +18,27 @@ 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 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. @@ -27,14 +46,17 @@ export function TodoApp(props: Props) { 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 (
diff --git a/src/tools/useConstCallback.ts b/src/tools/useEvent.ts similarity index 58% rename from src/tools/useConstCallback.ts rename to src/tools/useEvent.ts index da58134..b54488d 100644 --- a/src/tools/useConstCallback.ts +++ b/src/tools/useEvent.ts @@ -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 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 unknown) | undefined | null>( callback: NonNullable ): T { const callbackRef = useRef(null as any); diff --git a/src/tools/useListCallbacks.ts b/src/tools/usePartiallyAppliedEvent.ts similarity index 61% rename from src/tools/useListCallbacks.ts rename to src/tools/usePartiallyAppliedEvent.ts index 2c6f1d9..6869e05 100644 --- a/src/tools/useListCallbacks.ts +++ b/src/tools/usePartiallyAppliedEvent.ts @@ -2,27 +2,16 @@ import { useRef, useState } from "react"; import { memoize } from "./memoize"; import { id } from "tsafe/id"; -export type CallbackFactory = ( +export type PartiallyApplierEvent = ( ...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( - callback: (...callbackArgs: [FactoryArgs, Args]) => R -): CallbackFactory { - type Out = CallbackFactory; +export function usePartiallyAppliedEvent< + FactoryArgs extends unknown[], + Args extends unknown[], + R = void +>(callback: (...callbackArgs: [FactoryArgs, Args]) => R): PartiallyApplierEvent { + type Out = PartiallyApplierEvent; const callbackRef = useRef(callback); From cc8e11f1ea7a115bbbe1ca8ad26f75cc71fdb652 Mon Sep 17 00:00:00 2001 From: Joseph Garrone Date: Wed, 29 May 2024 17:09:11 +0200 Subject: [PATCH 2/2] Kitchen sink to manage rendering budget --- .eslintrc.cjs | 3 +- package.json | 1 + src/components/TodoApp/Todo.tsx | 4 +- src/tools/deepEqual/deepEqual.ts | 237 +++++++++++++++++++++++ src/tools/deepEqual/getPrototypeChain.ts | 27 +++ src/tools/deepEqual/index.ts | 1 + src/tools/deepEqual/types.ts | 55 ++++++ 7 files changed, 326 insertions(+), 2 deletions(-) create mode 100644 src/tools/deepEqual/deepEqual.ts create mode 100644 src/tools/deepEqual/getPrototypeChain.ts create mode 100644 src/tools/deepEqual/index.ts create mode 100644 src/tools/deepEqual/types.ts diff --git a/.eslintrc.cjs b/.eslintrc.cjs index ac77fb9..d2815e5 100644 --- a/.eslintrc.cjs +++ b/.eslintrc.cjs @@ -12,6 +12,7 @@ module.exports = { rules: { "react-refresh/only-export-components": ["warn", { allowConstantExport: true }], "@typescript-eslint/no-unused-vars": "warn", - "@typescript-eslint/no-explicit-any": "off" + "@typescript-eslint/no-explicit-any": "off", + "@typescript-eslint/no-namespace": "off" } }; diff --git a/package.json b/package.json index 4e926cb..f97d77e 100644 --- a/package.json +++ b/package.json @@ -27,6 +27,7 @@ "@tanstack/react-router": "^1.33.6", "axios": "^1.7.1", "dayjs": "^1.11.11", + "lodash": "^4.17.21", "oidc-spa": "^4.6.2", "react": "^18.2.0", "react-dom": "^18.2.0", diff --git a/src/components/TodoApp/Todo.tsx b/src/components/TodoApp/Todo.tsx index b3c71f6..9c84f9d 100644 --- a/src/components/TodoApp/Todo.tsx +++ b/src/components/TodoApp/Todo.tsx @@ -4,6 +4,7 @@ import { fr } from "@codegouvfr/react-dsfr"; import { Button } from "@codegouvfr/react-dsfr/Button"; import Checkbox from "@mui/material/Checkbox"; import { useEvent } from "tools/useEvent"; +import { deepEqual } from "tools/deepEqual"; export type Todo = { id: string; @@ -80,7 +81,8 @@ export const Todo = memo((props: TodoProps) => {
); -}); +}, deepEqual); +// NOTE: We use deepEqual above to avoid having the component re-render if the ref of the todo has changed but it's actually the same todo. const useStyles = tss .withName({ Todo }) diff --git a/src/tools/deepEqual/deepEqual.ts b/src/tools/deepEqual/deepEqual.ts new file mode 100644 index 0000000..e112b9e --- /dev/null +++ b/src/tools/deepEqual/deepEqual.ts @@ -0,0 +1,237 @@ +/* eslint-disable @typescript-eslint/ban-types */ +import { MapLike, SetLike, DateLike, ArrayLike } from "./types"; +import { assert } from "tsafe/assert"; +import { is } from "tsafe/is"; + +/** + * Function that perform a in depth comparison of two things of arbitrary type T + * to see if they represent the deepEqual date regardless of object references. + * + * Think of it as JSON.stringify(o1) === JSON.stringify(o2) + * but unlike a test performed with JSON.stringify the order in the property + * have been assigned to an object does not matter and circular references are supported. + * + * + * If takeIntoAccountArraysOrdering === false then + * representsSameData(["a", "b"], ["b", "a"]) will return true. + * + * If Date are compared via .getTime() + * + * The objects can includes Map and Set. + * */ +export const deepEqual = (() => { + function sameRec( + o1: T, + o2: T, + { takeIntoAccountArraysOrdering }: { takeIntoAccountArraysOrdering: boolean } = { + takeIntoAccountArraysOrdering: true + }, + o1Path: { key: string; obj: any }[], + o2Path: { key: string; obj: any }[], + o1RealRef: T = o1, + o2RealRef: T = o2 + ): boolean { + if (Object.is(o1, o2)) { + return true; + } + + { + const i1 = o1Path.map(({ obj }) => obj).indexOf(o1RealRef); + + if (i1 >= 0) { + const i2 = o2Path.map(({ obj }) => obj).indexOf(o2RealRef); + + if (i1 !== i2) { + return false; + } + + { + const [a, b] = ([o1Path, o2Path] as const).map(oPath => + oPath.map(({ key }) => key).join("") + ); + + return a === b; + } + } + } + + if (!(o1 instanceof Object && o2 instanceof Object)) { + return false; + } + + if (typeof o1 === "function" || typeof o2 === "function") { + return false; + } + + if (DateLike.match(o1)) { + if (!DateLike.match(o2)) { + return false; + } + + return o1.getTime() === o2!.getTime(); + } + + if (MapLike.match(o1)) { + if (!MapLike.match(o2)) { + return false; + } + + type Entry = { key: any; value: any }; + + const newO1 = new Set(); + const newO2 = new Set(); + + for (const o of [o1, o2]) { + const newO = o === o1 ? newO1 : newO2; + + const arr = Array.from(o.keys()); + + for (let i = 0; i < arr.length; i++) { + const key = arr[i]; + const value = o.get(key)!; + + newO.add({ key, value }); + } + } + + return sameRec( + newO1, + newO2, + { takeIntoAccountArraysOrdering }, + o1Path, + o2Path, + o1RealRef as any, + o2RealRef + ); + } + + let takeIntoAccountArraysOrderingOv: false | undefined = undefined; + + if (SetLike.match(o1)) { + if (!SetLike.match(o2)) { + return false; + } + + o1 = Array.from(o1.values()) as any; + o2 = Array.from(o2.values()) as any; + + takeIntoAccountArraysOrderingOv = false; + } + + //NOTE: The two following lines shouldn't be necessary... + assert(is(o1)); + assert(is(o2)); + + if (ArrayLike.match(o1)) { + if (!ArrayLike.match(o2)) { + return false; + } + + if (o1.length !== o2.length) { + return false; + } + + if ( + !(takeIntoAccountArraysOrderingOv !== undefined + ? takeIntoAccountArraysOrderingOv + : takeIntoAccountArraysOrdering) + ) { + const o2Set = new Set(Array.from(o2)); + + for (let i = 0; i < o1.length; i++) { + if (!(`${i}` in o1)) { + continue; + } + + const val1 = o1[i]; + + if (o2Set.has(val1)) { + o2Set.delete(val1); + continue; + } + + let isFound = false; + + for (const val2 of o2Set.values()) { + if ( + !sameRec( + val1, + val2, + { takeIntoAccountArraysOrdering }, + [...o1Path, { obj: o1RealRef, key: "*" }], + [...o2Path, { obj: o2RealRef, key: "*" }] + ) + ) { + continue; + } + + isFound = true; + o2Set.delete(val2); + break; + } + + if (!isFound) { + return false; + } + } + + return true; + } + + //continue + } else if ( + !sameRec( + Object.keys(o1).filter(key => (o1 as any)[key] !== undefined), + Object.keys(o2).filter(key => (o2 as any)[key] !== undefined), + { takeIntoAccountArraysOrdering: false }, + [], + [] + ) + ) { + return false; + } + + for (const key in o1) { + if ( + !sameRec( + (o1 as any)[key], + (o2 as any)[key], + { takeIntoAccountArraysOrdering }, + [...o1Path, { obj: o1RealRef, key }], + [...o2Path, { obj: o2RealRef, key }] + ) + ) { + return false; + } + } + + return true; + } + + return function deepEqual( + o1: T, + o2: T, + { takeIntoAccountArraysOrdering }: { takeIntoAccountArraysOrdering: boolean } = { + takeIntoAccountArraysOrdering: true + } + ): boolean { + return sameRec(o1, o2, { takeIntoAccountArraysOrdering }, [], []); + }; +})(); + +/** + * Return the "deepEqual" function with "takeIntoAccountArraysOrdering" default value set as desired. + * */ +export function deepEqualFactory({ + takeIntoAccountArraysOrdering +}: { + takeIntoAccountArraysOrdering: boolean; +}) { + return { + deepEqual: ( + o1: T, + o2: T, + prop: { takeIntoAccountArraysOrdering: boolean } = { takeIntoAccountArraysOrdering } + ) => deepEqual(o1, o2, prop) + }; +} diff --git a/src/tools/deepEqual/getPrototypeChain.ts b/src/tools/deepEqual/getPrototypeChain.ts new file mode 100644 index 0000000..e6b5b28 --- /dev/null +++ b/src/tools/deepEqual/getPrototypeChain.ts @@ -0,0 +1,27 @@ +/* eslint-disable @typescript-eslint/ban-types */ + +export function getPrototypeChain(obj: Object, callback?: (proto: Object) => boolean): Object[] { + const proto = Object.getPrototypeOf(obj); + + if (!proto) { + return []; + } + + const doContinue = callback?.(proto); + + if (!doContinue) { + return [proto]; + } + + return [proto, ...getPrototypeChain(proto)]; +} +getPrototypeChain.isMatched = (obj: Object, regExp: RegExp): boolean => { + let out = false; + + getPrototypeChain(obj, ({ constructor }) => { + out = regExp.test(constructor.name); + return !out; + }); + + return out; +}; diff --git a/src/tools/deepEqual/index.ts b/src/tools/deepEqual/index.ts new file mode 100644 index 0000000..9ecde1e --- /dev/null +++ b/src/tools/deepEqual/index.ts @@ -0,0 +1 @@ +export * from "./deepEqual"; diff --git a/src/tools/deepEqual/types.ts b/src/tools/deepEqual/types.ts new file mode 100644 index 0000000..23b7ea3 --- /dev/null +++ b/src/tools/deepEqual/types.ts @@ -0,0 +1,55 @@ +/* eslint-disable @typescript-eslint/ban-types */ +import { typeGuard } from "tsafe/typeGuard"; +import { getPrototypeChain } from "./getPrototypeChain"; + +type SetLike = { + values: () => Iterable; +}; + +export namespace SetLike { + export function match(set: Object): set is SetLike { + return ( + typeGuard>(set, true) && + typeof set.values === "function" && + getPrototypeChain.isMatched(set, /Set/) + ); + } +} + +export type MapLike = { + keys: () => Iterable; + get(key: T): U | undefined; +}; + +export namespace MapLike { + export function match(map: Object): map is MapLike { + return ( + typeGuard>(map, true) && + typeof map.keys === "function" && + typeof map.get === "function" && + getPrototypeChain.isMatched(map, /Map/) + ); + } +} + +export namespace ArrayLike { + export function match(arr: Object): arr is ArrayLike { + return typeGuard>(arr, true) && typeof arr.length === "number" && arr.length !== 0 + ? `${arr.length - 1}` in arr + : getPrototypeChain.isMatched(arr, /Array/); + } +} + +export type DateLike = { + getTime: () => number; +}; + +export namespace DateLike { + export function match(date: Object): date is DateLike { + return ( + typeGuard(date, true) && + typeof date.getTime === "function" && + getPrototypeChain.isMatched(date, /Date/) + ); + } +}