Conversation
rosa
changed the title
rosa
force-pushed
the
offline-mode
branch
5 times, most recently
from
4b708a1 to
8ed57ae
Compare
There was a problem hiding this comment.
Pull request overview
Adds an offline mode to Fizzy by introducing Turbo Offline service-worker caching rules and wiring cache clearing into auth-related flows, backed by a Turbo-Rails branch that includes offline-cache support.
Changes:
- Pin and initialize Turbo Offline, registering a service worker for cached offline access.
- Replace the existing service worker with a rule-based TurboOffline configuration for documents/assets/storage and keep push notification handling.
- Clear offline caches on sign-out and (intended) on magic-link sign-in; switch turbo-rails gem to the
offline-cachebranch.
Reviewed changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| config/importmap.rb | Pins Turbo Offline module for use by the JS app. |
| app/views/users/show.html.erb | Adds cache-clearing Stimulus behavior to the sign-out form. |
| app/views/sessions/magic_links/show.html.erb | Adds cache-clearing Stimulus behavior to the magic-link sign-in form. |
| app/views/pwa/service_worker.js.erb | New TurboOffline-driven service worker with caching rules + existing push handling. |
| app/views/pwa/service_worker.js | Removes the prior minimal fetch handler service worker. |
| app/views/my/menus/_settings.html.erb | Adds cache-clearing Stimulus behavior to the sign-out form in settings menu. |
| app/javascript/initializers/offline.js | Starts Turbo Offline service worker registration for signed-in users. |
| app/javascript/initializers/index.js | Registers the new offline initializer. |
| app/javascript/controllers/clear_offline_cache_controller.js | Adds a controller to clear the Turbo Offline cache on form submit. |
| Gemfile | Switches turbo-rails to the offline-cache GitHub branch. |
| Gemfile.lock | Locks turbo-rails to the specified Git revision/branch. |
| Gemfile.saas.lock | Locks turbo-rails to the specified Git revision/branch for SaaS lockfile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { Turbo } from "@hotwired/turbo-rails" | ||
|
|
||
| if (Current.user) { | ||
| Turbo.offline.start("/service-worker.js", { | ||
| scope: "/", |
There was a problem hiding this comment.
This initializer calls Turbo.offline.start(...) but doesn’t import @hotwired/turbo/offline. If that module isn’t loaded elsewhere, Turbo.offline will be undefined and offline mode will fail to start. Import the offline module before calling Turbo.offline.* (or guard against it being missing).
| </header> | ||
|
|
||
| <%= form_with url: session_magic_link_path, method: :post, html: { data: { controller: "magic-link" } } do |form| %> | ||
| <%= form_with url: session_magic_link_path, method: :post, html: { data: { controller: "magic-link clear-offline-cache", action: "submit->clear-offline-cache#clearCache" } } do |form| %> |
There was a problem hiding this comment.
This form uses a submit->clear-offline-cache#clearCache action, but magic_link_controller submits via this.element.submit(), which bypasses the submit event. That means the offline cache won’t be cleared for the common “paste/enter to submit” flows. Consider switching the magic-link controller to requestSubmit() (or explicitly calling the cache-clear logic) so the Stimulus submit action reliably runs.
| }) | ||
|
|
||
| // Everything else | ||
| TurboOffline.addRule({ |
There was a problem hiding this comment.
The catch-all rule excludes /service-worker but not /service-worker.js (the URL being registered in Turbo.offline.start). As written, /service-worker.js can still match this rule; consider excluding the .js variant too (e.g., service-worker(\.js)?) to avoid caching/handling the service worker script unintentionally.
| TurboOffline.addRule({ | |
| except: /\/(service-worker(\.js)?|edit|pin|watch|new)$/, |
|
|
||
| pin "application" | ||
| pin "@hotwired/turbo-rails", to: "turbo.min.js" | ||
| pin "@hotwired/turbo/offline", to: "turbo-offline.min.js" |
There was a problem hiding this comment.
@hotwired/turbo/offline is pinned here, but there’s no import of this module in the JS bundle. Unless turbo-rails is implicitly loading it, Turbo.offline will be undefined at runtime and this pin will be unused. Consider importing @hotwired/turbo/offline from app/javascript/application.js or the offline initializer so the offline API is actually registered.
| pin "@hotwired/turbo/offline", to: "turbo-offline.min.js" |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 12 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| TurboOffline.addRule({ | ||
| except: /\/(service-worker|edit|pin|watch|new)$/, | ||
| handler: TurboOffline.handlers.networkFirst({ |
There was a problem hiding this comment.
The except: /\/(service-worker|edit|pin|watch|new)$/ regex won’t exclude /service-worker.js (the actual registration URL used elsewhere in the app) because of the .js/format suffix. This rule may also fail to exclude those paths when they have query strings. Update the exclusion to account for the optional format (.js) and query string, or match against new URL(request.url).pathname.
| match: (request) => request.destination === "document", | ||
| except: /\/(edit|pin|watch|new)$/, | ||
| handler: TurboOffline.handlers.networkFirst({ |
There was a problem hiding this comment.
The except: /\/(edit|pin|watch|new)$/ pattern won’t exclude URLs that include a query string or trailing slash (e.g. /cards/123/edit?from=...), so those write-related pages can still be cached. Consider matching on new URL(request.url).pathname or extending the regex to allow optional /? and \?.* before end-of-string.
| match: /\/(boards|cards|users)\//, | ||
| except: /\/(edit|pin|watch|new)$/, | ||
| handler: TurboOffline.handlers.networkFirst({ |
There was a problem hiding this comment.
Same issue here: except: /\/(edit|pin|watch|new)$/ only matches URLs that end exactly with those segments, so /boards/.../new?foo=1 (or a trailing slash) won’t be excluded and may be cached. Prefer checking URL(...).pathname or allowing optional query string/trailing slash in the pattern.
rosa
force-pushed
the
offline-mode
branch
2 times, most recently
from
55bb39d to
215a494
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 13 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { Turbo } from "@hotwired/turbo-rails" | ||
|
|
||
| if (Current.user) { | ||
| Turbo.offline.start("/service-worker.js", { | ||
| scope: "/", | ||
| native: true, | ||
| preload: /\/assets\// | ||
| }) |
There was a problem hiding this comment.
Turbo.offline is used here but the offline extension module isn’t imported anywhere (app/javascript/application.js only imports @hotwired/turbo-rails). Without importing @hotwired/turbo/offline (or whatever module provides the offline patch), Turbo.offline will be undefined and this initializer will throw on page load.
| import { Controller } from "@hotwired/stimulus" | ||
| import { Turbo } from "@hotwired/turbo-rails" | ||
|
|
||
| export default class extends Controller { | ||
| clearCache() { | ||
| Turbo.offline.clearCache() | ||
| } |
There was a problem hiding this comment.
This controller calls Turbo.offline.clearCache(), but the offline extension module isn’t imported anywhere (only @hotwired/turbo-rails is imported in application.js). If @hotwired/turbo/offline isn’t loaded before this runs, Turbo.offline will be undefined and signing in/out will error.
This change conditionally renders the TurboOffline caching rules based on whether the request comes from a Hotwire Native app. Web browsers get a minimal service worker that only handles push notifications and a simple document fetch fallback. Why separate behavior for native vs web? ----------------------------------------- We want offline caching for Hotwire Native apps (where users expect app-like offline behavior) but not for regular web browsers. Why use ERB conditional rendering? ---------------------------------- We explored several approaches to detect Hotwire Native requests in the service worker: 1. User-Agent detection in fetch handler: Would be ideal, but Android WebViews override the User-Agent header with the system default when requests are intercepted by service workers, stripping the custom "Hotwire Native" identifier. 2. Custom header (X-Hotwire-Native) from native apps: Would require intercepting ALL requests at the native level using both WebViewClient.shouldInterceptRequest() and ServiceWorkerClient. Complex to implement and still incomplete coverage for all request types (navigation, resources loaded by HTML). 3. In-memory flag with IndexedDB persistence: Service workers can be terminated when idle and restart with fresh state. Reading from IndexedDB is async, but the decision to call respondWith() in a fetch handler must be synchronous. 4. Separate service worker URLs: Same theoretical churn problem as ERB rendering, with more complexity. Why ERB conditional rendering works in practice ----------------------------------------------- The main concern with conditional ERB rendering was "churn" — the service worker constantly updating as different client types fetch it. However, this only happens when web and native share storage on the same device. In practice, this is rare because: - Android native apps use isolated WebView storage - iOS doesn't support service workers in WebViews (yet) - Web browsers have completely separate storage So each context gets its own stable service worker without churn. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Adds a logout Stimulus controller that sends a message to the service
worker to clear all cached content when the user logs out. This ensures
that cached data from one user isn't accessible after logout.
The implementation:
- Adds logout_controller.js that posts { action: "clearCache" } to the
service worker via postMessage
- Updates logout buttons to use the controller on form submission
Also fixes data-turbo placement: moved from button to form element where
it actually takes effect for disabling Turbo Drive form submissions.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Previously, offline caching was conditionally enabled only for Hotwire Native apps. This removes that restriction and enables offline support for all users, including PWAs and regular browsers. This Uses the new `fetchOptions` support in `TurboOffline` handlers to pass `cache: "no-cache"` for document fetches, which we need to work around a quite annoying Safari PWA bug (see #1014). Also, simplify a bit the cache names and remove the `misc` one. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Cached video and audio files.
1MB and no limit for number of entries except for attachments, where we limit individual entries to 2MB and total of entries to 500. The number is based on the following percentiles for Active Storage blobs: ``` p50: 97.1044921875 KB p75: 236.9140625 KB p90: 917.7548828125 KB ```
When the service worker is registered for the first time, resources loaded
before it becomes active won't go through the service worker. These resources
may be served from the browser's HTTP cache on subsequent requests, bypassing
the service worker cache entirely.
The new `preload` option accepts a regex pattern. On first visit (when no
controller exists), it waits for the service worker to take control, then
sends a message with URLs from `performance.getEntriesByType("resource")`
that match the pattern. The service worker fetches and caches these resources.
Rename logout controller to clear-offline-cache and attach it to the magic link verification form so the service worker cache is cleared when a different user signs in. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Simplify the clear-offline-cache controller to use the new Turbo.offline.clearCache() API instead of messaging the service worker directly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
We don't need the service worker to cache itself, or pages that won't work offline anyway.
Gate Turbo.offline.start() behind Current.user so the service worker is only registered for authenticated users. This avoids errors on unauthenticated pages where /service-worker.js requires auth. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Otherwise, for resources like images loaded via <img> tags, the browser sets `mode: "no-cors"` (as these aren't CORS requests), so the service worker gets an opaque response even though the server sends CORS headers. We could upgrade all no-cors requests to cors mode, sending a `mode: "cors"` request to a server that doesn't send CORS headers will fail entirely: the `fetch` call throws a `TypeError` (network error), and the browser blocks the response. So the resource wouldn't load at all, not even as opaque. We wouldn't be able to cache it at all. By opting in via `fetchOptions`, we can do it only for rules where we know the server will send CORS headers.
CORS mode doesn't work with redirect-based Active Storage URLs when the storage bucket uses wildcard Access-Control-Allow-Origin. Relying on maxEntries alone until specific origin CORS is available. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The load balancer now returns a specific `Access-Control-Allow-Origin` instead of a wildcard, so credentials can be included by default. This enables `maxEntrySize` enforcement for storage. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This adds offline mode to Fizzy, for web and the upcoming Hotwire Native apps. This is the simplest version where only stuff you've already seen is available offline. Any write action will fail.
This relies on hotwired/turbo#1427, which has been extended with more functionality after testing this on Fizzy, and extends the service worker that was only handling web push notifications to cache resources for offline access, with different rules depending on the nature of the resources.