diff --git a/AGENTS.md b/AGENTS.md index 8e65aa2..ce97a2a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -234,6 +234,31 @@ session with **no other memory of this project**. So: `isInteractiveSigner` is true, so merely opening the notification bell (which marks read) no longer pops Amber twice (encrypt + sign). Interactive signers only prompt for real signed actions. + - ⚠️ **Don't make read-state mutators (or any hook they flow from) unstable + (fixed 2026-09-27).** `/newsletter` was stuck in an **infinite render loop** + (~100% CPU, React "Maximum update depth exceeded") because: `useSyncedAppData` + returned a **fresh object literal every render** → `useReadState.apply` + (dep `synced`) got a new identity → `markNewsletterRead`/`markAllRead` changed + every render → page effects depending on them re-ran after every render → + the mark set a **new state object** → re-render → loop. Same latent loop in + `NotificationBell` **while open**. Fixes (keep them): `useLocalStorage`'s + setter is **referentially stable** (ref + functional update), `useSyncedAppData` + returns a **memoized** object, `useReadState.apply` reads `synced`/`interactive` + from **refs** so it's stable, and `markAllRead`/`markNewsletterRead` are + **idempotent** (return the SAME object when the marker already covers `now`, so + React bails). Page effects that mark read on open use a **mount-once ref guard** + (`NewsletterPage`). Diagnose with CDP `Performance.getMetrics` `TaskDuration` + over ~4 s idle (a looping route burns ~4 s; a healthy one ~30–100 ms). + - **Newsletter publish/read relay rule:** issues (kind 30023, + `t=skatespotsnews`) and the subscription marker (kind 30078, + `d=skatespots:newsletter:subscription`) are **read via `useSkateRelays` + (`querySkateContent`)** — i.e. the community relay + the reader's relays, and + for logged-out visitors **only the community relay**. So they must be + **published to `getWriteRelays()`** (community relay + the author's relays), + NOT the default pool (`useNostrPublish`). The composer / backfill / subscribe + hook do this and fail loudly if every relay rejects. `useSpotChat` is the + reference pattern. (Before the fix, an admin whose own NIP-65 lacked + `goskatespots` would publish issues nobody could find.) - **Nav parity (fixed 2026-09-27):** the desktop header is `max-w-[1700px]` (wider than the page `container`'s 1400px) and nav items are `px-3` + `whitespace-nowrap`; the nav keeps `min-w-0 overflow-x-auto scrollbar-hide`, so diff --git a/src/components/NewsletterBackfillPanel.tsx b/src/components/NewsletterBackfillPanel.tsx index aa8b028..78ffdb6 100644 --- a/src/components/NewsletterBackfillPanel.tsx +++ b/src/components/NewsletterBackfillPanel.tsx @@ -1,9 +1,10 @@ import { useState } from 'react'; +import { useNostr } from '@nostrify/react'; import { useQueryClient } from '@tanstack/react-query'; import { Archive, Loader2, Check, AlertTriangle } from 'lucide-react'; import { Button } from '@/components/ui/button'; import { useCurrentUser } from '@/hooks/useCurrentUser'; -import { useNostrPublish } from '@/hooks/useNostrPublish'; +import { useSkateRelays } from '@/hooks/useSkateRelays'; import { useNewsletterIssues } from '@/hooks/useNewsletterIssues'; import { useToast } from '@/hooks/useToast'; import { isAdminPubkey } from '@/lib/admins'; @@ -18,7 +19,8 @@ import { buildIssueArticle, issueSlug, type StaticIssueLike } from '@/lib/newsle */ export function NewsletterBackfillPanel({ issues }: { issues?: StaticIssueLike[] }) { const { user } = useCurrentUser(); - const { mutateAsync: publish } = useNostrPublish(); + const { nostr } = useNostr(); + const { getWriteRelays } = useSkateRelays(); const { data: liveIssues } = useNewsletterIssues(); const { toast } = useToast(); const queryClient = useQueryClient(); @@ -40,7 +42,22 @@ export function NewsletterBackfillPanel({ issues }: { issues?: StaticIssueLike[] const ordered = [...missing].sort((a, b) => a.number - b.number); for (const issue of ordered) { try { - await publish(buildIssueArticle(issue)); + if (!user) throw new Error('Not logged in'); + const template = buildIssueArticle(issue); + const event = await user.signer.signEvent({ + kind: template.kind, + content: template.content, + tags: template.tags, + created_at: Math.floor(Date.now() / 1000), + }); + // Same reason as the composer: publish to the COMMUNITY relay (+ the + // admin's relays) so readers — logged in or not — actually find it. + const results = await Promise.allSettled( + getWriteRelays().map((url) => + nostr.relay(url).event(event, { signal: AbortSignal.timeout(8000) }), + ), + ); + if (results.every((r) => r.status === 'rejected')) throw new Error('all relays failed'); published++; } catch { failures.push(issue.number); diff --git a/src/components/NewsletterComposer.tsx b/src/components/NewsletterComposer.tsx index 0d88996..6f9000e 100644 --- a/src/components/NewsletterComposer.tsx +++ b/src/components/NewsletterComposer.tsx @@ -1,4 +1,5 @@ import { useState } from 'react'; +import { useNostr } from '@nostrify/react'; import { useQueryClient } from '@tanstack/react-query'; import { PenLine, Send, Loader2, Sparkles } from 'lucide-react'; import { @@ -14,7 +15,7 @@ import { Input } from '@/components/ui/input'; import { Textarea } from '@/components/ui/textarea'; import { Label } from '@/components/ui/label'; import { useCurrentUser } from '@/hooks/useCurrentUser'; -import { useNostrPublish } from '@/hooks/useNostrPublish'; +import { useSkateRelays } from '@/hooks/useSkateRelays'; import { useToast } from '@/hooks/useToast'; import { isAdminPubkey } from '@/lib/admins'; import { NEWSLETTER_KIND, NEWSLETTER_TAG } from '@/lib/types'; @@ -35,10 +36,12 @@ function slugify(input: string): string { */ export function NewsletterComposer() { const { user } = useCurrentUser(); - const { mutateAsync: publish, isPending } = useNostrPublish(); + const { nostr } = useNostr(); + const { getWriteRelays } = useSkateRelays(); const { toast } = useToast(); const queryClient = useQueryClient(); + const [isPending, setIsPending] = useState(false); const [open, setOpen] = useState(false); const [title, setTitle] = useState(''); const [slug, setSlug] = useState(''); @@ -55,10 +58,10 @@ export function NewsletterComposer() { // `d` and the newer one would silently OVERWRITE the older. So an auto-slug // gets a short date suffix to make each published issue its own coordinate. const manualSlug = slug.trim(); - const canPublish = title.trim().length >= 2 && body.trim().length >= 10 && (!!manualSlug || title.trim().length >= 2); + const canPublish = title.trim().length >= 2 && body.trim().length >= 10; const handlePublish = async () => { - if (!canPublish) return; + if (!canPublish || !user) return; const now = Math.floor(Date.now() / 1000); // YYYYMMDD suffix (UTC) keeps auto-slugs readable and unique per day; a // second same-title issue on the same day also gets a time component. @@ -66,10 +69,12 @@ export function NewsletterComposer() { const dateSuffix = `${d.getUTCFullYear()}${String(d.getUTCMonth() + 1).padStart(2, '0')}${String(d.getUTCDate()).padStart(2, '0')}`; const autoSlug = `${slugify(title) || 'issue'}-${dateSuffix}-${now.toString(36).slice(-4)}`; const effectiveSlug = manualSlug || autoSlug; + setIsPending(true); try { - await publish({ + const event = await user.signer.signEvent({ kind: NEWSLETTER_KIND, content: body, + created_at: now, tags: [ ['d', effectiveSlug], ['title', title.trim()], @@ -83,6 +88,18 @@ export function NewsletterComposer() { ['alt', `SkateSpots newsletter issue: ${title.trim()}`], ], }); + // Publish to the COMMUNITY relay (+ the admin's own relays), NOT just the + // default pool. Readers — including logged-out visitors — only query the + // community relay, so an issue written to the admin's personal relays + // alone would be invisible to almost everyone. + const results = await Promise.allSettled( + getWriteRelays().map((url) => + nostr.relay(url).event(event, { signal: AbortSignal.timeout(8000) }), + ), + ); + if (results.every((r) => r.status === 'rejected')) { + throw new Error('Could not reach any relay — check your connection and try again.'); + } toast({ title: 'Issue published! 🛹', description: 'Subscribers will see it in their notification bell.', @@ -100,6 +117,8 @@ export function NewsletterComposer() { description: err instanceof Error ? err.message : 'Please try again.', variant: 'destructive', }); + } finally { + setIsPending(false); } }; diff --git a/src/components/NewsletterOverlay.tsx b/src/components/NewsletterOverlay.tsx index fd4908b..f1c80f7 100644 --- a/src/components/NewsletterOverlay.tsx +++ b/src/components/NewsletterOverlay.tsx @@ -43,11 +43,13 @@ function formatIssueNumber(n: number): string { function LatestDropPill() { const location = useLocation(); const latest = NEWSLETTER_ISSUES[0]; - const age = daysSinceIssue(latest.date); const [dismissed, setDismissed] = useLocalStorage( 'newsletter-pill-dismissed-issue', 0 ); + // Guard: no archive yet ⇒ nothing to promote. + if (!latest) return null; + const age = daysSinceIssue(latest.date); // Hide on newsletter page, map page (to avoid conflicts with map UI), if too old, or if already dismissed const isMapPage = location.pathname === '/' || location.pathname === '/map'; @@ -104,6 +106,7 @@ function MondayAdminReminder() { !isAdmin || !isMonday() || alreadyDismissedToday || + NEWSLETTER_ISSUES.length === 0 || location.pathname === '/newsletter' ) { return null; diff --git a/src/hooks/useLocalStorage.ts b/src/hooks/useLocalStorage.ts index 73c2959..15b18fc 100644 --- a/src/hooks/useLocalStorage.ts +++ b/src/hooks/useLocalStorage.ts @@ -1,7 +1,14 @@ -import { useState, useEffect } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; /** - * Generic hook for managing localStorage state + * Generic hook for managing localStorage state. + * + * The setter is **referentially stable** (and safe against stale closures): it + * keeps the latest value in a ref and updates via a functional `setState`. This + * matters because several callers put the setter (directly or through a chain of + * callbacks) in an effect's dependency array — an unstable identity there causes + * the effect to re-run every render, and if it also sets state that is an + * infinite render loop. */ export function useLocalStorage( key: string, @@ -11,8 +18,18 @@ export function useLocalStorage( deserialize: (value: string) => T; } ) { - const serialize = serializer?.serialize || JSON.stringify; - const deserialize = serializer?.deserialize || JSON.parse; + // Keep the serializer in a ref so `serialize`/`deserialize` stay stable even + // when callers pass an inline object literal on every render. + const serializerRef = useRef(serializer); + serializerRef.current = serializer; + const serialize = useCallback( + (value: T) => (serializerRef.current?.serialize ?? JSON.stringify)(value), + [], + ); + const deserialize = useCallback( + (value: string) => (serializerRef.current?.deserialize ?? JSON.parse)(value), + [], + ); const [state, setState] = useState(() => { try { @@ -24,15 +41,24 @@ export function useLocalStorage( } }); - const setValue = (value: T | ((prev: T) => T)) => { - try { - const valueToStore = value instanceof Function ? value(state) : value; - setState(valueToStore); - localStorage.setItem(key, serialize(valueToStore)); - } catch (error) { - console.warn(`Failed to save ${key} to localStorage:`, error); - } - }; + // Mirror the latest state so the stable setter can support functional updates + // without a stale closure. + const stateRef = useRef(state); + stateRef.current = state; + + const setValue = useCallback( + (value: T | ((prev: T) => T)) => { + try { + const valueToStore = value instanceof Function ? value(stateRef.current) : value; + stateRef.current = valueToStore; + setState(valueToStore); + localStorage.setItem(key, serialize(valueToStore)); + } catch (error) { + console.warn(`Failed to save ${key} to localStorage:`, error); + } + }, + [key, serialize], + ); // Sync with localStorage changes from other tabs useEffect(() => { @@ -51,4 +77,4 @@ export function useLocalStorage( }, [key, deserialize]); return [state, setValue] as const; -} \ No newline at end of file +} diff --git a/src/hooks/useNewsletterSubscription.ts b/src/hooks/useNewsletterSubscription.ts index ad93083..fd711c2 100644 --- a/src/hooks/useNewsletterSubscription.ts +++ b/src/hooks/useNewsletterSubscription.ts @@ -1,6 +1,7 @@ import { useNostr } from '@nostrify/react'; import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; import { useCurrentUser } from '@/hooks/useCurrentUser'; +import { useSkateRelays } from '@/hooks/useSkateRelays'; import { APP_DATA_KIND, NEWSLETTER_SUB_D } from '@/lib/types'; /** @@ -23,6 +24,7 @@ function subscriptionKey(pubkey: string | undefined) { export function useNewsletterSubscription() { const { nostr } = useNostr(); const { user } = useCurrentUser(); + const { getWriteRelays } = useSkateRelays(); const queryClient = useQueryClient(); const query = useQuery({ @@ -64,7 +66,17 @@ export function useNewsletterSubscription() { ['alt', 'SkateSpots newsletter subscription'], ], }); - await nostr.event(event, { signal: AbortSignal.timeout(10000) }); + // Publish to the COMMUNITY relay (+ the user's own relays), not just the + // default pool: the public subscriber tally reads the community relay, so + // a marker written only to personal relays would never be counted. + const results = await Promise.allSettled( + getWriteRelays().map((url) => + nostr.relay(url).event(event, { signal: AbortSignal.timeout(10000) }), + ), + ); + if (results.every((r) => r.status === 'rejected')) { + throw new Error('Could not reach any relay — check your connection and try again.'); + } return subscribe; }, onError: () => { diff --git a/src/hooks/useReadState.ts b/src/hooks/useReadState.ts index 6fc5900..95df110 100644 --- a/src/hooks/useReadState.ts +++ b/src/hooks/useReadState.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useMemo } from 'react'; +import { useCallback, useEffect, useMemo, useRef } from 'react'; import { useNostrLogin } from '@nostrify/react/login'; import { useLocalStorage } from '@/hooks/useLocalStorage'; import { useCurrentUser } from '@/hooks/useCurrentUser'; @@ -117,6 +117,17 @@ export function useReadState() { }); }, [remote, setLocal]); + // Hold the synced engine + interactive flag in refs so `apply` can be a + // STABLE callback. Depending on them directly made `apply` (and every mark* + // derived from it) change identity on every render, which made effects such as + // NewsletterPage's / the notification bell's run after every render and — + // because marking read set a new state object — loop until React aborted with + // "Maximum update depth exceeded". + const syncedRef = useRef(synced); + syncedRef.current = synced; + const interactiveRef = useRef(interactive); + interactiveRef.current = interactive; + /** Apply a patch to BOTH the instant local blob and the synced mirror. */ const apply = useCallback( (producePatch: (current: ReadState) => ReadState) => { @@ -124,10 +135,10 @@ export function useReadState() { // Interactive signer: local-only (see `interactive` above) — publishing // here would pop Amber just for reading. Otherwise mirror to the synced // event (no-op for anon; debounced + merged inside). - if (interactive) return; - void synced.patch((current) => producePatch(current)); + if (interactiveRef.current) return; + void syncedRef.current.patch((current) => producePatch(current)); }, - [setLocal, synced, interactive], + [setLocal], ); return { state: local, apply }; @@ -143,7 +154,14 @@ export function useNotificationsRead() { const markAllRead = useCallback(() => { const now = Math.floor(Date.now() / 1000); - apply((s) => ({ ...s, notifications: now, notificationsSeen: now })); + // Idempotent: return the SAME object when nothing actually moves, so React + // bails out instead of re-rendering (and re-running any effect that calls + // this). A fresh object every call was the other half of the render loop. + apply((s) => + s.notifications >= now && s.notificationsSeen >= now + ? s + : { ...s, notifications: now, notificationsSeen: now }, + ); }, [apply]); return { lastRead: state.notifications, seenAt: state.notificationsSeen, markAllRead }; @@ -155,7 +173,8 @@ export function useNewsletterRead() { const markNewsletterRead = useCallback(() => { const now = Math.floor(Date.now() / 1000); - apply((s) => ({ ...s, newsletter: now })); + // Idempotent (see markAllRead) — bail with the same object if already marked. + apply((s) => (s.newsletter >= now ? s : { ...s, newsletter: now })); }, [apply]); return { lastRead: state.newsletter, markNewsletterRead }; diff --git a/src/hooks/useSyncedAppData.ts b/src/hooks/useSyncedAppData.ts index 99fac6f..fa7910a 100644 --- a/src/hooks/useSyncedAppData.ts +++ b/src/hooks/useSyncedAppData.ts @@ -212,15 +212,22 @@ export function useSyncedAppData(options: UseSyncedAppDataOptions) { }; }, [publishNow]); - return { - /** Decrypted synced object, or `null` while loading / when unavailable. */ - data: (data ?? null) as T | null, - isLoading, - /** True when the user can actually sync (logged in + NIP-44). */ - canSync, - /** Merge a patch in and schedule a debounced, clobber-safe publish. */ - patch, - /** Force an immediate publish of any queued write. */ - flush: publishNow, - }; + // Memoize the returned object: it feeds `useCallback`/`useEffect` deps in + // consumers (e.g. useReadState → mark* → page effects). A fresh object literal + // every render made those identities churn, which turned "mark read on mount" + // effects into an infinite render loop. + return useMemo( + () => ({ + /** Decrypted synced object, or `null` while loading / when unavailable. */ + data: (data ?? null) as T | null, + isLoading, + /** True when the user can actually sync (logged in + NIP-44). */ + canSync, + /** Merge a patch in and schedule a debounced, clobber-safe publish. */ + patch, + /** Force an immediate publish of any queued write. */ + flush: publishNow, + }), + [data, isLoading, canSync, patch, publishNow], + ); } diff --git a/src/pages/NewsletterPage.tsx b/src/pages/NewsletterPage.tsx index f375efc..962270d 100644 --- a/src/pages/NewsletterPage.tsx +++ b/src/pages/NewsletterPage.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; import { useSeoMeta } from '@unhead/react'; import { nip19 } from 'nostr-tools'; import { ImageWithFallback } from '@/components/ImageWithFallback'; @@ -611,9 +611,15 @@ export default function NewsletterPage() { description: 'Weekly drops from the SkateSpots community. App updates, new spots, events and the people keeping skateboarding alive.', }); - // Opening the newsletter clears the "News" unread badge in the bell. + // Opening the newsletter clears the "News" unread badge in the bell — once + // per visit. The ref guard means a changing callback identity can never turn + // this into a render loop (it did: `markNewsletterRead` used to change on + // every render, so the effect re-ran forever: "Maximum update depth exceeded"). const { markNewsletterRead } = useNewsletterRead(); + const markedReadRef = useRef(false); useEffect(() => { + if (markedReadRef.current) return; + markedReadRef.current = true; markNewsletterRead(); }, [markNewsletterRead]);