Pull to refresh

Comments 27

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

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

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

Еще использование window напрямую может принести сюрпризы когда вы решите перейти на SSR, — его там просто не будет.

На 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)

Раньше вроде бы была история что, все что ниже 25ms для браузера(ов) едино, может быть для ИЕ только, не помню уже. Но вот я проверил в свежем хроме, вроде бы для него это работает почти корректно


var t1 = Date.now();
setTimeout(()=> console.log(Date.now()-t1), 10); //12

Вообще можно попробовать переписать это вместо throttle на requestAnimationFrame

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


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

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

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

Не очень понял почему там должен быть не 0.
Вот если бы 0 был в таком коде, я бы удивился:

var t1 = Date.now();
function f() { console.log('diff: ',Date.now() - t1) };
Promise.resolve(0).then(f);
var x = 0;
while (x < 1000_000_000) { x++ }

Если я не правильно понял вас, поправьте пожалуйста.

Не очень понял почему там должен быть не 0

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


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

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

Собственно именно поэтому в моем примере у нас выводится не 0, т.к. мы сначала выполняем текущую макротаску, в ее процессе создаем микротаску (.then) которая ждет завершения текущей макротаски, а та не завершается т.к. выполняет очень длинный цикл. Когда цикл закончится, макротаска выполнится, , мы начнем выполнять микротаску и выведем в консоль время.

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

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

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

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


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

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

Значит все-таки мы об одном и том же. Ну и чудно, спасибо за диалог)

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


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

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

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

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

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

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

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

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

Only those users with full accounts are able to leave comments. Log in, please.