Skip to content

Commit f201fe0

Browse files
dastratakosclaude
andcommitted
fix(PLA-106): move localStorage write out of setState updater; memoize derived
setState updaters must be pure — React may invoke them more than once under Strict Mode and concurrent rendering. Move writeState to the persist call site in the event handler, with stateRef mirroring React state so the next mutation reads the freshly committed value synchronously. Memoize sortedChapters in ChaptersView and filePaths in ChapterCard so they aren't recomputed on unrelated re-renders (e.g., toggling a checkbox). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent af72bb6 commit f201fe0

2 files changed

Lines changed: 23 additions & 10 deletions

File tree

web/src/App.tsx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import type { Chapter, ChaptersFile } from "@/lib/chapters-types";
33
import { type UseViewStateApi, useViewState } from "@/lib/use-view-state";
44
import { cn } from "@/lib/utils";
55
import { Circle, CircleCheck } from "lucide-react";
6-
import { useEffect, useState } from "react";
6+
import { useEffect, useMemo, useState } from "react";
77

88
type FetchState =
99
| { status: "loading" }
@@ -62,7 +62,10 @@ export function App() {
6262

6363
function ChaptersView({ file }: { file: ChaptersFile }) {
6464
const view = useViewState(file.scope.headSha);
65-
const sortedChapters = [...file.chapters].sort((a, b) => a.order - b.order);
65+
const sortedChapters = useMemo(
66+
() => [...file.chapters].sort((a, b) => a.order - b.order),
67+
[file.chapters],
68+
);
6669

6770
return (
6871
<div className="min-h-screen bg-background p-6 text-foreground">
@@ -104,7 +107,7 @@ function uniqueFilePaths(chapter: Chapter): string[] {
104107

105108
function ChapterCard({ chapter, index, view }: ChapterCardProps) {
106109
const isViewed = view.isChapterViewed(chapter.id);
107-
const filePaths = uniqueFilePaths(chapter);
110+
const filePaths = useMemo(() => uniqueFilePaths(chapter), [chapter]);
108111

109112
return (
110113
<li

web/src/lib/use-view-state.ts

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,14 @@ function withRemoved(set: Set<string>, id: string): Set<string> {
7777
export function useViewState(storageKey: string): UseViewStateApi {
7878
const [state, setState] = useState<ViewState>(() => readState(storageKey));
7979

80+
// Mirror state in a ref so persist can compute the next value and write to
81+
// localStorage in the event handler, not inside a setState updater. State
82+
// updaters must be pure (https://react.dev/learn/keeping-components-pure),
83+
// and React may invoke them more than once under Strict Mode / concurrent
84+
// rendering — keeping writeState out of them avoids the redundant writes.
85+
const stateRef = useRef(state);
86+
stateRef.current = state;
87+
8088
// Re-hydrate from localStorage when the storage key changes. Adjusting state
8189
// during render (https://react.dev/learn/you-might-not-need-an-effect) keeps
8290
// the new render and any persist call against it in sync — without this,
@@ -85,17 +93,19 @@ export function useViewState(storageKey: string): UseViewStateApi {
8593
const lastKeyRef = useRef(storageKey);
8694
if (lastKeyRef.current !== storageKey) {
8795
lastKeyRef.current = storageKey;
88-
setState(readState(storageKey));
96+
const fresh = readState(storageKey);
97+
stateRef.current = fresh;
98+
setState(fresh);
8999
}
90100

91101
const persist = useCallback(
92102
(updater: (prev: ViewState) => ViewState) => {
93-
setState((prev) => {
94-
const next = updater(prev);
95-
if (next === prev) return prev;
96-
writeState(storageKey, next);
97-
return next;
98-
});
103+
const prev = stateRef.current;
104+
const next = updater(prev);
105+
if (next === prev) return;
106+
stateRef.current = next;
107+
setState(next);
108+
writeState(storageKey, next);
99109
},
100110
[storageKey],
101111
);

0 commit comments

Comments
 (0)