Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 10 additions & 13 deletions eslint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,16 @@ export default tseslint.config(
},
rules: {
...reactHooks.configs.recommended.rules,
// eslint-plugin-react-hooks v7 folded the React Compiler rule set into
// `recommended`, which flags 67 pre-existing sites (set-state-in-effect,
// ref access during render, manual-memoization, immutability). Adopting
// them is a deliberate, codebase-wide refactor — not part of a version
// bump — so they are deferred here to preserve the enforcement level the
// code was written and verified against. Re-enable incrementally (per
// rule, file-scoped) in a dedicated react-hooks-7 adoption pass — see #1063.
"react-hooks/set-state-in-effect": "off",
"react-hooks/refs": "off",
"react-hooks/preserve-manual-memoization": "off",
"react-hooks/immutability": "off",
// Historically `warn` under v5's recommended; v7 raised it to error.
// Keep it non-blocking to match prior behavior.
// The React Compiler rule set (folded into `recommended` by
// eslint-plugin-react-hooks v7) is fully adopted — #1063. Genuine
// render-derivable cases were refactored (adjust-state-during-render, layout
// effects, re-keyed memos); legitimate effect-bound cases (async I/O, timers,
// DOM measurement, external-event sync, open/close transitions) carry a
// scoped disable with a per-site reason. These stay at `error` so new
// violations are caught at the source.
// `exhaustive-deps` was historically `warn` under v5's recommended; v7 raised
// it to error. Kept non-blocking to match prior behavior — its ~67 sites are
// out of scope for #1063.
"react-hooks/exhaustive-deps": "warn",
"@typescript-eslint/no-unused-vars": [
"error",
Expand Down
19 changes: 14 additions & 5 deletions src/components/CommandPalette/CommandPalette.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export function CommandPalette() {
const close = useCommandPaletteStore((s) => s.close);
const [query, setQuery] = useState("");
const [selectedIndex, setSelectedIndex] = useState(0);
const [prevQuery, setPrevQuery] = useState(query);
const inputRef = useRef<HTMLInputElement>(null);
const previousFocusRef = useRef<Element | null>(null);

Expand All @@ -48,7 +49,18 @@ export function CommandPalette() {
[isOpen, query],
);

// Reset and focus on open; restore previous focus on close (a11y).
// Reset the highlighted row to the top whenever the query changes — adjusted
// during render (React's recommended alternative to a setState-in-effect, which
// would cost an extra render per keystroke). #1063
if (query !== prevQuery) {
setPrevQuery(query);
setSelectedIndex(0);
}

// Reset and focus on open; restore previous focus on close (a11y). Legitimate
// setState-in-effect: bound to the open/close transition and bundled with focus
// capture/restore + RAF focus, not derivable during render (#1063).
/* eslint-disable react-hooks/set-state-in-effect */
useEffect(() => {
if (isOpen) {
previousFocusRef.current = document.activeElement;
Expand All @@ -63,10 +75,7 @@ export function CommandPalette() {
previousFocusRef.current = null;
}
}, [isOpen]);

useEffect(() => {
setSelectedIndex(0);
}, [query]);
/* eslint-enable react-hooks/set-state-in-effect */

if (!isOpen) return null;

Expand Down
4 changes: 4 additions & 0 deletions src/components/ContentSearch/useContentSearchScheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ export function useContentSearchScheduler({
// query/option change without a stale capture and without re-running the
// search merely because the array identity changed.
const excludeFoldersRef = useRef(excludeFolders);
// Synced during render (not an effect) so an already-pending debounced search
// reads the latest exclusions even after an exclusion-only re-render. Read only
// at query-execution time inside the debounce, never during render. #1063
// eslint-disable-next-line react-hooks/refs
excludeFoldersRef.current = excludeFolders;

useEffect(() => {
Expand Down
25 changes: 16 additions & 9 deletions src/components/Editor/HeadingPicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,12 @@ export function HeadingPicker() {
const previousFocusRef = useRef<Element | null>(null);
const [portalTarget, setPortalTarget] = useState<HTMLElement | null>(null);

// Find editor container for portal mounting
// Find editor container for portal mounting. Legitimate setState-in-effect: the
// target is read from the DOM after mount, so it can't be resolved during
// render (#1063).
useEffect(() => {
const editorContainer = document.querySelector('.editor-container') as HTMLElement | null;
// eslint-disable-next-line react-hooks/set-state-in-effect
setPortalTarget(editorContainer);
}, []);

Expand Down Expand Up @@ -156,7 +159,10 @@ export function HeadingPicker() {
capture: false,
});

// Calculate popup position when opening
// Calculate popup position when opening. Legitimate setState-in-effect: depends
// on DOM measurement (portalTarget.getBoundingClientRect) that is only valid
// after layout, not during render (#1063).
/* eslint-disable react-hooks/set-state-in-effect */
useEffect(() => {
if (!isOpen) return;

Expand Down Expand Up @@ -190,14 +196,15 @@ export function HeadingPicker() {
setPosition({ top, left });
}
}, [isOpen, anchorRect, containerBounds, portalTarget]);
/* eslint-enable react-hooks/set-state-in-effect */

// Reset and clamp selection when filter changes
useEffect(() => {
setSelectedIndex((prev) => {
if (filteredHeadings.length === 0) return 0;
return Math.min(prev, filteredHeadings.length - 1);
});
}, [filter, filteredHeadings.length]);
// Clamp the selection when the filtered list shrinks. Adjusted during render
// (converges immediately) rather than in an effect (#1063).
if (filteredHeadings.length === 0) {
if (selectedIndex !== 0) setSelectedIndex(0);
} else if (selectedIndex > filteredHeadings.length - 1) {
setSelectedIndex(filteredHeadings.length - 1);
}

// Scroll selected item into view
useEffect(() => {
Expand Down
5 changes: 4 additions & 1 deletion src/components/Editor/ImageContextMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,11 @@ export function ImageContextMenu({ onAction }: ImageContextMenuProps) {
menu.style.top = `${adjustedY}px`;
}, [position]);

// Focus the first item whenever the menu opens.
// Focus the first item whenever the menu opens (reset to -1 on close).
// Legitimate setState-in-effect: resets the roving-focus index on the open/close
// transition (including on mount), paired with the DOM-focus effect below (#1063).
useEffect(() => {
// eslint-disable-next-line react-hooks/set-state-in-effect
setFocusedIndex(isOpen ? 0 : -1);
}, [isOpen]);

Expand Down
24 changes: 12 additions & 12 deletions src/components/Editor/SourceEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ export function SourceEditor({ hidden = false, readOnly = false }: SourceEditorP
const viewRef = useRef<EditorView | null>(null);
const isInternalChange = useRef(false);
const hiddenRef = useRef(hidden);
hiddenRef.current = hidden;

useSourceOutlineSync(viewRef, hidden);

Expand All @@ -80,10 +79,14 @@ export function SourceEditor({ hidden = false, readOnly = false }: SourceEditorP
const setCursorInfoRef = useRef(setCursorInfo);
const setSelectedTextRef = useRef(setSelectedText);
const cursorInfoRef = useRef(cursorInfo);
setContentRef.current = setContent;
setCursorInfoRef.current = setCursorInfo;
setSelectedTextRef.current = setSelectedText;
cursorInfoRef.current = cursorInfo;
// Sync "latest value" refs after commit (read only from the CodeMirror listener/effects) — concurrent-safe (#1063).
useEffect(() => {
hiddenRef.current = hidden;
setContentRef.current = setContent;
setCursorInfoRef.current = setCursorInfo;
setSelectedTextRef.current = setSelectedText;
cursorInfoRef.current = cursorInfo;
});

// Use editor store for global settings
const wordWrap = useUIStore((state) => state.wordWrap);
Expand All @@ -102,20 +105,17 @@ export function SourceEditor({ hidden = false, readOnly = false }: SourceEditorP
enabled: !hidden,
});

// Reset parent scroll when source editor mounts or becomes visible.
// .editor-content retains its scrollTop from WYSIWYG mode even after
// overflow switches to hidden, causing the source editor to appear
// displaced (content at bottom instead of top).
// Reset parent scroll on mount/show: .editor-content keeps its WYSIWYG scrollTop
// after overflow flips to hidden, displacing the source editor's content.
useEffect(() => {
const editorContent = containerRef.current?.closest(".editor-content") as HTMLElement | null;
if (editorContent && !hidden) {
editorContent.scrollTop = 0;
}
}, [hidden]);

// Clear shared selectedText when this editor becomes hidden — keeps the
// status bar from showing this editor's last selection while the other
// editor (WYSIWYG mode) is active.
// Clear shared selectedText when hidden — keeps the status bar from showing this
// editor's last selection while the WYSIWYG editor is active.
useEffect(() => {
if (hidden) setSelectedTextRef.current("");
}, [hidden]);
Expand Down
5 changes: 4 additions & 1 deletion src/components/Editor/SplitPaneEditor/SourcePane.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,10 @@ export function SourcePane({
// would tear down and rebuild the CodeMirror view, blowing away undo
// history and the user's selection. (Audit finding H3.)
const onDiagnosticsRef = useRef(onDiagnostics);
onDiagnosticsRef.current = onDiagnostics;
// Synced after commit (read only from the CodeMirror diagnostics callback). #1063
useEffect(() => {
onDiagnosticsRef.current = onDiagnostics;
});

// Stable jump-to-position handle, safe to re-emit whenever the parent's
// callback prop changes identity. Lives outside the mount effect so a
Expand Down
52 changes: 26 additions & 26 deletions src/components/Editor/TiptapEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -180,12 +180,15 @@ export function TiptapEditorInner({ hidden = false, readOnly = false, preview =
const contentRef = useRef(content);
const editorRef = useRef<TiptapEditor | null>(null);
const flushToStoreRef = useRef<((editor: TiptapEditor) => void) | null>(null);
cursorInfoRef.current = cursorInfo;
preserveLineBreaksRef.current = preserveLineBreaks;
hardBreakStyleOnSaveRef.current = hardBreakStyleOnSave;
hiddenRef.current = hidden;
previewRef.current = preview;
contentRef.current = content;
// Sync "latest value" refs after commit (read only from callbacks/effects) — concurrent-safe (#1063).
useEffect(() => {
cursorInfoRef.current = cursorInfo;
preserveLineBreaksRef.current = preserveLineBreaks;
hardBreakStyleOnSaveRef.current = hardBreakStyleOnSave;
hiddenRef.current = hidden;
previewRef.current = preview;
contentRef.current = content;
});

const extensions = useMemo(
() => createTiptapExtensions({ tabId: activeTabId, lintEnabled }),
Expand Down Expand Up @@ -228,6 +231,9 @@ export function TiptapEditorInner({ hidden = false, readOnly = false, preview =
},
[setContent, windowLabel]
);
// Synced during render so the unmount-flush cleanup below sees the latest flusher
// even if a passive effect hasn't run yet (#755).
// eslint-disable-next-line react-hooks/refs
flushToStoreRef.current = flushToStore;

const flushCursorInfo = useCallback(() => {
Expand Down Expand Up @@ -427,27 +433,24 @@ export function TiptapEditorInner({ hidden = false, readOnly = false, preview =
},
});

// Keep editorRef aligned with the live editor so unmount cleanup can flush
// directly without depending on the global flusher registry — which may be
// nulled by this component's own registration cleanup before the flush
// cleanup runs (React runs effect cleanups in reverse registration order).
// Keep editorRef aligned with the live editor for the unmount-flush cleanup.
// Synced during render (not an effect) so it is set even if a passive effect
// hasn't run, and so it survives the reverse-order cleanup race (#755).
// eslint-disable-next-line react-hooks/refs
editorRef.current = editor ?? null;

// Show-invisibles toggle — flip the extension storage flag and
// dispatch a transaction that the plugin's apply() picks up to
// rebuild decorations.
// Show-invisibles toggle — flip the extension storage flag, then dispatch a
// tagged transaction the plugin's apply() picks up to rebuild decorations.
useEffect(() => {
if (!editor) return;
// Update the extension's storage flag so future doc-changed
// transactions see the new value in the plugin's apply() path.
const allStorage = editor.storage as unknown as
| Record<string, { enabled?: boolean } | undefined>
| undefined;
const storage = allStorage?.showInvisibles;
// editor.storage is Tiptap's intentionally-mutable extension storage, not React state (#1063).
// eslint-disable-next-line react-hooks/immutability
if (storage) storage.enabled = showInvisibles;
// Force an immediate rebuild via the plugin's exported helper —
// this dispatches a tagged transaction the plugin recognises by
// PluginKey identity (a string meta key would silently no-op).
// Force a rebuild via the plugin's helper (recognised by PluginKey identity).
const view = editor.view;
if (!view) return;
setShowInvisibles(view, showInvisibles);
Expand All @@ -468,16 +471,13 @@ export function TiptapEditorInner({ hidden = false, readOnly = false, preview =
enabled: !!editor && !hidden && !preview,
});

// Cleanup all pending timers/RAFs on unmount to prevent memory leaks.
// Flush any pending content BEFORE cancelling timers to avoid data loss —
// keystrokes within the debounce window exist only in PM's in-memory doc (#755).
// Cleanup pending timers/RAFs on unmount. Flush pending content BEFORE
// cancelling — keystrokes in the debounce window live only in PM's doc (#755).
useEffect(() => {
return () => {
// Flush pending content directly via this instance's editor — relying on
// the global flushActiveWysiwygNow() registry was racy: React cleans up
// effects in reverse registration order, so the flusher deregistration
// (useEffect below) runs before this cleanup and the flush becomes a
// no-op, losing keystrokes within the debounce window (#755).
// Flush directly via this instance's editor: the global flushActiveWysiwygNow()
// registry was racy — React cleans effects up in reverse registration order, so
// the flusher deregistration ran first and the flush no-op'd, losing data (#755).
if ((pendingRaf.current || pendingDebounceTimeout.current) && editorRef.current && flushToStoreRef.current) {
try { flushToStoreRef.current(editorRef.current); } catch { /* defensive */ }
}
Expand Down
30 changes: 15 additions & 15 deletions src/components/Editor/UniversalToolbar/UniversalToolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@
* A universal, single-line toolbar anchored at the bottom of the window.
* Triggered by Shift+Cmd+P, provides formatting actions across WYSIWYG and Source.
*
* Per redesign spec:
* - Focus toggle model (Shift+Cmd+P toggles focus, not visibility)
* - Two-step Escape (dropdown first, then toolbar)
* - Session memory (cleared on toolbar close)
* - Smart initial focus (active marks > selection > context > default)
* Per redesign spec: focus-toggle model (Shift+Cmd+P toggles focus, not visibility),
* two-step Escape (dropdown then toolbar), session memory (cleared on close), and
* smart initial focus (active marks > selection > context > default).
*
* @module components/Editor/UniversalToolbar
*/
Expand Down Expand Up @@ -89,9 +87,8 @@ export function UniversalToolbar() {
[buttons, toolbarContext]
);

// AI-Prompts action button: trailing pseudo-button in the roving-tabindex
// model at index `buttons.length` (a11y/A4 — keyboard-reachable). Always
// enabled, an action (never a dropdown).
// AI-Prompts action button: trailing pseudo-button in the roving-tabindex model
// at index `buttons.length` (a11y/A4 — keyboard-reachable). Always enabled, an action.
const genieFocusIndex = buttons.length;

const isButtonFocusable = useCallback(
Expand Down Expand Up @@ -287,9 +284,8 @@ export function UniversalToolbar() {
(direction: "left" | "right" | "forward" | "backward") => {
const isArrowNav = direction === "left" || direction === "right";
const isNext = direction === "right" || direction === "forward";
// genieFocusIndex + 1 = full roving count (group buttons + the trailing
// AI-Prompts pseudo-button), so dropdown-exit nav can land on the Genie
// button too (A4).
// genieFocusIndex + 1 = full roving count (group buttons + trailing AI-Prompts
// pseudo-button), so dropdown-exit nav can land on the Genie button too (A4).
const newIndex = isNext
? getNextFocusableIndex(focusedIndex, genieFocusIndex + 1, isButtonFocusable)
: getPrevFocusableIndex(focusedIndex, genieFocusIndex + 1, isButtonFocusable);
Expand Down Expand Up @@ -332,17 +328,18 @@ export function UniversalToolbar() {
}
}, [focusedIndex]);

// Sync with store's dropdown state (for global Escape handling)
// Sync local dropdown state from the external store (for global Escape handling).
useEffect(() => {
// Store says dropdown should be closed, but local state says open
if (!storeDropdownOpen && menuOpen) {
// eslint-disable-next-line react-hooks/set-state-in-effect -- reacts to external store dropdown state (#1063)
closeMenu();
}
}, [storeDropdownOpen, menuOpen, closeMenu]);

// Close dropdown when focus leaves toolbar (focus toggle)
// Close dropdown when focus leaves the toolbar (focus toggle).
useEffect(() => {
if (!toolbarHasFocus && menuOpen) {
// eslint-disable-next-line react-hooks/set-state-in-effect -- reacts to external toolbar-focus signal (#1063)
closeMenu(false);
}
}, [toolbarHasFocus, menuOpen, closeMenu]);
Expand All @@ -359,7 +356,9 @@ export function UniversalToolbar() {
}
}, [visible, toolbarHasFocus, focusActiveEditor]);

// Handle toolbar open/close and initial focus
// Handle toolbar open/close and initial focus — reacts to the external visibility
// toggle and seeds keyboard focus from session memory / button states (#1063).
/* eslint-disable react-hooks/set-state-in-effect */
useEffect(() => {
if (!visible) {
wasVisibleRef.current = false;
Expand Down Expand Up @@ -389,6 +388,7 @@ export function UniversalToolbar() {

wasVisibleRef.current = true;
}, [visible, buttonStates, setFocusedIndex, closeMenu, sessionFocusIndex, tDialog]);
/* eslint-enable react-hooks/set-state-in-effect */

// Handle click outside dropdown
useEffect(() => {
Expand Down
5 changes: 5 additions & 0 deletions src/components/Editor/WorkflowEditor/useActionMetadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ export function useActionMetadata(
: { state: "idle" },
);

// Legitimate setState-in-effect: transitions to loading then resolves from an
// async metadata fetch (with a mounted guard) — driven by I/O keyed on `uses`,
// not derivable during render (#1063).
/* eslint-disable react-hooks/set-state-in-effect */
useEffect(() => {
if (!uses || !isResolvableRef(uses)) {
setResult({ state: "idle" });
Expand Down Expand Up @@ -132,6 +136,7 @@ export function useActionMetadata(
mounted = false;
};
}, [uses, isLocalCtx]);
/* eslint-enable react-hooks/set-state-in-effect */

return result;
}
Loading