fix(newsletter): infinite render loop + issues published to the wrong relays

Audit of the newsletter section found two real bugs.

1) /newsletter rendered in an infinite loop (100% CPU, React 'Maximum update
   depth exceeded'). Chain: useSyncedAppData returned a fresh object each render
   -> useReadState.apply (dep: synced) changed identity -> markNewsletterRead /
   markAllRead changed every render -> the page effect (deps include it) re-ran
   after every render -> the mark set a NEW state object -> loop. The
   notification bell looped the same way while open.
   - useLocalStorage: setter is now referentially stable (ref + functional set).
   - useSyncedAppData: returned object is memoized.
   - useReadState: apply reads synced/interactive from refs (stable); marks are
     idempotent (same object when already marked) so React bails.
   - NewsletterPage marks read once per visit (mount-once ref guard).

2) Issues + subscription markers were published via the default pool (the
   author's own NIP-65 relays) but READ via useSkateRelays (community relay +
   reader's relays; logged-out = community relay only), so an admin without
   goskatespots in their list would publish drops nobody could see. Composer,
   backfill panel and the subscribe hook now publish to getWriteRelays() and
   fail loudly if every relay rejects (useSpotChat pattern). NIP.md untouched
   (no schema change).

Also: guard the overlay pill/reminder against an empty archive, and drop a
redundant canPublish clause.
This commit is contained in:
SkateSpots 2026-09-27 17:40:02 +07:00
parent 6a442c394f
commit e98c996945
9 changed files with 177 additions and 43 deletions

View file

@ -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

View file

@ -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);

View file

@ -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);
}
};

View file

@ -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<number>(
'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;

View file

@ -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<T>(
key: string,
@ -11,8 +18,18 @@ export function useLocalStorage<T>(
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<T>(() => {
try {
@ -24,15 +41,24 @@ export function useLocalStorage<T>(
}
});
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<T>(
}, [key, deserialize]);
return [state, setValue] as const;
}
}

View file

@ -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: () => {

View file

@ -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 };

View file

@ -212,15 +212,22 @@ export function useSyncedAppData<T>(options: UseSyncedAppDataOptions<T>) {
};
}, [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],
);
}

View file

@ -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]);