fix: address P0 and P1 quality issues with shared utilities

P0 Critical fixes:
- Fix race conditions in useProfile using AbortController pattern
- Add proper cleanup for async operations that outlive unmount

P1 High priority fixes:
- Extract shared Nostr kind constants to src/lib/nostr-kinds.ts
- Re-export isReplaceableKind, isEphemeralKind from nostr-tools
- Create useStable hooks for dependency stabilization
- Remove duplicated kind range logic from BaseEventRenderer and KindRenderer
- Update timeline hooks to use shared useStable utilities

New files:
- src/lib/nostr-kinds.ts: Centralized kind utilities (re-exports nostr-tools)
- src/lib/nostr-kinds.test.ts: Comprehensive tests for kind functions
- src/hooks/useStable.ts: Reusable hooks for dependency stabilization
- ACCESSIBILITY_PLAN.md: Detailed roadmap for WCAG 2.1 AA compliance

This reduces code duplication and improves consistency across the codebase.
This commit is contained in:
Claude
2025-12-21 11:29:42 +00:00
parent b1bc9bccb8
commit 2cfe70c0b7
10 changed files with 825 additions and 87 deletions

View File

@@ -10,14 +10,17 @@ import {
getContentTypeDescription,
} from "@/lib/nostr-schema";
import { CenteredContent } from "./ui/CenteredContent";
// NIP-01 Kind ranges
const REPLACEABLE_START = 10000;
const REPLACEABLE_END = 20000;
const EPHEMERAL_START = 20000;
const EPHEMERAL_END = 30000;
const PARAMETERIZED_REPLACEABLE_START = 30000;
const PARAMETERIZED_REPLACEABLE_END = 40000;
import {
REPLACEABLE_START,
REPLACEABLE_END,
EPHEMERAL_START,
EPHEMERAL_END,
PARAMETERIZED_REPLACEABLE_START,
PARAMETERIZED_REPLACEABLE_END,
isReplaceableKind,
isEphemeralKind,
isParameterizedReplaceableKind,
} from "@/lib/nostr-kinds";
export default function KindRenderer({ kind }: { kind: number }) {
const kindInfo = getKindInfo(kind);
@@ -194,15 +197,9 @@ function getKindCategory(kind: number): string {
if (kind >= 20 && kind <= 39) return "Media & Content";
if (kind >= 40 && kind <= 49) return "Channels";
if (kind >= 1000 && kind <= 9999) return "Application Specific";
if (kind >= REPLACEABLE_START && kind < REPLACEABLE_END)
return "Regular Lists";
if (kind >= EPHEMERAL_START && kind < EPHEMERAL_END)
return "Ephemeral Events";
if (
kind >= PARAMETERIZED_REPLACEABLE_START &&
kind < PARAMETERIZED_REPLACEABLE_END
)
return "Parameterized Replaceable";
if (isReplaceableKind(kind)) return "Regular Lists";
if (isEphemeralKind(kind)) return "Ephemeral Events";
if (isParameterizedReplaceableKind(kind)) return "Parameterized Replaceable";
if (kind >= 40000) return "Custom/Experimental";
return "Other";
}
@@ -211,20 +208,13 @@ function getKindCategory(kind: number): string {
* Determine the replaceability of an event kind
*/
function getEventType(kind: number): string {
if (
kind === kinds.Metadata ||
kind === kinds.Contacts ||
(kind >= REPLACEABLE_START && kind < REPLACEABLE_END)
) {
if (kind === kinds.Metadata || kind === kinds.Contacts || isReplaceableKind(kind)) {
return "Replaceable";
}
if (
kind >= PARAMETERIZED_REPLACEABLE_START &&
kind < PARAMETERIZED_REPLACEABLE_END
) {
if (isParameterizedReplaceableKind(kind)) {
return "Parameterized Replaceable";
}
if (kind >= EPHEMERAL_START && kind < EPHEMERAL_END) {
if (isEphemeralKind(kind)) {
return "Ephemeral";
}
return "Regular";

View File

@@ -2,7 +2,6 @@ import { useState } from "react";
import { NostrEvent } from "@/types/nostr";
import { UserName } from "../UserName";
import { KindBadge } from "@/components/KindBadge";
// import { kinds } from "nostr-tools";
import {
DropdownMenu,
DropdownMenuContent,
@@ -20,17 +19,7 @@ import { nip19 } from "nostr-tools";
import { getTagValue } from "applesauce-core/helpers";
import { EventFooter } from "@/components/EventFooter";
import { cn } from "@/lib/utils";
// import { RichText } from "../RichText";
// import { getEventReply } from "@/lib/nostr-utils";
// import { useNostrEvent } from "@/hooks/useNostrEvent";
// import type { EventPointer, AddressPointer } from "nostr-tools/nip19";
// import { Skeleton } from "@/components/ui/skeleton";
// NIP-01 Kind ranges
const REPLACEABLE_START = 10000;
const REPLACEABLE_END = 20000;
const PARAMETERIZED_REPLACEABLE_START = 30000;
const PARAMETERIZED_REPLACEABLE_END = 40000;
import { isAddressableKind } from "@/lib/nostr-kinds";
/**
* Universal event properties and utilities shared across all kind renderers
@@ -117,13 +106,8 @@ export function EventMenu({ event }: { event: NostrEvent }) {
const openEventDetail = () => {
// For replaceable/parameterized replaceable events, use AddressPointer
const isAddressable =
(event.kind >= REPLACEABLE_START && event.kind < REPLACEABLE_END) ||
(event.kind >= PARAMETERIZED_REPLACEABLE_START &&
event.kind < PARAMETERIZED_REPLACEABLE_END);
let pointer;
if (isAddressable) {
if (isAddressableKind(event.kind)) {
// Find d-tag for identifier
const dTag = getTagValue(event, "d") || "";
pointer = {
@@ -143,12 +127,7 @@ export function EventMenu({ event }: { event: NostrEvent }) {
const copyEventId = () => {
// For replaceable/parameterized replaceable events, encode as naddr
const isAddressable =
(event.kind >= REPLACEABLE_START && event.kind < REPLACEABLE_END) ||
(event.kind >= PARAMETERIZED_REPLACEABLE_START &&
event.kind < PARAMETERIZED_REPLACEABLE_END);
if (isAddressable) {
if (isAddressableKind(event.kind)) {
// Find d-tag for identifier
const dTag = getTagValue(event, "d") || "";
const naddr = nip19.naddrEncode({
@@ -242,15 +221,10 @@ export function ClickableEventTitle({
const handleClick = (e: React.MouseEvent) => {
e.stopPropagation();
// Determine if event is addressable/replaceable
const isAddressable =
(event.kind >= REPLACEABLE_START && event.kind < REPLACEABLE_END) ||
(event.kind >= PARAMETERIZED_REPLACEABLE_START &&
event.kind < PARAMETERIZED_REPLACEABLE_END);
let pointer;
if (isAddressable) {
// For replaceable/parameterized replaceable events, use AddressPointer
if (isAddressableKind(event.kind)) {
// For replaceable/parameterized replaceable events, use AddressPointer
const dTag = getTagValue(event, "d") || "";
pointer = {

View File

@@ -1,8 +1,9 @@
import { useState, useEffect, useMemo } from "react";
import { useState, useEffect } from "react";
import pool from "@/services/relay-pool";
import type { NostrEvent, Filter } from "nostr-tools";
import { useEventStore, useObservableMemo } from "applesauce-react/hooks";
import { isNostrEvent } from "@/lib/type-guards";
import { useStableValue, useStableArray } from "./useStable";
interface UseLiveTimelineOptions {
limit?: number;
@@ -38,12 +39,9 @@ export function useLiveTimeline(
const [error, setError] = useState<Error | null>(null);
const [eoseReceived, setEoseReceived] = useState(false);
// Stabilize filters and relays for dependency array
// Using JSON.stringify and .join() for deep comparison - this is intentional
// eslint-disable-next-line react-hooks/exhaustive-deps
const stableFilters = useMemo(() => filters, [JSON.stringify(filters)]);
// eslint-disable-next-line react-hooks/exhaustive-deps
const stableRelays = useMemo(() => relays, [relays.join(",")]);
// Stabilize filters and relays to prevent unnecessary re-renders
const stableFilters = useStableValue(filters);
const stableRelays = useStableArray(relays);
// 1. Subscription Effect - Fetch data and feed EventStore
useEffect(() => {

View File

@@ -1,19 +1,38 @@
import { useState, useEffect } from "react";
import { useState, useEffect, useRef } from "react";
import { profileLoader } from "@/services/loaders";
import { ProfileContent, getProfileContent } from "applesauce-core/helpers";
import { kinds } from "nostr-tools";
import db from "@/services/db";
/**
* Hook to fetch and cache user profile metadata
*
* Uses AbortController to prevent race conditions when:
* - Component unmounts during async operations
* - Pubkey changes while a fetch is in progress
*
* @param pubkey - The user's public key (hex)
* @returns ProfileContent or undefined if loading/not found
*/
export function useProfile(pubkey?: string): ProfileContent | undefined {
const [profile, setProfile] = useState<ProfileContent | undefined>();
const abortControllerRef = useRef<AbortController | null>(null);
useEffect(() => {
let mounted = true;
if (!pubkey) return;
if (!pubkey) {
setProfile(undefined);
return;
}
// Load from IndexedDB first
// Abort any in-flight requests from previous effect runs
abortControllerRef.current?.abort();
const controller = new AbortController();
abortControllerRef.current = controller;
// Load from IndexedDB first (fast path)
db.profiles.get(pubkey).then((cachedProfile) => {
if (mounted && cachedProfile) {
if (controller.signal.aborted) return;
if (cachedProfile) {
setProfile(cachedProfile);
}
});
@@ -21,6 +40,7 @@ export function useProfile(pubkey?: string): ProfileContent | undefined {
// Fetch from network
const sub = profileLoader({ kind: kinds.Metadata, pubkey }).subscribe({
next: async (fetchedEvent) => {
if (controller.signal.aborted) return;
if (!fetchedEvent || !fetchedEvent.content) return;
// Use applesauce helper for safe profile parsing
@@ -30,24 +50,34 @@ export function useProfile(pubkey?: string): ProfileContent | undefined {
return;
}
// Save to IndexedDB
await db.profiles.put({
// Save to IndexedDB (fire and forget if aborted)
const savePromise = db.profiles.put({
...profileData,
pubkey,
created_at: fetchedEvent.created_at,
});
if (mounted) {
// Only update state if not aborted
if (!controller.signal.aborted) {
setProfile(profileData);
}
// Await save after state update to avoid blocking UI
try {
await savePromise;
} catch (err) {
// Log but don't throw - cache failure shouldn't break the UI
console.error("[useProfile] Failed to cache profile:", err);
}
},
error: (err) => {
if (controller.signal.aborted) return;
console.error("[useProfile] Error fetching profile:", err);
},
});
return () => {
mounted = false;
controller.abort();
sub.unsubscribe();
};
}, [pubkey]);

View File

@@ -3,6 +3,7 @@ import pool from "@/services/relay-pool";
import type { NostrEvent, Filter } from "nostr-tools";
import { useEventStore } from "applesauce-react/hooks";
import { isNostrEvent } from "@/lib/type-guards";
import { useStableValue, useStableArray } from "./useStable";
interface UseReqTimelineOptions {
limit?: number;
@@ -47,12 +48,9 @@ export function useReqTimeline(
);
}, [eventsMap]);
// Stabilize filters and relays for dependency array
// Using JSON.stringify and .join() for deep comparison - this is intentional
// eslint-disable-next-line react-hooks/exhaustive-deps
const stableFilters = useMemo(() => filters, [JSON.stringify(filters)]);
// eslint-disable-next-line react-hooks/exhaustive-deps
const stableRelays = useMemo(() => relays, [relays.join(",")]);
// Stabilize filters and relays to prevent unnecessary re-renders
const stableFilters = useStableValue(filters);
const stableRelays = useStableArray(relays);
useEffect(() => {
if (relays.length === 0) {

62
src/hooks/useStable.ts Normal file
View File

@@ -0,0 +1,62 @@
import { useMemo } from "react";
/**
* Stabilize a value for use in dependency arrays
*
* React's useEffect/useMemo compare dependencies by reference.
* For objects/arrays that are recreated each render but have the same content,
* this causes unnecessary re-runs. This hook memoizes the value based on
* a serialized representation.
*
* @param value - The value to stabilize
* @param serialize - Optional custom serializer (defaults to JSON.stringify)
* @returns The memoized value
*
* @example
* ```typescript
* // Instead of: useMemo(() => filters, [JSON.stringify(filters)])
* const stableFilters = useStableValue(filters);
* ```
*/
export function useStableValue<T>(
value: T,
serialize?: (v: T) => string
): T {
const serialized = serialize?.(value) ?? JSON.stringify(value);
// eslint-disable-next-line react-hooks/exhaustive-deps
return useMemo(() => value, [serialized]);
}
/**
* Stabilize a string array for use in dependency arrays
*
* Optimized version of useStableValue for string arrays.
* Uses join(",") instead of JSON.stringify for better performance.
*
* @param arr - The array to stabilize
* @returns The memoized array
*
* @example
* ```typescript
* // Instead of: useMemo(() => relays, [relays.join(",")])
* const stableRelays = useStableArray(relays);
* ```
*/
export function useStableArray<T extends string>(arr: T[]): T[] {
// eslint-disable-next-line react-hooks/exhaustive-deps
return useMemo(() => arr, [arr.join(",")]);
}
/**
* Stabilize a Nostr filter or array of filters
*
* Specialized stabilizer for Nostr filters which are commonly
* recreated on each render.
*
* @param filters - Single filter or array of filters
* @returns The memoized filter(s)
*/
export function useStableFilters<T>(filters: T): T {
// eslint-disable-next-line react-hooks/exhaustive-deps
return useMemo(() => filters, [JSON.stringify(filters)]);
}

View File

@@ -1,9 +1,10 @@
import { useState, useEffect, useMemo } from "react";
import { useState, useEffect } from "react";
import type { NostrEvent, Filter } from "nostr-tools";
import { useEventStore, useObservableMemo } from "applesauce-react/hooks";
import { createTimelineLoader } from "@/services/loaders";
import pool from "@/services/relay-pool";
import { AGGREGATOR_RELAYS } from "@/services/loaders";
import { useStableValue, useStableArray } from "./useStable";
interface UseTimelineOptions {
limit?: number;
@@ -35,12 +36,9 @@ export function useTimeline(
const [loading, setLoading] = useState(false);
const [error, setError] = useState<Error | null>(null);
// Stabilize filters and relays for dependency array
// Using JSON.stringify and .join() for deep comparison - this is intentional
// eslint-disable-next-line react-hooks/exhaustive-deps
const stableFilters = useMemo(() => filters, [JSON.stringify(filters)]);
// eslint-disable-next-line react-hooks/exhaustive-deps
const stableRelays = useMemo(() => relays, [relays.join(",")]);
// Stabilize filters and relays to prevent unnecessary re-renders
const stableFilters = useStableValue(filters);
const stableRelays = useStableArray(relays);
// Load events into store
useEffect(() => {

162
src/lib/nostr-kinds.test.ts Normal file
View File

@@ -0,0 +1,162 @@
import { describe, expect, it } from "vitest";
import {
isRegularKind,
isReplaceableKind,
isEphemeralKind,
isParameterizedReplaceableKind,
isAddressableKind,
getKindCategory,
REGULAR_START,
REGULAR_END,
REPLACEABLE_START,
REPLACEABLE_END,
EPHEMERAL_START,
EPHEMERAL_END,
PARAMETERIZED_REPLACEABLE_START,
PARAMETERIZED_REPLACEABLE_END,
} from "./nostr-kinds";
describe("nostr-kinds constants", () => {
it("should have correct NIP-01 boundaries", () => {
expect(REGULAR_START).toBe(0);
expect(REGULAR_END).toBe(10000);
expect(REPLACEABLE_START).toBe(10000);
expect(REPLACEABLE_END).toBe(20000);
expect(EPHEMERAL_START).toBe(20000);
expect(EPHEMERAL_END).toBe(30000);
expect(PARAMETERIZED_REPLACEABLE_START).toBe(30000);
expect(PARAMETERIZED_REPLACEABLE_END).toBe(40000);
});
});
describe("isRegularKind (from nostr-tools)", () => {
it("should return true for regular kinds", () => {
expect(isRegularKind(1)).toBe(true); // Text note
expect(isRegularKind(7)).toBe(true); // Reaction
expect(isRegularKind(9999)).toBe(true);
});
it("should return false for special replaceable kinds 0 and 3", () => {
// nostr-tools treats 0 (Metadata) and 3 (Contacts) as replaceable, not regular
expect(isRegularKind(0)).toBe(false);
expect(isRegularKind(3)).toBe(false);
});
it("should return false for non-regular kinds", () => {
expect(isRegularKind(10000)).toBe(false);
expect(isRegularKind(20000)).toBe(false);
expect(isRegularKind(30000)).toBe(false);
});
});
describe("isReplaceableKind (from nostr-tools)", () => {
it("should return true for replaceable kinds (0, 3, 10000-19999)", () => {
// nostr-tools includes 0 (Metadata) and 3 (Contacts) as replaceable
expect(isReplaceableKind(0)).toBe(true); // Metadata
expect(isReplaceableKind(3)).toBe(true); // Contacts
expect(isReplaceableKind(10000)).toBe(true);
expect(isReplaceableKind(10002)).toBe(true); // Relay list
expect(isReplaceableKind(19999)).toBe(true);
});
it("should return false for non-replaceable kinds", () => {
expect(isReplaceableKind(1)).toBe(false);
expect(isReplaceableKind(7)).toBe(false);
expect(isReplaceableKind(20000)).toBe(false);
expect(isReplaceableKind(30000)).toBe(false);
});
});
describe("isEphemeralKind (from nostr-tools)", () => {
it("should return true for ephemeral kinds (20000-29999)", () => {
expect(isEphemeralKind(20000)).toBe(true);
expect(isEphemeralKind(22242)).toBe(true); // Auth
expect(isEphemeralKind(29999)).toBe(true);
});
it("should return false for non-ephemeral kinds", () => {
expect(isEphemeralKind(0)).toBe(false);
expect(isEphemeralKind(10000)).toBe(false);
expect(isEphemeralKind(19999)).toBe(false);
expect(isEphemeralKind(30000)).toBe(false);
});
});
describe("isParameterizedReplaceableKind", () => {
it("should return true for parameterized replaceable kinds (30000-39999)", () => {
expect(isParameterizedReplaceableKind(30000)).toBe(true);
expect(isParameterizedReplaceableKind(30023)).toBe(true); // Long-form content
expect(isParameterizedReplaceableKind(30311)).toBe(true); // Live activity
expect(isParameterizedReplaceableKind(39999)).toBe(true);
});
it("should return false for non-parameterized replaceable kinds", () => {
expect(isParameterizedReplaceableKind(0)).toBe(false);
expect(isParameterizedReplaceableKind(1)).toBe(false);
expect(isParameterizedReplaceableKind(10002)).toBe(false);
expect(isParameterizedReplaceableKind(20000)).toBe(false);
expect(isParameterizedReplaceableKind(40000)).toBe(false);
});
});
describe("isAddressableKind", () => {
it("should return true for special replaceable kinds 0 and 3", () => {
expect(isAddressableKind(0)).toBe(true); // Metadata
expect(isAddressableKind(3)).toBe(true); // Contacts
});
it("should return true for replaceable kinds (10000-19999)", () => {
expect(isAddressableKind(10000)).toBe(true);
expect(isAddressableKind(10002)).toBe(true);
expect(isAddressableKind(19999)).toBe(true);
});
it("should return true for parameterized replaceable kinds", () => {
expect(isAddressableKind(30000)).toBe(true);
expect(isAddressableKind(30023)).toBe(true);
expect(isAddressableKind(39999)).toBe(true);
});
it("should return false for regular kinds", () => {
expect(isAddressableKind(1)).toBe(false);
expect(isAddressableKind(7)).toBe(false);
expect(isAddressableKind(9999)).toBe(false);
});
it("should return false for ephemeral kinds", () => {
expect(isAddressableKind(20000)).toBe(false);
expect(isAddressableKind(22242)).toBe(false);
expect(isAddressableKind(29999)).toBe(false);
});
});
describe("getKindCategory", () => {
it("should categorize special replaceable kinds 0 and 3", () => {
expect(getKindCategory(0)).toBe("replaceable");
expect(getKindCategory(3)).toBe("replaceable");
});
it("should categorize regular kinds", () => {
expect(getKindCategory(1)).toBe("regular");
expect(getKindCategory(7)).toBe("regular");
expect(getKindCategory(9999)).toBe("regular");
});
it("should categorize replaceable kinds", () => {
expect(getKindCategory(10000)).toBe("replaceable");
expect(getKindCategory(10002)).toBe("replaceable");
expect(getKindCategory(19999)).toBe("replaceable");
});
it("should categorize ephemeral kinds", () => {
expect(getKindCategory(20000)).toBe("ephemeral");
expect(getKindCategory(22242)).toBe("ephemeral");
expect(getKindCategory(29999)).toBe("ephemeral");
});
it("should categorize parameterized replaceable kinds", () => {
expect(getKindCategory(30000)).toBe("parameterized_replaceable");
expect(getKindCategory(30023)).toBe("parameterized_replaceable");
expect(getKindCategory(39999)).toBe("parameterized_replaceable");
});
});

70
src/lib/nostr-kinds.ts Normal file
View File

@@ -0,0 +1,70 @@
/**
* Nostr event kind range constants and utilities
*
* Re-exports from nostr-tools where available, with additional
* Grimoire-specific utilities.
*
* Based on NIP-01 specification:
* - Regular kinds: 0-9999 (non-replaceable, except 0 and 3)
* - Replaceable kinds: 0, 3, 10000-19999 (replaced by newer)
* - Ephemeral kinds: 20000-29999 (not stored)
* - Parameterized replaceable: 30000-39999 (replaced by kind+pubkey+d-tag)
*/
// Re-export from nostr-tools for consistency
export {
isRegularKind,
isReplaceableKind,
isEphemeralKind,
classifyKind,
} from "nostr-tools/kinds";
// Import for internal use
import {
isReplaceableKind as _isReplaceableKind,
isEphemeralKind as _isEphemeralKind,
} from "nostr-tools/kinds";
// Kind range boundaries (NIP-01) - exported for display purposes only
export const REGULAR_START = 0;
export const REGULAR_END = 10000;
export const REPLACEABLE_START = 10000;
export const REPLACEABLE_END = 20000;
export const EPHEMERAL_START = 20000;
export const EPHEMERAL_END = 30000;
export const PARAMETERIZED_REPLACEABLE_START = 30000;
export const PARAMETERIZED_REPLACEABLE_END = 40000;
/**
* Check if a kind is parameterized replaceable (NIP-01)
* Kinds 30000-39999 are replaced by newer events from same pubkey with same d-tag
*
* Note: nostr-tools calls this "addressable" but we use "parameterized replaceable"
* for consistency with NIP-01 terminology
*/
export function isParameterizedReplaceableKind(kind: number): boolean {
return kind >= PARAMETERIZED_REPLACEABLE_START && kind < PARAMETERIZED_REPLACEABLE_END;
}
/**
* Check if a kind should use naddr/AddressPointer instead of nevent/EventPointer
*
* This includes both:
* - Replaceable kinds (0, 3, 10000-19999) - identified by pubkey+kind
* - Parameterized replaceable kinds (30000-39999) - identified by pubkey+kind+d-tag
*
* Use this to determine how to encode event references (naddr vs nevent)
*/
export function isAddressableKind(kind: number): boolean {
return _isReplaceableKind(kind) || isParameterizedReplaceableKind(kind);
}
/**
* Get the category of a kind for display purposes
*/
export function getKindCategory(kind: number): 'regular' | 'replaceable' | 'ephemeral' | 'parameterized_replaceable' {
if (_isReplaceableKind(kind)) return 'replaceable';
if (_isEphemeralKind(kind)) return 'ephemeral';
if (isParameterizedReplaceableKind(kind)) return 'parameterized_replaceable';
return 'regular';
}