Как стать автором
Обновить

Комментарии 27

Чтениеwindow.scrollY должно вызывать reflow, на простой разметке это не скажется, а на сложной это может сильно тормозить, особенно когда после reflow вносятся изменения в dom.

хорошее дополнение

Почему чтение window.scrollY именно должно, а не просто может вызывать reflow? По логике даже изменение положения скролла окна часто не требует reflow.

Возможно это надуманная проблема, я не работал с хуками и файбером(он ведь может отложить изменения), да и с реактом в общем давно не работал, так что недостаточно хорошо представляю когда реакт вносит изменения в дом и насколько реально, что после скрола будут внесены изменения до выполнения хука, после внесения изменений в дом window.scrollY тригерит reflow, в данном случае изменение текста на странице - уже изменение требующее пересчета и reflow в любом случае произойдет, другой вопрос может ли чтение при вызове useState вызвать лишний reflow, я бы перестраховался и не читал каждый раз для дефолтного значения.

НЛО прилетело и опубликовало эту надпись здесь

Тоже верно подмечено, спасибо

На throttle функции хорошо бы вызывать `cancel()` в cleanup callback.

useEffect(() => {
    fetchRandomNumber().then(setNumber);
}, []);

лучше переписать так

useEffect(() => {
    let mounted = true
    fetchRandomNumber().then(value => {
        if (mounted) {
            setNumber(value)
        }
    });
    return () => mounted = false
}, []);

Пока там просто асинхронный запрос чего-то, самое худшее, что может случиться — warning в консоли. Но вот если там происходит, например, отправление формы и по успеху делается роутинг типа такого

submitForm().then(() => router.push('/success'))

то можно поломать UX.

Отличное внимание к деталям, чувствуется опыт, побольше бы таких на собесы приходило)

Не соглашусь. Можно смело вызывать setNumber на уже мёртвом компоненте. Ничего не случится. Кажется в свежей версии React убрали (или вот вот уберут) этот warning, из-за бесконечных ложно-позитивных результатов.


Но вот если там происходит, например, отправление формы и по успеху делается роутинг типа такого

ИМХО, только в таком случае и стоит так поступать. А более простой код калечить не стоит. YAGNI.


P.S. мы написали простенький useIsUnmounted: () => (): boolean для таких целей. Компактнее и нагляднее получается. Плюс можно переиспользовать для нескольких callback-ов и effect-ов.


const isUnmountFn = useIsUnmounted();

useEffect(() => {
  whatever().then(() => {
    if (isUnmountFn()) return;
    // do staff
  });
}, [/**/);

Но избегать обыкновенный useState этим — из пушки по воробьям. Лаконичность и простота кодовой базы сильно важнее.

Кажется в свежей версии React убрали (или вот вот уберут) этот warning, из-за бесконечных ложно-позитивных результатов.

update: я кажется что-то напутал. В @latest версии (17.02) warning всё ещё на месте. А того issue найти не удалось. Возможно я что-то напутал. Но выглядит всё так, словно все эти isUnmount и AbortController штуки по любому поводу всё ещё higly recommended. На мой взгляд (может быть наши особенности проекта просто такие), это false positive почти всегда.

На месте интервьювера я бы спросил что такое 37. Ожидал бы, что человек после этого вопроса скажет, что magic numbers это плохо и вынесет это в константу. А заодно даст внятный комментарий. И тут самое интересное — почему 37? Может лучше 16.7 (1/60 секунды).


P.S. забыл дописать — нет обработки ошибок асинхронного запроса (хотя бы console.error)

НЛО прилетело и опубликовало эту надпись здесь

C requestAnimationFrame результат может быть даже лучше.


А если сделать Promise.resolve().then(fn) то вообще ноль покажет (это кстати довольно хардкорный JS вопрос на собесах). Но это уже немного не в тему, да :)

Ну на самом деле же там не 0. Если мерять через Performance API, а не Date.now(), все видно.

Всё так. Но давайте мыслить дискретно позитивно! :-)

НЛО прилетело и опубликовало эту надпись здесь
Не очень понял почему там должен быть не 0

Суть в том что у promise-ов своя приоритетная (над обычной) асинхронная event-loop очередь. Поэтому там 0. Разгадка в этом. В то время как у setTimeout(,0) и даже у requestAnimationFrame это не так. По сути аналог, ЕМНИП, есть в nodejsprocess.nextTick.


ЕМНИП, то с её помощью можно даже повешать браузер\nodejs-процесс.

НЛО прилетело и опубликовало эту надпись здесь
Разве у Promise именно "своя" очередь? Мануалы вроде говорят что они используют очередь микротасок.

Не совсем своя. Я просто не стал вдаваться в детали. Количество и состав очередей зависит от платформы. На мой взгляд эти нюансы мало кому важны, главное просто знать, что эта очередь "микротасок" есть, и с чем её едят. Чтобы, например, не уйти в вечный loop.


А в вашем примере макротаска заканчивается выделением микротаски и микротаска сразу стартует, давая условный 0.

Всё правильно.

НЛО прилетело и опубликовало эту надпись здесь

Я бы сказал, что не "можно", а "нужно".
Вообще по тексту статьи не прослеживается главного — а зачем вообще появился throttle? setState можно фигачить десятками тысяч раз в секунду без каких-либо проблем, это я вам точно говорю. Там тормозить попросту нечему. Что будет тормозить — так это вовсе не setState, а последующий render.


Но да это проблемы архитектурного уровня. Если мы говорим про реальность, а не выдуманный пример — тут надо бы начинать с вопросов типа "вы одним компонентом делаете 2 разные вещи, почему тут не два компонента?"; "задачи вида "получить значение с сервера" должны решаться не компонентами (не презентационным слоем приложения), а стейт-менеджментом, либо же вообще в своем отдельном I/O слое, если у нас проект достаточно масштабен, чтоб его выделить отдельно"; "почему реальность получения значения с сервера замазана гораздо более упрощенной абстракцией, что произойдет, если запрос упадёт, будете вечно показывать Number: undefined?"

а зачем вообще появился throttle?

Нуууу… Скролл бывает очень дробным. Скажем проскроллил тачпадом на 15px, браузер вызвал 15 событий. Из которых 10 попало в один кадр. 10 раз был изменён textContent, а он, к примеру 3 раза поменял layout страницы (не моноширинный шрифт). Где-то плачет Грета и считает углеродный след. Но в целом согласен.

Если это скролл, то имеет смысл добавлять листенер с `{passive: true}`.

Да, любопытно, видимо это было сделано одновременно с установкой дефолта на passive: true. Во всех ли браузерах это так?

Прочитал абзац как

Плюсом идёт кандидату, если он знает что такое ТРОЛЛИНГ и правильно его применяет.

и реально задумался, почему это умение важно для компании...

Зарегистрируйтесь на Хабре, чтобы оставить комментарий

Публикации

Истории