[SOK-31] Service Worker #19
Conversation
| if (cachedResponse) { | ||
| return cachedResponse | ||
| } | ||
| return new Response('Network error happened', { |
There was a problem hiding this comment.
Выглядит так, что где-то тут приложение упадет, если пользователь не залогинен. Нам нужно как-то обработать сценарий, где пользователь попал на главую, приложение закешировалось, затем интернет отвалился. У пользователя должна остаться возможность хотябы попасть на главую.
Верхом будет, если закинуть ему тостер, что "Ваше интернет соединение не стабильно".
There was a problem hiding this comment.
Так смотри. Если после отвала интернета, пользователь заходит в приложение (на главную), то тут сработает сценарий на 19 строке. Он достанет страницу из кеша и отдаст. Так что тут всё хорошо в плане логики.
| const DYNAMIC_CACHE_NAME = 'dynamic-data-v1' | ||
| const urlsToCache = [ | ||
| '/', | ||
| '/manifest.json', |
There was a problem hiding this comment.
А имеет ли смысл тут указывать все файлы, если первый "/" уже покрывает все остальные?
| } | ||
| return response | ||
| } catch (error) { | ||
| console.error('Fetch failed; returning cached page instead.', error) |
There was a problem hiding this comment.
Возможно не стоит это указывать в error. Ведь это поведение мы отлавливаем. И ожидаем, что такой сценарий может быть.
Возможно стоит warning или info использовать. Но не уверен.
There was a problem hiding this comment.
Тут на ваше усмотрение. Консоль только разработчики открывают. Если так удобно, то окей.
В реальном же мире, на реальном продукте, тут будет отправка запроса в аналитику (раз интернета нет, то сохранение где-то в localStorage, а отправка уже при появлении инета), чтобы аналитики, да и разработчики могли отслеживать графики того, как часто у юзеров отваливается инет
| const cache = await caches.open(DYNAMIC_CACHE_NAME) | ||
| try { | ||
| const response = await fetch(request) | ||
| if (response && response.status === 200 && response.type === 'basic') { |
There was a problem hiding this comment.
Возможно стоит вынести в переменную isResponseSuccess, так как если добавиться хотя бы еще одно условие, может быть слишком много условий в if-е.
There was a problem hiding this comment.
Поддерживаю. Доп. переменные — это всегда хорошо, потому что код становится более читаемым.
| @@ -0,0 +1,58 @@ | |||
| const STATIC_CACHE_NAME = 'static-data-v1' | |||
There was a problem hiding this comment.
Не понимаю смысла разделения тут кеша на два. Если направишь меня, буду благодарен.
There was a problem hiding this comment.
Честно, я тоже не до конца понимаю. Я изначально подумал, что тут просто несколько стратегий реализуется, и для каждой стратегии свой кеш.
Думаю, что в данном случае, это просто доп. усложнение, так как придётся в случае сброса кеша, обновлять две переменные, а не одну.
| const cache = await caches.open(DYNAMIC_CACHE_NAME) | ||
| try { | ||
| const response = await fetch(request) | ||
| if (response && response.status === 200 && response.type === 'basic') { |
There was a problem hiding this comment.
Поддерживаю. Доп. переменные — это всегда хорошо, потому что код становится более читаемым.
| @@ -0,0 +1,58 @@ | |||
| const STATIC_CACHE_NAME = 'static-data-v1' | |||
There was a problem hiding this comment.
Честно, я тоже не до конца понимаю. Я изначально подумал, что тут просто несколько стратегий реализуется, и для каждой стратегии свой кеш.
Думаю, что в данном случае, это просто доп. усложнение, так как придётся в случае сброса кеша, обновлять две переменные, а не одну.
| return | ||
| } | ||
|
|
||
| if (!(url.indexOf('http') === 0)) { |
There was a problem hiding this comment.
Можно сразу !== сделать, чем отрицать равенство, через !.
Можно лучше: немного сложная проверка, потому что сложно читать, и легко ошибиться. Давай разбираться. Ты ищешь в урле http. Метод indexOf либо вернёт индекс, либо -1. Ты проверяешь, что http находится не в начале строки. И если это так, то делаешь return. То есть ты не обрабатываешь не http(s) запросы через sw.
Тут понятно. Но можно красивее:
!url.startsWith('http')
| } | ||
| return response | ||
| } catch (error) { | ||
| console.error('Fetch failed; returning cached page instead.', error) |
There was a problem hiding this comment.
Тут на ваше усмотрение. Консоль только разработчики открывают. Если так удобно, то окей.
В реальном же мире, на реальном продукте, тут будет отправка запроса в аналитику (раз интернета нет, то сохранение где-то в localStorage, а отправка уже при появлении инета), чтобы аналитики, да и разработчики могли отслеживать графики того, как часто у юзеров отваливается инет
| return cachedResponse | ||
| } | ||
| return new Response('Network error happened', { | ||
| status: 408, |
There was a problem hiding this comment.
Тут можно кинуть 408, но чаще тут кидают 503
| if (cachedResponse) { | ||
| return cachedResponse | ||
| } | ||
| return new Response('Network error happened', { |
There was a problem hiding this comment.
Так смотри. Если после отвала интернета, пользователь заходит в приложение (на главную), то тут сработает сценарий на 19 строке. Он достанет страницу из кеша и отдаст. Так что тут всё хорошо в плане логики.
### Какую задачу решаем
Скриншоты/видяшка (если есть)
TBD (если есть)