From 74d8a4f540124c5437d74a7b1269dd76b0cb801c Mon Sep 17 00:00:00 2001 From: NesiciCoding Date: Tue, 11 Aug 2026 23:45:39 +0200 Subject: [PATCH 1/2] perf(render): stabilize cross-domain action identities via currentStateRef MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three roster/settings actions closed over state slices owned by other domains (createStudentRubric, createGroupStudentRubrics over rubrics; getActiveGradeScale over gradeScales), so a change in the authoring domain re-identified them and re-rendered every consumer of the roster and settings domains even though their own data had not changed. They now read the fresh state through currentStateRef with empty dep arrays, keeping their identity stable: a domain value only changes when the slices it exposes change. The actions are only ever called from event handlers, so reading the ref (which is synced to the latest committed state in an effect) is never stale. Adds two regression tests asserting roster/settings consumers do not re-render when rubrics/grade scales change; both fail without the fix. 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- src/context/AppContext.test.tsx | 83 ++++++++++++++++++++++++++++++++- src/context/AppContext.tsx | 23 ++++++--- 2 files changed, 98 insertions(+), 8 deletions(-) diff --git a/src/context/AppContext.test.tsx b/src/context/AppContext.test.tsx index 67a2a68e..0cc7e40a 100644 --- a/src/context/AppContext.test.tsx +++ b/src/context/AppContext.test.tsx @@ -1,10 +1,11 @@ import React, { ReactNode } from 'react'; import { renderHook, act, render } from '@testing-library/react'; import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { AppProvider, useApp, useRoster, useSettings } from './AppContext'; +import { AppProvider, useApp, useRoster, useSettings, useAuthoring } from './AppContext'; import * as storage from '../store/storage'; import { storageSync } from '../services/database'; import type { Rubric, GradeScale } from '../types'; +import { DEFAULT_FORMAT } from '../types'; // vi.hoisted so this spy can be created before vi.mock('../hooks/useToast', ...) runs // (vi.mock factories are hoisted above regular imports/consts) and still be referenced @@ -686,4 +687,84 @@ describe('AppContext', () => { expect(rosterRenders.length).toBeGreaterThan(1); // roster consumer re-rendered expect(settingsRenders.length).toBe(1); // settings consumer did NOT }); + + it('roster consumers do not re-render when authoring data (rubrics) changes', () => { + // Cross-domain action stability: createStudentRubric / createGroupStudentRubrics read + // rubrics via currentStateRef, so their identity (and thus rosterValue) must not change + // when the authoring rubrics collection changes. + const rosterRenders: number[] = []; + let triggerAddRubric: (() => void) | null = null; + + function RosterProbe() { + const { students } = useRoster(); + rosterRenders.push(students.length); + return
{students.length}
; + } + + function TriggerProbe() { + const { addRubric } = useAuthoring(); + triggerAddRubric = () => + addRubric({ + name: 'New Rubric', + subject: '', + description: '', + criteria: [], + gradeScaleId: 'default-scale', + format: DEFAULT_FORMAT, + attachmentIds: [], + totalMaxPoints: 100, + scoringMode: 'weighted-percentage', + }); + return null; + } + + render( + + + + + ); + + expect(rosterRenders.length).toBe(1); + + act(() => { + triggerAddRubric!(); + }); + + expect(rosterRenders.length).toBe(1); // roster consumer did NOT re-render + }); + + it('settings consumers do not re-render when authoring data (grade scales) changes', () => { + // getActiveGradeScale reads gradeScales via currentStateRef, so its identity (and thus + // settingsValue) must not change when the authoring grade-scales collection changes. + const settingsRenders: string[] = []; + let triggerAddGradeScale: (() => void) | null = null; + + function SettingsProbe() { + const { settings } = useSettings(); + settingsRenders.push(settings.language); + return
{settings.language}
; + } + + function TriggerProbe() { + const { addGradeScale } = useAuthoring(); + triggerAddGradeScale = () => addGradeScale({ name: 'New Scale', type: 'points', ranges: [] }); + return null; + } + + render( + + + + + ); + + expect(settingsRenders.length).toBe(1); + + act(() => { + triggerAddGradeScale!(); + }); + + expect(settingsRenders.length).toBe(1); // settings consumer did NOT re-render + }); }); diff --git a/src/context/AppContext.tsx b/src/context/AppContext.tsx index b26cb246..aacc1bcb 100644 --- a/src/context/AppContext.tsx +++ b/src/context/AppContext.tsx @@ -1919,9 +1919,14 @@ export function AppProvider({ children }: { children: ReactNode }) { [] ); + // These actions read state via currentStateRef instead of closing over a slice so their + // identity stays stable: a closure over e.g. `state.rubrics` would re-identify the action + // (and thus re-render every roster consumer) whenever rubrics change, even though rubrics + // belong to the authoring domain. Read-only helpers built from fresh state keep each domain + // value changing only when the slices it exposes change. const createStudentRubric = useCallback( (rubricId: string, studentId: string): StudentRubric => { - const rubric = state.rubrics.find((r) => r.id === rubricId); + const rubric = currentStateRef.current.rubrics.find((r) => r.id === rubricId); const entries: ScoreEntry[] = (rubric?.criteria ?? []).map((c) => ({ criterionId: c.id, levelId: null, @@ -1939,7 +1944,7 @@ export function AppProvider({ children }: { children: ReactNode }) { dispatch({ type: 'SAVE_STUDENT_RUBRIC', payload: sr }); return sr; }, - [state.rubrics] + [] ); /** @@ -1949,7 +1954,7 @@ export function AppProvider({ children }: { children: ReactNode }) { */ const createGroupStudentRubrics = useCallback( (rubricId: string, studentIds: string[]): StudentRubric[] => { - const rubric = state.rubrics.find((r) => r.id === rubricId); + const rubric = currentStateRef.current.rubrics.find((r) => r.id === rubricId); const entries: ScoreEntry[] = (rubric?.criteria ?? []).map((c) => ({ criterionId: c.id, levelId: null, @@ -1958,7 +1963,7 @@ export function AppProvider({ children }: { children: ReactNode }) { })); const groupId = nanoid(); const srs = studentIds.map((studentId): StudentRubric => { - const existing = state.studentRubrics.find( + const existing = currentStateRef.current.studentRubrics.find( (sr) => sr.rubricId === rubricId && sr.studentId === studentId && !sr.isPeerReview ); return { @@ -1980,7 +1985,7 @@ export function AppProvider({ children }: { children: ReactNode }) { srs.forEach((sr) => dispatch({ type: 'SAVE_STUDENT_RUBRIC', payload: sr })); return srs; }, - [state.rubrics, state.studentRubrics] + [] ); const deleteStudentRubric = useCallback((id: string, scope: 'student' | 'group') => { @@ -2015,9 +2020,13 @@ export function AppProvider({ children }: { children: ReactNode }) { [] ); + // Read via currentStateRef for the same reason as createStudentRubric: gradeScales belongs to + // the authoring domain, so closing over it would re-render every settings consumer on any + // grade-scale change. const getActiveGradeScale = useCallback((): GradeScale => { - return state.gradeScales.find((gs) => gs.id === state.settings.defaultGradeScaleId) ?? state.gradeScales[0]; - }, [state.gradeScales, state.settings.defaultGradeScaleId]); + const { gradeScales, settings } = currentStateRef.current; + return gradeScales.find((gs) => gs.id === settings.defaultGradeScaleId) ?? gradeScales[0]; + }, []); const addFavoriteStandard = useCallback( (s: LinkedStandard) => dispatch({ type: 'ADD_FAVORITE_STANDARD', payload: s }), From 68a4e4f6b462cd99ee082cd54fc5f99a7d3a397b Mon Sep 17 00:00:00 2001 From: NesiciCoding Date: Wed, 12 Aug 2026 18:14:40 +0200 Subject: [PATCH 2/2] fix(render): sync currentStateRef during render so layout effects see fresh state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ref was synced in a passive useEffect, which fires after paint — a descendant's useLayoutEffect invoking createStudentRubric / getActiveGradeScale would read the previous commit's state (layout effects run child-first, so a provider-side effect can only ever be one commit behind). Writing the ref during render (documented latest-ref pattern) keeps it at the current render's state for every phase while preserving the stable action identities. Adds regression tests invoking the actions from a layout effect and asserting they observe the latest rubric and grade-scale state. 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- src/context/AppContext.test.tsx | 81 ++++++++++++++++++++++++++++++++- src/context/AppContext.tsx | 12 +++-- 2 files changed, 87 insertions(+), 6 deletions(-) diff --git a/src/context/AppContext.test.tsx b/src/context/AppContext.test.tsx index 174b7ee6..cce31e2e 100644 --- a/src/context/AppContext.test.tsx +++ b/src/context/AppContext.test.tsx @@ -1,4 +1,4 @@ -import React, { ReactNode } from 'react'; +import React, { ReactNode, useLayoutEffect, useRef } from 'react'; import { renderHook, act } from '@testing-library/react'; import { describe, it, expect, vi, beforeEach } from 'vitest'; import { renderWithRouter } from '../test-utils/renderWithProviders'; @@ -771,4 +771,83 @@ describe('AppContext', () => { expect(settingsRenders.length).toBe(1); // settings consumer did NOT re-render }); + + it('actions invoked from a layout effect see the latest rubric state', () => { + // Regression for currentStateRef sync timing: the provider must refresh the ref in the + // layout phase (parent layout effects fire before descendants'), so an action called from + // a descendant's useLayoutEffect — e.g. pre-filling a just-created rubric — reads the + // fresh snapshot instead of the previous render's. + const entryCounts: number[] = []; + const rubricIdRef: { current: string } = { current: '' }; + + function LayoutEffectProbe() { + const { createStudentRubric, createGroupStudentRubrics } = useRoster(); + const { addRubric } = useAuthoring(); + const runRef = useRef(0); + + useLayoutEffect(() => { + runRef.current += 1; + if (runRef.current === 1) { + const rubric = addRubric({ + name: 'Layout Rubric', + subject: '', + description: '', + gradeScaleId: 'default-scale', + format: DEFAULT_FORMAT, + attachmentIds: [], + totalMaxPoints: 30, + scoringMode: 'weighted-percentage', + criteria: [ + { id: 'c1', title: 'C1', description: '', weight: 34, levels: [] }, + { id: 'c2', title: 'C2', description: '', weight: 33, levels: [] }, + { id: 'c3', title: 'C3', description: '', weight: 33, levels: [] }, + ], + }); + rubricIdRef.current = rubric.id; + } else if (runRef.current === 2) { + const single = createStudentRubric(rubricIdRef.current, 'student-1'); + entryCounts.push(single.entries.length); + const group = createGroupStudentRubrics(rubricIdRef.current, ['student-1', 'student-2']); + entryCounts.push(group[0].entries.length); + } + }); + + return
; + } + + renderWithRouter(, { withAppProvider: true }); + act(() => {}); // flush the re-render scheduled by the first layout effect + + // Each created StudentRubric must mirror the 3 criteria of the rubric added in run 1. + expect(entryCounts).toEqual([3, 3]); + }); + + it('getActiveGradeScale invoked from a layout effect sees the latest grade-scale state', () => { + const observedNames: string[] = []; + + function LayoutEffectScaleProbe() { + const { getActiveGradeScale, updateSettings } = useSettings(); + const { addGradeScale } = useAuthoring(); + const runRef = useRef(0); + + useLayoutEffect(() => { + runRef.current += 1; + if (runRef.current === 1) { + const scale = addGradeScale({ name: 'Layout Scale', type: 'points', ranges: [] }); + updateSettings({ defaultGradeScaleId: scale.id }); + } else if (runRef.current === 2) { + observedNames.push(getActiveGradeScale().name); + } + }); + + return
; + } + + renderWithRouter(, { withAppProvider: true }); + act(() => {}); // flush the re-render scheduled by the first layout effect + + // With the fix the ref is refreshed in the layout phase, so the action sees the scale + // added in run 1; with a passive-effect sync it would still resolve the old default. + expect(observedNames).toEqual(['Layout Scale']); + }); }); diff --git a/src/context/AppContext.tsx b/src/context/AppContext.tsx index 2eb728ec..6aea77a0 100644 --- a/src/context/AppContext.tsx +++ b/src/context/AppContext.tsx @@ -1419,7 +1419,14 @@ async function flushToLocalStorage(merged: StoreData, changedKeys?: Set { - currentStateRef.current = state; - }, [state]); - // 'checking' while we detect session; 'show' = show landing; 'hide' = in app const [landingState, setLandingState] = useState<'checking' | 'show' | 'hide'>('checking'); // Ref so the OTP handler ([] deps effect) can read current state without