Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vas3k review #1

Open
yumauri opened this issue Feb 6, 2024 · 1 comment
Open

vas3k review #1

yumauri opened this issue Feb 6, 2024 · 1 comment

Comments

@yumauri
Copy link

yumauri commented Feb 6, 2024

👋

const reRender = () => set({});

React hook FAQ советует использовать следующий подход:

const [, reRender] = useReducer(x => x + 1, 0);

baguni/src/index.js

Lines 30 to 36 in bbab8bb

const reRender = () => set({});
useEffect(() => {
bState.joinReRender(reRender);
return () => {
bState.cancelReRender(reRender);
};
}, [bState]);

Функция reRender создаётся новая на каждый рендер, а хук useEffect отрабатывает только на изменение bState, в результате чего в cancelReRender попадает другая функция reRender, чем была добавлена в joinReRender, то есть старые функции не удаляются, что вызывает утечку памяти. Если заюзать подход из п.1, проблема должна уйти, т.к. функция будет стабильная.

baguni/src/index.js

Lines 37 to 39 in bbab8bb

function setState(newState) {
bState.set(newState);
}

Функция setState создаётся новая на каждый рендер, это поведение не аналогично хуку useState, где функция стабильная. Это может вызвать проблемы с рендерами, если пользователь не будет учитывать этот момент.

return Object.freeze(Object.create(prototype));

Создание объекта через Object.freeze(Object.create(...)) выглядит как ненужное переусложнение на мой взгляд. Это выглядит как попытка защитить объект от изменений вручную, но такая «защита» без проблем обходится
image

PS
Я не пробовал код в работе, это просто визуальное ревью.
Замечу, что написание стейт менеджера это очень сложная задача, гораздо сложнее, чем кажется на первый взгляд.

@i2gor87
Copy link
Owner

i2gor87 commented Feb 11, 2024

Спасибо за ревью!
С reducer-ами попробую, но боюсь что это усложнит API библиотеки.
Про хак кстати интересно, надо будет подумать, можно ли защититься от этого внутри библиотеки

В целом, я собирался написать не столько всеобъемлющий стейт менеджер, а сделать маленькое глобальное хранилище, чтобы делиться переменными в разных компонентах

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants