fix(05-review): NEW-CR-01 pre-resolve SW registration to eliminate await before pushManager.subscribe
Pre-fetch ServiceWorkerRegistration into component state via useEffect in both PushPermissionPrompt and SettingsSheet. Gate the subscribe tap target as disabled until both vapidKey AND swRegistration are ready. The tap handler now has zero awaits between the user gesture and registration.pushManager.subscribe(), fully satisfying the iOS user-gesture requirement.
This commit is contained in:
@@ -83,6 +83,11 @@ export function PushPermissionPrompt({ onClose }: PushPermissionPromptProps) {
|
|||||||
// subscribe(registration, vapidKey) without any await before pushManager.subscribe().
|
// subscribe(registration, vapidKey) without any await before pushManager.subscribe().
|
||||||
// Button is disabled until the key is ready (null = not yet loaded).
|
// Button is disabled until the key is ready (null = not yet loaded).
|
||||||
const [vapidKey, setVapidKey] = useState<string | null>(null)
|
const [vapidKey, setVapidKey] = useState<string | null>(null)
|
||||||
|
// NEW-CR-01: Pre-resolve the ServiceWorkerRegistration into state so the tap
|
||||||
|
// handler has ZERO awaits between the user gesture and pushManager.subscribe().
|
||||||
|
// Any await (including navigator.serviceWorker.ready) between the tap and
|
||||||
|
// pushManager.subscribe() breaks the iOS user-gesture requirement.
|
||||||
|
const [swRegistration, setSwRegistration] = useState<ServiceWorkerRegistration | null>(null)
|
||||||
const headingId = useId()
|
const headingId = useId()
|
||||||
|
|
||||||
const { subscribe, permission } = usePushSubscription()
|
const { subscribe, permission } = usePushSubscription()
|
||||||
@@ -103,6 +108,20 @@ export function PushPermissionPrompt({ onClose }: PushPermissionPromptProps) {
|
|||||||
})
|
})
|
||||||
}, [installed, permission, dismissed])
|
}, [installed, permission, dismissed])
|
||||||
|
|
||||||
|
// NEW-CR-01: Pre-resolve the ServiceWorkerRegistration in a useEffect so the
|
||||||
|
// tap handler never has to await navigator.serviceWorker.ready.
|
||||||
|
// navigator.serviceWorker.ready resolves once the SW is active; doing this
|
||||||
|
// eagerly means the result is in state before the user can tap the button.
|
||||||
|
useEffect(() => {
|
||||||
|
if (!installed) return
|
||||||
|
if (permission !== 'default') return
|
||||||
|
if (dismissed) return
|
||||||
|
if (!navigator.serviceWorker) return
|
||||||
|
void navigator.serviceWorker.ready.then((reg) => {
|
||||||
|
setSwRegistration(reg)
|
||||||
|
})
|
||||||
|
}, [installed, permission, dismissed])
|
||||||
|
|
||||||
// Don't render when: not installed, already granted/denied, or dismissed
|
// Don't render when: not installed, already granted/denied, or dismissed
|
||||||
if (!installed) return null
|
if (!installed) return null
|
||||||
if (permission !== 'default') return null
|
if (permission !== 'default') return null
|
||||||
@@ -115,20 +134,21 @@ export function PushPermissionPrompt({ onClose }: PushPermissionPromptProps) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// onClick handler — subscribe() called synchronously (iOS user-gesture requirement).
|
// onClick handler — subscribe() called synchronously (iOS user-gesture requirement).
|
||||||
// CR-04: vapidKey is already resolved from state (pre-fetched in useEffect above).
|
// NEW-CR-01: Both vapidKey AND swRegistration are pre-resolved in state (useEffects above).
|
||||||
// No await before subscribe() in this scope; we pass the key directly.
|
// There is ZERO await between the tap gesture and registration.pushManager.subscribe()
|
||||||
|
// inside subscribe() — the iOS gesture gate is fully satisfied.
|
||||||
function handleEnableClick() {
|
function handleEnableClick() {
|
||||||
if (loading || !vapidKey) return
|
if (loading || !vapidKey || !swRegistration) return
|
||||||
setLoading(true)
|
setLoading(true)
|
||||||
setError(null)
|
setError(null)
|
||||||
|
|
||||||
// Capture the key synchronously before any await — iOS gesture gate requirement.
|
// Capture both synchronously — no await in this scope before subscribe().
|
||||||
const resolvedVapidKey = vapidKey
|
const resolvedVapidKey = vapidKey
|
||||||
|
const resolvedRegistration = swRegistration
|
||||||
|
|
||||||
void (async () => {
|
void (async () => {
|
||||||
try {
|
try {
|
||||||
const registration = await navigator.serviceWorker.ready
|
await subscribe(resolvedRegistration, resolvedVapidKey)
|
||||||
await subscribe(registration, resolvedVapidKey)
|
|
||||||
// On success: close the prompt (permission is now 'granted')
|
// On success: close the prompt (permission is now 'granted')
|
||||||
setLoading(false)
|
setLoading(false)
|
||||||
onClose?.()
|
onClose?.()
|
||||||
@@ -273,10 +293,10 @@ export function PushPermissionPrompt({ onClose }: PushPermissionPromptProps) {
|
|||||||
}}
|
}}
|
||||||
>
|
>
|
||||||
{/* Primary CTA — 48px, accent color */}
|
{/* Primary CTA — 48px, accent color */}
|
||||||
{/* CR-04: disabled until vapidKey is loaded (null = pre-fetch pending) */}
|
{/* NEW-CR-01: disabled until BOTH vapidKey and swRegistration are ready */}
|
||||||
<button
|
<button
|
||||||
onClick={handleEnableClick}
|
onClick={handleEnableClick}
|
||||||
disabled={loading || !vapidKey}
|
disabled={loading || !vapidKey || !swRegistration}
|
||||||
aria-busy={loading}
|
aria-busy={loading}
|
||||||
style={{
|
style={{
|
||||||
background: 'var(--color-member-0, #4A90D9)',
|
background: 'var(--color-member-0, #4A90D9)',
|
||||||
|
|||||||
@@ -54,6 +54,9 @@ export function SettingsSheet({ isOpen, onClose }: SettingsSheetProps) {
|
|||||||
// CR-04: pre-fetch the VAPID key into state so the toggle tap handler can call
|
// CR-04: pre-fetch the VAPID key into state so the toggle tap handler can call
|
||||||
// subscribe(registration, vapidKey) without any network await before pushManager.subscribe().
|
// subscribe(registration, vapidKey) without any network await before pushManager.subscribe().
|
||||||
const [vapidKey, setVapidKey] = useState<string | null>(null)
|
const [vapidKey, setVapidKey] = useState<string | null>(null)
|
||||||
|
// NEW-CR-01: Pre-resolve the ServiceWorkerRegistration into state so the toggle
|
||||||
|
// tap handler has ZERO awaits between the user gesture and pushManager.subscribe().
|
||||||
|
const [swRegistration, setSwRegistration] = useState<ServiceWorkerRegistration | null>(null)
|
||||||
const closeButtonRef = useRef<HTMLButtonElement>(null)
|
const closeButtonRef = useRef<HTMLButtonElement>(null)
|
||||||
|
|
||||||
// Compute initial toggle on/off state per UI-SPEC toggle initial state rule:
|
// Compute initial toggle on/off state per UI-SPEC toggle initial state rule:
|
||||||
@@ -87,6 +90,18 @@ export function SettingsSheet({ isOpen, onClose }: SettingsSheetProps) {
|
|||||||
})
|
})
|
||||||
}, [isOpen, permission])
|
}, [isOpen, permission])
|
||||||
|
|
||||||
|
// NEW-CR-01: Pre-resolve the ServiceWorkerRegistration into state so the toggle
|
||||||
|
// tap handler never needs to await navigator.serviceWorker.ready. Any await
|
||||||
|
// between the tap gesture and pushManager.subscribe() breaks iOS.
|
||||||
|
useEffect(() => {
|
||||||
|
if (!isOpen) return
|
||||||
|
if (permission === 'denied') return
|
||||||
|
if (!navigator.serviceWorker) return
|
||||||
|
void navigator.serviceWorker.ready.then((reg) => {
|
||||||
|
setSwRegistration(reg)
|
||||||
|
})
|
||||||
|
}, [isOpen, permission])
|
||||||
|
|
||||||
if (!isOpen) return null
|
if (!isOpen) return null
|
||||||
|
|
||||||
const handleToggle = async () => {
|
const handleToggle = async () => {
|
||||||
@@ -106,16 +121,15 @@ export function SettingsSheet({ isOpen, onClose }: SettingsSheetProps) {
|
|||||||
} else {
|
} else {
|
||||||
// off → on, permission 'default': needs tap-gated subscribe with OS dialog
|
// off → on, permission 'default': needs tap-gated subscribe with OS dialog
|
||||||
// The toggle click IS the tap gesture — call subscribe() directly here.
|
// The toggle click IS the tap gesture — call subscribe() directly here.
|
||||||
// CR-04: vapidKey was pre-fetched into state (useEffect above); pass it directly
|
// NEW-CR-01: Both vapidKey AND swRegistration are pre-resolved into state
|
||||||
// so there is no await-fetch between the tap and pushManager.subscribe().
|
// (useEffects above). There is ZERO await between the tap and
|
||||||
if (!vapidKey) return // key not ready — this should be rare; button should show spinner
|
// registration.pushManager.subscribe() — iOS gesture gate satisfied.
|
||||||
|
if (!vapidKey || !swRegistration) return // not ready — useEffects still resolving
|
||||||
const resolvedVapidKey = vapidKey
|
const resolvedVapidKey = vapidKey
|
||||||
|
const resolvedRegistration = swRegistration
|
||||||
setIsTogglingOn(true)
|
setIsTogglingOn(true)
|
||||||
try {
|
try {
|
||||||
const registration = await navigator.serviceWorker?.ready
|
await subscribe(resolvedRegistration, resolvedVapidKey)
|
||||||
if (registration) {
|
|
||||||
await subscribe(registration, resolvedVapidKey)
|
|
||||||
}
|
|
||||||
} catch {
|
} catch {
|
||||||
// Permission denied by OS or error — permission state will update reactively
|
// Permission denied by OS or error — permission state will update reactively
|
||||||
} finally {
|
} finally {
|
||||||
|
|||||||
Reference in New Issue
Block a user