Conversation
WalkthroughRefactors Zustand store subscriptions to granular selectors with Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI component (Create file)
participant Hook as useCreateFile
participant Store as Project Store (selectors)
participant Ref as collectionsRef
Note over Hook,Ref: New flow uses a ref to hold collections snapshot
UI->>Hook: call createNewFile(params)
Hook->>Ref: read collectionsRef.current
alt collection found
Hook->>Store: getState() -> primitive selectors (selectedCollection, subdir, projectPath)
Hook->>Store: createFileMutation(request)
Store-->>Hook: mutation result
else no suitable collection
Hook-->>UI: return error / fallback
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas to focus on:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-11-09T21:33:04.153ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
src-tauri/src/telemetry.rs (1)
97-102: Complete the formatting consistency refactor.Lines 97-101 still use positional placeholders, whilst line 102 uses the inline style. For consistency across the file, consider updating the earlier log statement as well.
Apply this diff to maintain consistency:
log::info!( - "Created new telemetry UUID: {} at {}", - uuid, - telemetry_file.display() + "Created new telemetry UUID: {uuid} at {}", + telemetry_file.display() );Note:
telemetry_file.display()must remain as an argument since it's a method call, not a simple variable reference.src/components/layout/LeftSidebar.tsx (2)
30-31: Consider removing or conditionally enabling performance logs for production.The performance log will run in production builds. Consider wrapping it in a development-only check or removing it before release.
Apply this diff to make the log development-only:
- // eslint-disable-next-line no-console - console.log('[PERF] LeftSidebar RENDER') + if (import.meta.env.DEV) { + // eslint-disable-next-line no-console + console.log('[PERF] LeftSidebar RENDER') + }
47-53: Optional: useShallow provides minimal benefit for primitive map access.The selector returns a primitive boolean from the map, so
useShallowperforms the same comparison as a direct selector. This pattern is acceptable but could be simplified.If you prefer simpler code:
- const showDraftsOnly = - useUIStore( - useShallow( - state => state.draftFilterByCollection[selectedCollection || ''] - ) - ) || false + const showDraftsOnly = + useUIStore(state => state.draftFilterByCollection[selectedCollection || '']) || falsesrc/components/layout/UnifiedTitleBar.tsx (1)
22-23: Consider removing or conditionally enabling performance logs for production.The performance log will run in production builds. Consider wrapping it in a development-only check or removing it before release.
Apply this diff to make the log development-only:
- // eslint-disable-next-line no-console - console.log('[PERF] UnifiedTitleBar RENDER') + if (import.meta.env.DEV) { + // eslint-disable-next-line no-console + console.log('[PERF] UnifiedTitleBar RENDER') + }src/components/layout/Layout.tsx (1)
36-37: Consider removing or conditionally enabling performance logs for production.The performance log will run in production builds. Consider wrapping it in a development-only check or removing it before release.
Apply this diff to make the log development-only:
- // eslint-disable-next-line no-console - console.log('[PERF] Layout RENDER') + if (import.meta.env.DEV) { + // eslint-disable-next-line no-console + console.log('[PERF] Layout RENDER') + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
docs/tasks-todo/task-1-fix-zustand-subscription-performance.md(1 hunks)src-tauri/src/lib.rs(1 hunks)src-tauri/src/telemetry.rs(2 hunks)src/components/frontmatter/FrontmatterPanel.tsx(2 hunks)src/components/layout/Layout.tsx(3 hunks)src/components/layout/LeftSidebar.tsx(2 hunks)src/components/layout/StatusBar.tsx(1 hunks)src/components/layout/UnifiedTitleBar.tsx(2 hunks)src/hooks/commands/useCommandContext.ts(2 hunks)src/hooks/settings/useEffectiveSettings.ts(2 hunks)src/hooks/useCommandPalette.ts(1 hunks)src/hooks/useCreateFile.ts(5 hunks)src/hooks/useEditorFileContent.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (10)
src/components/layout/StatusBar.tsx (2)
src/store/editorStore.ts (1)
useEditorStore(38-203)src/store/uiStore.ts (1)
useUIStore(25-83)
src/components/layout/UnifiedTitleBar.tsx (3)
src/store/editorStore.ts (1)
useEditorStore(38-203)src/store/projectStore.ts (1)
useProjectStore(46-428)src/store/uiStore.ts (1)
useUIStore(25-83)
src/hooks/settings/useEffectiveSettings.ts (1)
src/store/projectStore.ts (1)
useProjectStore(46-428)
src/hooks/useCreateFile.ts (1)
src/store/projectStore.ts (1)
useProjectStore(46-428)
src/components/layout/LeftSidebar.tsx (3)
src/store/editorStore.ts (1)
useEditorStore(38-203)src/store/projectStore.ts (1)
useProjectStore(46-428)src/store/uiStore.ts (1)
useUIStore(25-83)
src/components/frontmatter/FrontmatterPanel.tsx (2)
src/store/editorStore.ts (1)
useEditorStore(38-203)src/store/projectStore.ts (1)
useProjectStore(46-428)
src/hooks/useCommandPalette.ts (2)
src/hooks/commands/useCommandContext.ts (1)
useCommandContext(13-106)src/store/uiStore.ts (1)
useUIStore(25-83)
src/hooks/useEditorFileContent.ts (2)
src/store/editorStore.ts (1)
useEditorStore(38-203)src/store/projectStore.ts (1)
useProjectStore(46-428)
src/hooks/commands/useCommandContext.ts (3)
src/store/editorStore.ts (1)
useEditorStore(38-203)src/store/projectStore.ts (1)
useProjectStore(46-428)src/store/uiStore.ts (1)
useUIStore(25-83)
src/components/layout/Layout.tsx (2)
src/store/uiStore.ts (1)
useUIStore(25-83)src/lib/theme-provider.tsx (1)
useTheme(24-32)
🪛 LanguageTool
docs/tasks-todo/task-1-fix-zustand-subscription-performance.md
[uncategorized] ~12-~12: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...ed (40% of problem) 3. Subscribing to frequently-changing values that components don't display ...
(HYPHENATED_LY_ADVERB_ADJECTIVE)
[uncategorized] ~735-~735: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...d shallow 2. [ ] Does it subscribe to frequently-changing values it doesn't display? → Use `getSt...
(HYPHENATED_LY_ADVERB_ADJECTIVE)
[grammar] ~957-~957: A determiner may be missing.
Context: ...ct primitive selectors** where possible (best performance) 4. **Use getState() in c...
(THE_SUPERLATIVE)
[uncategorized] ~1072-~1072: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...hallow` comparison 3. Subscribing to frequently-changing values (5% of problem) - Component...
(HYPHENATED_LY_ADVERB_ADJECTIVE)
🪛 markdownlint-cli2 (0.18.1)
docs/tasks-todo/task-1-fix-zustand-subscription-performance.md
690-690: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
696-696: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
702-702: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
710-710: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
791-791: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
835-835: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
879-879: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
1014-1014: Bare URL used
(MD034, no-bare-urls)
1015-1015: Bare URL used
(MD034, no-bare-urls)
1016-1016: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
1087-1087: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
1101-1101: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
1153-1153: Bare URL used
(MD034, no-bare-urls)
1154-1154: Bare URL used
(MD034, no-bare-urls)
1155-1155: Bare URL used
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (rust)
🔇 Additional comments (14)
src-tauri/src/lib.rs (1)
89-89: LGTM!The inline placeholder syntax is more concise and readable.
src-tauri/src/telemetry.rs (1)
44-44: LGTM!The inline placeholder syntax is more concise and aligns with the refactoring objective.
src/hooks/settings/useEffectiveSettings.ts (1)
5-5: LGTM! Correct use of shallow subscription for object.The refactoring correctly applies
useShallowfor thecurrentProjectSettingsobject subscription, preventing unnecessary re-renders when the object reference changes but values remain the same.Also applies to: 16-19
src/hooks/useCreateFile.ts (1)
56-73: LGTM! Correct use of selector syntax and ref pattern.The changes correctly apply:
- Selector syntax without shallow for
projectPath(primitive)useShallowforcurrentProjectSettings(object)- Ref pattern to access collections without recreating the callback on every query update
This effectively prevents unnecessary re-renders and callback recreations.
src/hooks/commands/useCommandContext.ts (1)
17-40: LGTM! Correct use of shallow vs primitive subscriptions.The refactoring correctly distinguishes between:
- Object subscriptions using
useShallow(currentFile, globalSettings, currentProjectSettings)- Primitive/function subscriptions without shallow (isDirty, closeCurrentFile, selectedCollection, projectPath, etc.)
This prevents unnecessary re-renders when object references change but values don't.
docs/tasks-todo/task-1-fix-zustand-subscription-performance.md (1)
1-1155: Excellent planning document for the refactor.This comprehensive documentation provides clear guidance on the Zustand subscription performance fixes, including:
- Root cause analysis
- Phased implementation plan
- Code examples for correct patterns
- Testing and verification steps
- Documentation update requirements
The document effectively explains why destructuring subscribes to the entire store and why
useShallowis needed for object subscriptions.src/components/frontmatter/FrontmatterPanel.tsx (1)
16-22: LGTM! Correct use of shallow for object subscriptions.The refactoring correctly applies:
useShallowfor object subscriptions (currentFile, frontmatter, currentProjectSettings)- Selector syntax without shallow for primitive (projectPath)
This prevents unnecessary re-renders when object references change but values remain the same.
src/hooks/useCommandPalette.ts (1)
17-20: LGTM! Consistent use of selector syntax.The refactoring correctly uses selector syntax for the function subscription. While shallow equality isn't needed for functions, using consistent selector syntax across the codebase improves maintainability.
src/components/layout/StatusBar.tsx (1)
11-22: LGTM! Correct use of shallow for objects and selector syntax for primitives.The refactoring correctly applies:
useShallowfor object subscription (currentFile)- Individual selectors without shallow for primitives/functions (sidebarVisible, frontmatterPanelVisible, distractionFreeBarsHidden, showBars)
This prevents unnecessary re-renders and follows the established pattern.
src/hooks/useEditorFileContent.ts (1)
19-21: LGTM! Correct use of shallow for object subscription.The refactoring correctly applies:
useShallowfor object subscription (currentFile)- Selector syntax without shallow for primitive (projectPath)
This prevents unnecessary re-renders when the currentFile object reference changes but values remain the same.
src/components/layout/LeftSidebar.tsx (1)
33-44: LGTM! Correct use of shallow and selector patterns.The subscription pattern correctly uses
useShallowfor object subscriptions (currentFile,currentProjectSettings) and direct selectors for primitives. This optimises re-renders by preventing updates when object contents haven't changed.src/components/layout/UnifiedTitleBar.tsx (1)
25-51: LGTM! Excellent subscription optimisation.The refactor correctly distinguishes between object and primitive subscriptions:
currentFileusesuseShallowfor shallow equality checking of the object- All primitives and functions use direct selectors without
useShallowThis pattern minimises unnecessary re-renders whilst maintaining correct reactivity.
src/components/layout/Layout.tsx (2)
39-51: LGTM! Smart extraction of nested values for useEffect dependencies.Excellent refactor that:
- Uses direct selectors for primitives (
sidebarVisible,frontmatterPanelVisible,theme)- Correctly applies
useShallowtoheadingColorobject for shallow comparison- Extracts specific nested values rather than subscribing to entire
globalSettings, preventing unnecessary re-renders when unrelated settings changeThis is a textbook example of optimised Zustand subscriptions.
92-124: LGTM! useEffect dependencies correctly updated.The effects now depend on the extracted
themeandheadingColorvalues rather than the entireglobalSettingsobject. This ensures effects only run when the specific values they care about change, improving performance.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
docs/tasks-todo/task-1-fix-zustand-subscription-performance.md (5)
807-807: Add language identifiers to fenced code blocks in markdown.Several code blocks lack language specifiers, which impacts readability and IDE syntax highlighting. Add language tags (e.g.,
```typescriptinstead of```).Affected line ranges:
- Line 807: Fenced code block (subscription audit checklist context)
- Line 851: Fenced code block (CLAUDE.md section)
- Line 895: Fenced code block (architecture-guide.md section)
- Line 1032: Fenced code block (references section)
- Line 1131: Fenced code block (render cascade—before fixes)
- Line 1145: Fenced code block (render cascade—after fixes)
Also applies to: 851-851, 895-895, 1032-1032, 1131-1131, 1145-1145
1030-1030: Use proper markdown link syntax for URLs instead of bare URLs.Bare URLs should be wrapped in angle brackets or converted to proper markdown links for consistency.
Examples:
- Line 1030:
https://docs.pmnd.rs/zustand/guides/prevent-rerenders-with-use-shallow→[Zustand shallow docs](https://docs.pmnd.rs/zustand/guides/prevent-rerenders-with-use-shallow)- Line 1197–1199: Similar URLs in References section
Also applies to: 1031-1031, 1197-1197, 1198-1198, 1199-1199
706-706: Replace emphasis with proper markdown headings.Lines 706, 712, 718, and 726 use bold emphasis (
**...**) where actual heading syntax (###) would be more appropriate for markdown structure and navigation.Example (line 706):
-**1. First: Try primitive selectors** +### First: Try primitive selectorsThis improves document outline and accessibility.
Also applies to: 712-712, 718-718, 726-726
28-28: Remove hyphens from adverbs modifying adjectives.The term "frequently-changing" should be written as "frequently changing" (adverbs ending in -ly do not require hyphens in compound modifiers when they precede an adjective).
Occurrences:
- Line 28: "Subscribing to frequently-changing values"
- Line 751: "Does it subscribe to frequently-changing values"
- Line 1116: "Subscribing to frequently-changing values"
Also applies to: 751-751, 1116-1116
956-956: Minor grammar: Add missing articles.
- Line 956: "Component re-renders only when those properties change" → "The component re-renders only when those properties change"
- Line 973: "Extract primitives instead of objects (best performance)" → "Extract primitives instead of objects (the best performance)"
Also applies to: 973-973
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/tasks-todo/task-1-fix-zustand-subscription-performance.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/tasks-todo/task-1-fix-zustand-subscription-performance.md
[uncategorized] ~28-~28: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...ed (40% of problem) 3. Subscribing to frequently-changing values that components don't display ...
(HYPHENATED_LY_ADVERB_ADJECTIVE)
[uncategorized] ~751-~751: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...d shallow 2. [ ] Does it subscribe to frequently-changing values it doesn't display? → Use `getSt...
(HYPHENATED_LY_ADVERB_ADJECTIVE)
[uncategorized] ~956-~956: Possible missing article found.
Context: ...Subscribes only to specific properties. Component re-renders only when those properties c...
(AI_HYDRA_LEO_MISSING_THE)
[grammar] ~973-~973: A determiner may be missing.
Context: ...ct primitive selectors** where possible (best performance) 4. **Use getState() in c...
(THE_SUPERLATIVE)
[uncategorized] ~1116-~1116: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...hallow` comparison 3. Subscribing to frequently-changing values (5% of problem) - Component...
(HYPHENATED_LY_ADVERB_ADJECTIVE)
🪛 markdownlint-cli2 (0.18.1)
docs/tasks-todo/task-1-fix-zustand-subscription-performance.md
706-706: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
712-712: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
718-718: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
726-726: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
807-807: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
851-851: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
895-895: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
1030-1030: Bare URL used
(MD034, no-bare-urls)
1031-1031: Bare URL used
(MD034, no-bare-urls)
1032-1032: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
1131-1131: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
1145-1145: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
1197-1197: Bare URL used
(MD034, no-bare-urls)
1198-1198: Bare URL used
(MD034, no-bare-urls)
1199-1199: Bare URL used
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (rust)
🔇 Additional comments (1)
docs/tasks-todo/task-1-fix-zustand-subscription-performance.md (1)
1-16: Clarify merge-readiness status and pending blockers.The file header marks status as "Implementation Complete" but explicitly lists blocking tasks that should be completed before merging to
main. Please clarify:
- Should this PR merge with console logs in place (flagged for removal at line 1075)?
- Are Phase 7 documentation updates (CLAUDE.md, architecture-guide.md, performance-patterns.md) being deferred, or should they be included in this PR?
- Manual testing scenarios (lines 1063–1070) should be verified before merge or documented as post-merge validation.
Consider restructuring the status section to clearly distinguish between merged code vs. pending follow-up tasks.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Next steps:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/components/layout/LeftSidebar.tsx (1)
44-50: Minor: useShallow unnecessary for primitive return value.The selector returns a primitive boolean value, so
useShallowprovides no benefit here—strict equality comparison is already sufficient for primitives. Whilst not harmful, the wrapper could be simplified:- const showDraftsOnly = - useUIStore( - useShallow( - state => state.draftFilterByCollection[selectedCollection || ''] - ) - ) || false + const showDraftsOnly = + useUIStore( + state => state.draftFilterByCollection[selectedCollection || ''] + ) || falseHowever, this is purely a pedantic observation and the current implementation works correctly.
src/hooks/commands/useCommandContext.ts (1)
39-102: Consider memoizing the return value for stability.The hook returns a new object on every render, which could cause unnecessary re-renders in components that use this context. However, based on the PR's focus and the task documentation indicating this is optional, this is acceptable for now.
If consumers of this hook experience re-renders, consider wrapping the return in
useMemowith appropriate dependencies (following the patterns in the task doc Phase 3.1).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
CLAUDE.md(4 hunks)README.md(1 hunks)docs/developer/architecture-guide.md(5 hunks)docs/developer/form-patterns.md(16 hunks)docs/developer/performance-issue-excessive-rerenders.md(0 hunks)docs/developer/performance-patterns.md(7 hunks)docs/developer/state-management.md(9 hunks)docs/tasks-done/task-2025-11-09-fix-zustand-subscription-performance.md(1 hunks)docs/user-guide.md(1 hunks)src/components/frontmatter/FrontmatterPanel.tsx(2 hunks)src/components/layout/Layout.tsx(3 hunks)src/components/layout/LeftSidebar.tsx(2 hunks)src/components/layout/StatusBar.test.tsx(0 hunks)src/components/layout/StatusBar.tsx(0 hunks)src/components/layout/UnifiedTitleBar.tsx(2 hunks)src/components/layout/index.ts(0 hunks)src/hooks/commands/useCommandContext.ts(2 hunks)src/hooks/useCommandPalette.ts(1 hunks)src/hooks/useCreateFile.ts(5 hunks)src/hooks/useEditorFileContent.ts(2 hunks)src/lib/editor/extensions/theme.ts(1 hunks)src/lib/layout-constants.ts(0 hunks)
💤 Files with no reviewable changes (5)
- src/components/layout/index.ts
- src/lib/layout-constants.ts
- src/components/layout/StatusBar.test.tsx
- docs/developer/performance-issue-excessive-rerenders.md
- src/components/layout/StatusBar.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/frontmatter/FrontmatterPanel.tsx
- src/hooks/useCommandPalette.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-20T20:15:05.062Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-10-20T20:15:05.062Z
Learning: Read CLAUDE.md before working on the codebase or submitting changes
Applied to files:
CLAUDE.md
🧬 Code graph analysis (6)
src/components/layout/UnifiedTitleBar.tsx (3)
src/store/editorStore.ts (1)
useEditorStore(38-203)src/store/projectStore.ts (1)
useProjectStore(46-428)src/store/uiStore.ts (1)
useUIStore(25-83)
src/hooks/useCreateFile.ts (1)
src/store/projectStore.ts (1)
useProjectStore(46-428)
src/hooks/useEditorFileContent.ts (2)
src/store/editorStore.ts (1)
useEditorStore(38-203)src/store/projectStore.ts (1)
useProjectStore(46-428)
src/hooks/commands/useCommandContext.ts (3)
src/store/editorStore.ts (1)
useEditorStore(38-203)src/store/projectStore.ts (1)
useProjectStore(46-428)src/store/uiStore.ts (1)
useUIStore(25-83)
src/components/layout/LeftSidebar.tsx (3)
src/store/editorStore.ts (1)
useEditorStore(38-203)src/store/projectStore.ts (1)
useProjectStore(46-428)src/store/uiStore.ts (1)
useUIStore(25-83)
src/components/layout/Layout.tsx (2)
src/store/uiStore.ts (1)
useUIStore(25-83)src/store/projectStore.ts (1)
useProjectStore(46-428)
🪛 LanguageTool
docs/developer/performance-patterns.md
[uncategorized] ~122-~122: A comma may be missing after the conjunctive/linking adverb ‘However’.
Context: ...tore(state => state.currentFile) ``` However, subscribing to objects/arrays can st...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
docs/tasks-done/task-2025-11-09-fix-zustand-subscription-performance.md
[uncategorized] ~28-~28: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...ed (40% of problem) 3. Subscribing to frequently-changing values that components don't display ...
(HYPHENATED_LY_ADVERB_ADJECTIVE)
[uncategorized] ~751-~751: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...d shallow 2. [ ] Does it subscribe to frequently-changing values it doesn't display? → Use `getSt...
(HYPHENATED_LY_ADVERB_ADJECTIVE)
[uncategorized] ~956-~956: Possible missing article found.
Context: ...Subscribes only to specific properties. Component re-renders only when those properties c...
(AI_HYDRA_LEO_MISSING_THE)
[grammar] ~973-~973: A determiner may be missing.
Context: ...ct primitive selectors** where possible (best performance) 4. **Use getState() in c...
(THE_SUPERLATIVE)
[uncategorized] ~1116-~1116: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...hallow` comparison 3. Subscribing to frequently-changing values (5% of problem) - Component...
(HYPHENATED_LY_ADVERB_ADJECTIVE)
docs/developer/state-management.md
[uncategorized] ~28-~28: Possible missing preposition found.
Context: ...ausing 15+ component re-renders because destructuring subscribed components to every store up...
(AI_HYDRA_LEO_MISSING_OF)
[formatting] ~77-~77: Did you mean “?”
Context: ... | Nested primitive | Direct selector | useStore(state => state.file?.name) | | Object | Selector + `useShall...
(MULTIPLE_PUNCTATION_MARKS_1)
[uncategorized] ~90-~90: When ‘Extract-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...] ✅ Primitive selectors preferred - Extract specific properties when possible - [ ] ✅ **getS...
(SPECIFIC_HYPHEN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (rust)
🔇 Additional comments (27)
docs/developer/form-patterns.md (1)
73-98: Excellent documentation of the selector pattern.The updated examples clearly demonstrate the correct use of Zustand selectors for accessing frontmatter fields and update functions. The pattern is consistent with the broader refactor across the codebase and properly illustrates the "Direct Store Pattern" critical for avoiding infinite loops.
docs/user-guide.md (1)
104-110: Documentation correctly reflects auto-save behaviour.The auto-save section clearly explains both the pause-based save and flow-state backup mechanisms whilst preserving the manual save option. The removal of specific UI notification text aligns with the StatusBar component removal elsewhere in this PR.
src/lib/editor/extensions/theme.ts (1)
27-27: Correct adjustment for StatusBar removal.The minHeight calculation now correctly subtracts only the titlebar height (44px) following the removal of the StatusBar component (previously 24px). This ensures the editor content fills the available viewport appropriately.
src/hooks/useEditorFileContent.ts (1)
16-18: Excellent use of shallow subscriptions for object state.The refactor correctly uses
useShallowfor thecurrentFileobject subscription to prevent unnecessary re-renders when the object reference changes but contents remain the same. The primitiveprojectPathsubscription appropriately uses a plain selector. This aligns with Zustand best practices and the broader performance optimisation pattern in this PR.src/components/layout/UnifiedTitleBar.tsx (1)
22-48: Well-executed migration to granular selectors.The refactor correctly distinguishes between object subscriptions (using
useShallowforcurrentFile) and primitive/function subscriptions (using plain selectors). The inline comments clearly explain the reasoning, and the pattern is consistent across all store accesses. This optimisation will reduce unnecessary re-renders whilst maintaining correct reactivity.src/hooks/useCreateFile.ts (2)
53-57: Correct use of selectors and shallow equality.The selector pattern correctly uses
useShallowfor thecurrentProjectSettingsobject whilst using a plain selector for the primitiveprojectPath. This follows Zustand best practices and aligns with the PR-wide refactor.
66-70: Effective ref pattern for stabilising callback dependencies.The ref pattern successfully stabilises the
createNewFilecallback by avoiding recreation on everycollectionsdata change. The useEffect ensures the ref stays synchronised with query data, and the stable dependency array (line 269) prevents unnecessary callback recreations. Whilst there's a theoretical risk of stale ref data if the callback executes before the useEffect runs, React's synchronous rendering makes this highly unlikely in practice.Also applies to: 84-84, 269-269
src/components/layout/LeftSidebar.tsx (1)
30-41: Excellent migration to granular, shallow-aware selectors.The refactor correctly uses
useShallowfor object subscriptions (currentFile,currentProjectSettings) and plain selectors for primitive subscriptions (selectedCollection,currentSubdirectory,projectPath). This follows Zustand best practices and will significantly reduce unnecessary re-renders.src/hooks/commands/useCommandContext.ts (2)
1-21: Excellent refactoring to granular selectors with shallow equality.The migration from destructuring to selector syntax with
useShallowfor objects is exactly right. This prevents re-renders from both unrelated store updates (via selector syntax) and object reference changes (via shallow comparison).The inline comments clearly document the distinction between object vs. primitive subscriptions, which will help maintain this pattern going forward.
23-37: Primitive and function subscriptions correctly implemented.Primitive values (booleans, strings) and functions use direct selectors without
useShallow, which is correct since they don't suffer from reference equality issues. This demonstrates a good understanding of the subscription pattern hierarchy.CLAUDE.md (2)
217-239: Documentation accurately reflects the refactored patterns.The Direct Store Pattern section now correctly emphasises selector syntax and
useShallowusage. The code examples match the implementation patterns used throughout the PR, providing clear guidance for future development.
416-444: Excellent troubleshooting guidance added.The new Performance Issues and Common Issues sections provide practical, actionable guidance for developers. The Quick Fix pattern for searching and replacing destructuring patterns is particularly helpful.
The emphasis on "NEVER destructure" and "ALWAYS use selector syntax" is appropriately strong given the performance impact documented in the task file.
docs/developer/architecture-guide.md (3)
210-219: CRITICAL warning appropriately placed and emphasised.This prominent warning at the beginning of the Critical Patterns section ensures developers understand the importance of proper subscription patterns before diving into implementation details. The cross-reference to state-management.md provides a clear path for comprehensive guidance.
222-290: Pattern examples accurately updated to reflect best practices.The getState() and Direct Store Pattern sections now correctly demonstrate selector syntax throughout. The updated examples provide clear before/after comparisons that match the codebase refactoring.
The note on line 290 about using
useShallowfor objects/arrays prevents reference change re-renders is a crucial addition.
564-567: Module diagram correctly updated to reflect StatusBar removal.The simplified module structure with Editor components under a single Editor node better reflects the current architecture. The change aligns with the StatusBar removal in other parts of the PR.
docs/developer/performance-patterns.md (3)
5-60: Critical warning and pattern examples effectively communicate the issues.The prominent warning at the top of the document ensures developers understand this is the "#1 performance issue in the codebase" before reading further. The getState() pattern examples clearly show the cascade problem and solution.
97-154: Comprehensive explanation of subscription optimization patterns.The section effectively explains the two-part problem (destructuring + object references) and provides clear solutions. The distinction between subscribing to the entire store vs. creating granular subscriptions is well-articulated.
The progressive examples (selector syntax → useShallow → advanced selectors) provide a clear learning path.
505-574: Anti-patterns section provides clear do/don't examples.The expanded anti-patterns section, particularly the new "Destructuring Subscribes to Entire Store" subsection, clearly demonstrates the performance pitfalls and their solutions. The three-tier solution (selector syntax → useShallow → primitive selectors) gives developers options based on their specific needs.
src/components/layout/Layout.tsx (3)
2-47: Subscription patterns correctly implemented with clear documentation.The refactoring properly distinguishes between primitive subscriptions (theme, visibility flags) and object subscriptions (headingColor). The inline comment on line 46 explaining that headingColor is an object with
.lightand.darkproperties is particularly helpful for future maintainers.This aligns perfectly with the Phase 0 pre-implementation fixes documented in the task file.
87-119: Effect dependencies correctly refactored to use stable selectors.The three useEffect hooks now depend on the extracted
themeandheadingColorvalues rather than nested property paths. This ensures the effects trigger correctly when these values change, solving the dependency issue that would have occurred withuseShallowon the fullglobalSettingsobject.This is exactly the fix outlined in Phase 0.1 of the task documentation.
121-180: Layout structure correctly updated without StatusBar.The removal of StatusBar from the layout aligns with the broader architectural cleanup mentioned in the PR. The three-panel resizable structure is maintained with proper visibility handling for sidebar and frontmatter panel.
docs/developer/state-management.md (3)
7-104: Outstanding comprehensive guide to Zustand subscription patterns.This new CRITICAL section is exemplary documentation. It:
- Clearly explains both problems (destructuring + object references)
- Provides a clear solution hierarchy from best to worst
- Includes a quick reference table for common scenarios
- Offers a code review checklist for PR reviews
- Documents real-world performance impact with metrics
The progression from problem → solution → patterns → checklist makes this immediately actionable for developers.
392-477: getState() pattern thoroughly documented with appropriate caveats.The section clearly explains when and why to use
getState(), with particularly good emphasis on the "When NOT to use" section. The bad example showing a component that never updates is a crucial warning that prevents developers from misapplying the pattern.
553-654: Hybrid Action Hooks Pattern excellently documented.This section provides a complete, real-world example of the pattern used throughout the codebase (particularly in the editor save flow). The three-part structure (hook → store → layout wiring) clearly shows how the pieces fit together.
The benefits list emphasises the advantages (no polling, type-safe, synchronous access) that make this pattern superior to event-based alternatives.
docs/tasks-done/task-2025-11-09-fix-zustand-subscription-performance.md (3)
1-121: Comprehensive task documentation with clear status and progression.The task file provides excellent documentation of the refactoring effort. The status badges clearly communicate progress, the overview explains the performance impact (87% reduction in re-renders), and Phase 0 thoroughly documents the critical pre-implementation fix needed for Layout.tsx.
The before/after code examples make the changes immediately understandable.
123-570: Detailed phase-by-phase implementation plan with examples.Each phase is thoroughly documented with:
- Current problematic code
- Proposed fix with explanations
- Why the fix works
- Testing verification steps
Phase 1's coverage of all 5 components (FrontmatterPanel, UnifiedTitleBar, LeftSidebar, Layout, StatusBar) matches the actual refactoring performed. The progressive phases (subscriptions → primitives → hooks → testing → docs) provide a logical implementation order.
1036-1082: Clear completion tracking with remaining tasks identified.The completion checklist accurately tracks progress:
- ✅ All implementation phases complete
- ✅ Quality checks passing
- 🚧 Manual testing needed
- 🚧 Documentation updates pending
- 🚧 Console logs need removal
This transparency about remaining work is excellent project management. The list of specific manual testing scenarios provides clear acceptance criteria.
Before merging to main, ensure:
- Manual testing scenarios (lines 1063-1069) are verified
- Console logs with
[PERF]are removed from all files (noted in past review comment for useCommandContext.ts)- Documentation updates in Phase 7 are completed if not already done
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/developer/architecture-guide.md (1)
80-80: Cross-reference documentation structure is helpful.The link to
state-management.mdfor comprehensive store decomposition and pattern details maintains good separation of concerns across documentation. Ensure this file is kept in sync as patterns evolve.Consider adding similar reference links at other critical junctures (e.g., after the Direct Store Pattern section on line 292) to guide developers to
form-patterns.mdandperformance-patterns.mdfor related details. This improves discoverability for pattern-specific deep dives.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/developer/architecture-guide.md(6 hunks)docs/developer/command-system.md(1 hunks)docs/user-guide.md(3 hunks)src/components/layout/LeftSidebar.tsx(2 hunks)src/components/preferences/panes/GeneralPane.tsx(0 hunks)
💤 Files with no reviewable changes (1)
- src/components/preferences/panes/GeneralPane.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/layout/LeftSidebar.tsx
- docs/user-guide.md
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-09T21:33:04.153Z
Learnt from: dannysmith
Repo: dannysmith/astro-editor PR: 55
File: README.md:39-39
Timestamp: 2025-11-09T21:33:04.153Z
Learning: The command palette in Astro Editor uses the keyboard shortcut Cmd+P (not Cmd+K).
Applied to files:
docs/developer/command-system.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (rust)
🔇 Additional comments (7)
docs/developer/command-system.md (2)
25-25: Keyboard shortcut update is correct and consistent.The change from Cmd+K to Cmd+P for command palette invocation aligns with the PR's Zustand refactoring goals and frees up Cmd+K for the insert-link command (documented later on lines 604 and 649). The documentation accurately reflects this change.
Please verify that related documentation files (especially
keyboard-shortcuts.md,state-management.md, andperformance-patterns.mdreferenced on lines 942–945) have been updated to reflect the new Cmd+P shortcut for consistency across the codebase.
442-530: ZustandgetState()pattern documentation is comprehensive and well-aligned with PR refactoring goals.The documentation correctly emphasises the
getState()pattern for accessing Zustand stores in command contexts where hooks cannot be used (lines 458–484). The provided examples are accurate, and the anti-pattern section (lines 487–498) clearly demonstrates what to avoid. Multi-store access is properly documented (lines 500–529).docs/developer/architecture-guide.md (5)
229-245: getState() pattern examples are clear and well-justified.The BAD vs GOOD comparison effectively illustrates why destructuring causes render cascades and dependency chaining. The stable empty dependency array in the GOOD example is correct.
262-290: Direct Store Pattern correctly emphasises selector syntax and useShallow.The
StringFieldexample accurately demonstrates the recommended pattern:
- Granular selectors:
useEditorStore(state => state.frontmatter[name])- Direct action selectors:
useEditorStore(state => state.updateFrontmatterField)- Line 290's note about
useShallowfor objects/arrays reinforces the performance guidance from the CRITICAL section.
564-566: Module dependencies diagram correctly reflects Editor restructuring.The removal of StatusBar from the MainEditor branch and the reorganisation of the Editor subtree align with the PR's stated changes. Diagram remains clear and helpful for new developers.
36-43: getState() example pattern is correct and well-commented.The
openInIdefunction properly demonstrates the acceptable use ofgetState()in pure business logic without React coupling, consistent with the "Exception" rule in the boundary guidelines.
210-219: Reference link verified—comprehensive supporting details confirmed.The
state-management.mdfile exists and contains an excellent, detailed section with clear examples of the two critical problems, ✅/❌ pattern demonstrations, and the useShallow solution. The guidance is consistent with and expands upon the critical callout in architecture-guide.md.Minor note: The anchor in the link (
#️-critical-zustand-subscription-patterns) includes an emoji character, which is unusual. Standard Markdown anchor generation would typically be#critical-zustand-subscription-patterns(without emoji). GitHub's link resolution is flexible and may auto-correct, but you may want to simplify the anchor to#critical-zustand-subscription-patternsfor reliability.
2 participants
Summary by CodeRabbit
Bug Fixes
UI Changes
Documentation