docs: Add Zustand development conventions to coding guidelines (resolves #289). #332
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughAdded a new "Zustand" section to the coding guidelines documenting store file naming, state/action typing patterns, default value naming, slicing conventions, and access/usage patterns; no runtime logic or exported API changes. Changes
Sequence Diagram(s)No sequence diagrams — documentation-only changes; no control-flow modifications. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docs/src/dev-guide/coding-guidelines.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
docs/src/dev-guide/coding-guidelines.md (3)
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/Editor/index.tsx:138-139
Timestamp: 2025-06-01T13:40:12.222Z
Learning: In the yscope-log-viewer codebase, when using Zustand stores in React components, the preferred pattern is to use `getState()` for static setters that never change (like `setLogEventNum`) to avoid unnecessary subscriptions, while using hooks for actions that do more than just setting values. All store state variables should be declared at the beginning of the component for consistency and clear dependency overview.
🪛 markdownlint-cli2 (0.17.2)
docs/src/dev-guide/coding-guidelines.md
47-47: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔇 Additional comments (1)
docs/src/dev-guide/coding-guidelines.md (1)
48-52: Naming conventions for Zustand store actions align with best practicesThe new guidelines correctly differentiate
setXfor simple state setters andupdateXfor actions involving extra logic (API calls, multiple state updates), matching existing conventions.
junhaoliao
requested changes
Jun 24, 2025
Member
There was a problem hiding this comment.
Shall we also document these?
(below X represents the store name)
-
Type definitions
DefineXValuesandXActionsfirst, then create the union typeXStateorXSlice. -
Feature-based slices
Slices are encouraged and should be organized by feature rather than by value or action. Avoid grouping by type (e.g., all values / actions in one object) — it's a common anti-pattern. -
Default values
Default state objects should be namedX_DEFAULT. Always declare them with explicit types to support type checking, and spread them into thecreate()function. -
Non-React action and value access
- For actions, whose definitions rarely change, use the non-React approach to retrieve them right before usage:
const {actionName} = useX.getState() - For reading state without subscribing to changes, also use:
const {valueName} = useX.getState()
This avoids unnecessary dependencies inuseEffect,useCallback, etc.
- For actions, whose definitions rarely change, use the non-React approach to retrieve them right before usage:
-
Minimize store interdependencies
To reduce the risk of circular dependencies between stores, always retrieve data from other stores just before it's used.
Essentially, anyone should be able to use the guidelines above, along with the official Zustand documentation, to reproduce or work with the stores in this repo exactly as they are currently used.
Contributor
Author
|
@junhaoliao I think we need to create another title for zustand usages, considering these are more than zustand actions naming. With that said, should we move zustand actions naming under the zustand usages tab? The overall structure would be: NamingWeb worker filesIndex variablesReactZustand usagesType definition...Action naming |
Member
sure |
Member
looks very good. For the header, we can say just "Zustand" instead of "Zustand usages" |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docs/src/dev-guide/coding-guidelines.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/Editor/index.tsx:138-139
Timestamp: 2025-06-01T13:40:12.222Z
Learning: In the yscope-log-viewer codebase, when using Zustand stores in React components, the preferred pattern is to use `getState()` for static setters that never change (like `setLogEventNum`) to avoid unnecessary subscriptions, while using hooks for actions that do more than just setting values. All store state variables should be declared at the beginning of the component for consistency and clear dependency overview.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-10-08T15:52:50.753Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-09-30T20:49:32.508Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#265
File: src/stores/logFileStore.ts:125-125
Timestamp: 2025-05-17T05:13:39.947Z
Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/logFileStore/loadSlice.ts:1-1
Timestamp: 2025-04-28T08:21:28.568Z
Learning: When importing from npm packages, use the public API entrypoint (e.g., `import {StateCreator} from "zustand"`) rather than deep-linking into package internals (e.g., `import {StateCreator} from "zustand/index"`), as internal file structures may change between package versions.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
docs/src/dev-guide/coding-guidelines.md (7)
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/Editor/index.tsx:138-139
Timestamp: 2025-06-01T13:40:12.222Z
Learning: In the yscope-log-viewer codebase, when using Zustand stores in React components, the preferred pattern is to use `getState()` for static setters that never change (like `setLogEventNum`) to avoid unnecessary subscriptions, while using hooks for actions that do more than just setting values. All store state variables should be declared at the beginning of the component for consistency and clear dependency overview.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/logFileStore/loadSlice.ts:1-1
Timestamp: 2025-04-28T08:21:28.568Z
Learning: When importing from npm packages, use the public API entrypoint (e.g., `import {StateCreator} from "zustand"`) rather than deep-linking into package internals (e.g., `import {StateCreator} from "zustand/index"`), as internal file structures may change between package versions.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#265
File: src/stores/logFileStore.ts:125-125
Timestamp: 2025-05-17T05:13:39.947Z
Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
🪛 LanguageTool
docs/src/dev-guide/coding-guidelines.md
[uncategorized] ~75-~75: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...XxxState for store creation. Here's a real world example: ```ts interface LogExportValu...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 markdownlint-cli2 (0.17.2)
docs/src/dev-guide/coding-guidelines.md
56-56: Multiple top-level headings in the same document
null
(MD025, single-title, single-h1)
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
docs/src/dev-guide/coding-guidelines.md (1)
56-65: Demote the “Zustand” heading to comply with MD025 (single-H1 rule)The file already has a top-level “# Coding guidelines” heading. Keeping another
#breaks Markdownlint MD025. Please demote this and its descendants by one level.-# Zustand -## File Naming -## Creation -### Values and actions -### Default values -### Actions Naming -## Feature-based slicing -## Store Usage -### Inside zustand creation +## Zustand +### File Naming +### Creation +#### Values and actions +#### Default values +#### Actions Naming +### Feature-based slicing +### Store Usage +#### Inside zustand creation
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docs/src/dev-guide/coding-guidelines.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/Editor/index.tsx:138-139
Timestamp: 2025-06-01T13:40:12.222Z
Learning: In the yscope-log-viewer codebase, when using Zustand stores in React components, the preferred pattern is to use `getState()` for static setters that never change (like `setLogEventNum`) to avoid unnecessary subscriptions, while using hooks for actions that do more than just setting values. All store state variables should be declared at the beginning of the component for consistency and clear dependency overview.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-10-08T15:52:50.753Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-09-30T20:49:32.508Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#265
File: src/stores/logFileStore.ts:125-125
Timestamp: 2025-05-17T05:13:39.947Z
Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/logFileStore/loadSlice.ts:1-1
Timestamp: 2025-04-28T08:21:28.568Z
Learning: When importing from npm packages, use the public API entrypoint (e.g., `import {StateCreator} from "zustand"`) rather than deep-linking into package internals (e.g., `import {StateCreator} from "zustand/index"`), as internal file structures may change between package versions.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
docs/src/dev-guide/coding-guidelines.md (8)
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/Editor/index.tsx:138-139
Timestamp: 2025-06-01T13:40:12.222Z
Learning: In the yscope-log-viewer codebase, when using Zustand stores in React components, the preferred pattern is to use `getState()` for static setters that never change (like `setLogEventNum`) to avoid unnecessary subscriptions, while using hooks for actions that do more than just setting values. All store state variables should be declared at the beginning of the component for consistency and clear dependency overview.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/logFileStore/loadSlice.ts:1-1
Timestamp: 2025-04-28T08:21:28.568Z
Learning: When importing from npm packages, use the public API entrypoint (e.g., `import {StateCreator} from "zustand"`) rather than deep-linking into package internals (e.g., `import {StateCreator} from "zustand/index"`), as internal file structures may change between package versions.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#265
File: src/stores/logFileStore.ts:125-125
Timestamp: 2025-05-17T05:13:39.947Z
Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/viewStore.ts:108-109
Timestamp: 2025-05-09T01:07:32.803Z
Learning: For the viewStore in the yscope-log-viewer project, it's more appropriate to suppress the max-lines-per-function ESLint rule than to split the store into slices due to the tight coupling and cohesive nature of its functions.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
🪛 LanguageTool
docs/src/dev-guide/coding-guidelines.md
[uncategorized] ~73-~73: The preposition “into” seems more likely in this position than the preposition “in”.
Context: ...variables. We split values and actions in two different interfaces during the def...
(AI_EN_LECTOR_REPLACEMENT_PREPOSITION_IN_INTO)
[uncategorized] ~101-~101: You might be missing the article “the” here.
Context: ...define LOG_EXPORT_STORE_DEFAULT using LogExportValues interface, which can he...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[misspelling] ~115-~115: This word is normally spelled as one.
Context: ... actions in one object) — it's a common anti-pattern. ## Store Usage There are three ways ...
(EN_COMPOUNDS_ANTI_PATTERN)
🪛 markdownlint-cli2 (0.17.2)
docs/src/dev-guide/coding-guidelines.md
56-56: Multiple top-level headings in the same document
null
(MD025, single-title, single-h1)
74-74: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
120-120: Lists should be surrounded by blank lines
null
(MD032, blanks-around-lists)
141-141: Multiple consecutive blank lines
Expected: 1; Actual: 2
(MD012, no-multiple-blanks)
144-144: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
150-150: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
3 tasks
junhaoliao
requested changes
Jun 27, 2025
Member
junhaoliao
left a comment
junhaoliao
left a comment
There was a problem hiding this comment.
for the PR title, how about:
docs: Add Zustand development conventions to coding guidelines (fixes #289).
Henry8192
changed the title
Co-authored-by: Junhao Liao <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docs/src/dev-guide/coding-guidelines.md (1)
56-67: Demote “Zustand” H1 and its children – MD025 blocker still unresolved
# Zustandintroduces a second top-level heading in the document, which violates Markdownlint MD025 and keeps the docs job red. Demote it to H2 and shift every child heading down one level.-# Zustand +## Zustand -## File Naming +### File Naming … -## Store structure +### Store structureMake the same one-level demotion for:
### Type definitions→#### …### Default values→#### …### Action naming→#### …## Feature-based slicing→### …## Store access patterns→### …### Inside store creation→#### …### Inside React components→#### …### Outside React components→#### …This single change removes the MD025 error and unblocks the pipeline.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docs/src/dev-guide/coding-guidelines.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-10-08T15:52:50.753Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-09-30T20:49:32.508Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/Editor/index.tsx:138-139
Timestamp: 2025-06-01T13:40:12.222Z
Learning: In the yscope-log-viewer codebase, when using Zustand stores in React components, the preferred pattern is to use `getState()` for static setters that never change (like `setLogEventNum`) to avoid unnecessary subscriptions, while using hooks for actions that do more than just setting values. All store state variables should be declared at the beginning of the component for consistency and clear dependency overview.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#265
File: src/stores/logFileStore.ts:125-125
Timestamp: 2025-05-17T05:13:39.947Z
Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/logFileStore/loadSlice.ts:1-1
Timestamp: 2025-04-28T08:21:28.568Z
Learning: When importing from npm packages, use the public API entrypoint (e.g., `import {StateCreator} from "zustand"`) rather than deep-linking into package internals (e.g., `import {StateCreator} from "zustand/index"`), as internal file structures may change between package versions.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
docs/src/dev-guide/coding-guidelines.md (20)
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/Editor/index.tsx:138-139
Timestamp: 2025-06-01T13:40:12.222Z
Learning: In the yscope-log-viewer codebase, when using Zustand stores in React components, the preferred pattern is to use `getState()` for static setters that never change (like `setLogEventNum`) to avoid unnecessary subscriptions, while using hooks for actions that do more than just setting values. All store state variables should be declared at the beginning of the component for consistency and clear dependency overview.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/logFileStore/loadSlice.ts:1-1
Timestamp: 2025-04-28T08:21:28.568Z
Learning: When importing from npm packages, use the public API entrypoint (e.g., `import {StateCreator} from "zustand"`) rather than deep-linking into package internals (e.g., `import {StateCreator} from "zustand/index"`), as internal file structures may change between package versions.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#265
File: src/stores/logFileStore.ts:125-125
Timestamp: 2025-05-17T05:13:39.947Z
Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/viewStore.ts:108-109
Timestamp: 2025-05-09T01:07:32.803Z
Learning: For the viewStore in the yscope-log-viewer project, it's more appropriate to suppress the max-lines-per-function ESLint rule than to split the store into slices due to the tight coupling and cohesive nature of its functions.
Learnt from: jackluo923
PR: y-scope/yscope-log-viewer#301
File: tools/deployment/k8s/s3/sample-resource-files/deploy-yscope-log-viewer.yaml:0-0
Timestamp: 2025-05-29T04:48:35.145Z
Learning: In the y-scope/yscope-log-viewer project, extra spaces inside YAML array brackets (e.g., `[ "sh", "-c", "command" ]`) are acceptable and preferred over strict YAML lint compliance.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/SidebarTabs/CustomListItem.tsx:31-31
Timestamp: 2024-10-08T15:52:50.753Z
Learning: In this project, avoid using `React.FC` due to concerns highlighted in https://github.com/facebook/create-react-app/pull/8177.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/SidebarTabs/CustomListItem.tsx:31-31
Timestamp: 2024-09-28T01:15:34.533Z
Learning: In this project, avoid using `React.FC` due to concerns highlighted in https://github.com/facebook/create-react-app/pull/8177.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#129
File: src/components/PopUps/PopUpMessageBox.tsx:119-131
Timestamp: 2024-11-27T16:16:28.173Z
Learning: In `src/components/PopUps/PopUpMessageBox.tsx`, future enhancements may include adding multiple actions to the pop-up message box. Suggestions should account for the possibility of multiple actions rather than just a single one.
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#107
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/Result.tsx:1-7
Timestamp: 2024-10-28T18:40:29.816Z
Learning: In the y-scope/yscope-log-viewer project, importing components via destructuring from '@mui/joy' is acceptable, as the package size impact has been analyzed and found acceptable.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#77
File: new-log-viewer/src/components/StatusBar/LogLevelSelect/index.tsx:99-105
Timestamp: 2024-10-08T15:52:50.753Z
Learning: Using `Object.values(LOG_LEVEL)` returns both the enum's numeric values and key names, so extra filtering is needed to obtain only the numeric values.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#77
File: new-log-viewer/src/components/StatusBar/LogLevelSelect/index.tsx:99-105
Timestamp: 2024-09-26T08:12:44.589Z
Learning: Using `Object.values(LOG_LEVEL)` returns both the enum's numeric values and key names, so extra filtering is needed to obtain only the numeric values.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#94
File: new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx:99-118
Timestamp: 2024-10-19T03:33:29.578Z
Learning: In `new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx`, when using `useEffect` to register window resize event handlers, it's acceptable to have an empty dependency array because the variables and functions used within the effect (`getPanelWidth`, `PANEL_CLIP_THRESHOLD_IN_PIXELS`, and `tabListRef`) are constants or refs declared outside the functional component and do not change.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-09-30T20:49:32.508Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-10-08T15:52:50.753Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/logFileStore.ts:72-79
Timestamp: 2025-05-09T22:39:04.485Z
Learning: In the log viewer application, silently ignoring calls to `handleExportChunk` when `logExportManager` is null is intentional, as the page is reloaded in this scenario anyway.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/contexts/NotificationContextProvider/index.tsx:0-0
Timestamp: 2024-10-11T19:38:16.041Z
Learning: The `NotificationContextProvider` component in the yscope-log-viewer project is always mounted and does not unmount during the application's lifecycle.
🪛 markdownlint-cli2 (0.17.2)
docs/src/dev-guide/coding-guidelines.md
56-56: Multiple top-level headings in the same document
null
(MD025, single-title, single-h1)
72-72: Lists should be surrounded by blank lines
null
(MD032, blanks-around-lists)
🪛 GitHub Actions: docs
docs/src/dev-guide/coding-guidelines.md
[error] 76-76: Sphinx myst-parser highlighting failure: Lexing literal_block resulted in an error at token '#'.
Member
|
@Henry8192 GH doesn't render the suggestion correctly - please apply the rest of the suggestion. #332 (comment) |
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
docs/src/dev-guide/coding-guidelines.md (2)
56-60: Heading level still violates MD025 – see prior feedback
# Zustandintroduces yet another H1. Previous review already requested demoting this to##and cascading the sub-levels—please revisit.
69-76: Blank-line & trailing-space issues break docs build (MD032 / myst crash)The list must be separated from the preceding sentence and the following code fence, and the double-space hard-break after “action functions” is unnecessary.
-Split store types into three interfaces: -* `{Name}Values` - state variables -* `{Name}Actions` - action functions -* `{Name}State` or `{Name}Slice` - combined type +Split store types into three interfaces: + +* `{Name}Values` - state variables +* `{Name}Actions` - action functions +* `{Name}State` or `{Name}Slice` - combined type +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docs/src/dev-guide/coding-guidelines.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-09-30T20:49:32.508Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-10-08T15:52:50.753Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/Editor/index.tsx:138-139
Timestamp: 2025-06-01T13:40:12.222Z
Learning: In the yscope-log-viewer codebase, when using Zustand stores in React components, the preferred pattern is to use `getState()` for static setters that never change (like `setLogEventNum`) to avoid unnecessary subscriptions, while using hooks for actions that do more than just setting values. All store state variables should be declared at the beginning of the component for consistency and clear dependency overview.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#265
File: src/stores/logFileStore.ts:125-125
Timestamp: 2025-05-17T05:13:39.947Z
Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/logFileStore/loadSlice.ts:1-1
Timestamp: 2025-04-28T08:21:28.568Z
Learning: When importing from npm packages, use the public API entrypoint (e.g., `import {StateCreator} from "zustand"`) rather than deep-linking into package internals (e.g., `import {StateCreator} from "zustand/index"`), as internal file structures may change between package versions.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
docs/src/dev-guide/coding-guidelines.md (20)
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/Editor/index.tsx:138-139
Timestamp: 2025-06-01T13:40:12.222Z
Learning: In the yscope-log-viewer codebase, when using Zustand stores in React components, the preferred pattern is to use `getState()` for static setters that never change (like `setLogEventNum`) to avoid unnecessary subscriptions, while using hooks for actions that do more than just setting values. All store state variables should be declared at the beginning of the component for consistency and clear dependency overview.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/logFileStore/loadSlice.ts:1-1
Timestamp: 2025-04-28T08:21:28.568Z
Learning: When importing from npm packages, use the public API entrypoint (e.g., `import {StateCreator} from "zustand"`) rather than deep-linking into package internals (e.g., `import {StateCreator} from "zustand/index"`), as internal file structures may change between package versions.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#265
File: src/stores/logFileStore.ts:125-125
Timestamp: 2025-05-17T05:13:39.947Z
Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/viewStore.ts:108-109
Timestamp: 2025-05-09T01:07:32.803Z
Learning: For the viewStore in the yscope-log-viewer project, it's more appropriate to suppress the max-lines-per-function ESLint rule than to split the store into slices due to the tight coupling and cohesive nature of its functions.
Learnt from: jackluo923
PR: y-scope/yscope-log-viewer#301
File: tools/deployment/k8s/s3/sample-resource-files/deploy-yscope-log-viewer.yaml:0-0
Timestamp: 2025-05-29T04:48:35.145Z
Learning: In the y-scope/yscope-log-viewer project, extra spaces inside YAML array brackets (e.g., `[ "sh", "-c", "command" ]`) are acceptable and preferred over strict YAML lint compliance.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/SidebarTabs/CustomListItem.tsx:31-31
Timestamp: 2024-10-08T15:52:50.753Z
Learning: In this project, avoid using `React.FC` due to concerns highlighted in https://github.com/facebook/create-react-app/pull/8177.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/SidebarTabs/CustomListItem.tsx:31-31
Timestamp: 2024-09-28T01:15:34.533Z
Learning: In this project, avoid using `React.FC` due to concerns highlighted in https://github.com/facebook/create-react-app/pull/8177.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#129
File: src/components/PopUps/PopUpMessageBox.tsx:119-131
Timestamp: 2024-11-27T16:16:28.173Z
Learning: In `src/components/PopUps/PopUpMessageBox.tsx`, future enhancements may include adding multiple actions to the pop-up message box. Suggestions should account for the possibility of multiple actions rather than just a single one.
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#107
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/Result.tsx:1-7
Timestamp: 2024-10-28T18:40:29.816Z
Learning: In the y-scope/yscope-log-viewer project, importing components via destructuring from '@mui/joy' is acceptable, as the package size impact has been analyzed and found acceptable.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#77
File: new-log-viewer/src/components/StatusBar/LogLevelSelect/index.tsx:99-105
Timestamp: 2024-09-26T08:12:44.589Z
Learning: Using `Object.values(LOG_LEVEL)` returns both the enum's numeric values and key names, so extra filtering is needed to obtain only the numeric values.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#77
File: new-log-viewer/src/components/StatusBar/LogLevelSelect/index.tsx:99-105
Timestamp: 2024-10-08T15:52:50.753Z
Learning: Using `Object.values(LOG_LEVEL)` returns both the enum's numeric values and key names, so extra filtering is needed to obtain only the numeric values.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#94
File: new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx:99-118
Timestamp: 2024-10-19T03:33:29.578Z
Learning: In `new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx`, when using `useEffect` to register window resize event handlers, it's acceptable to have an empty dependency array because the variables and functions used within the effect (`getPanelWidth`, `PANEL_CLIP_THRESHOLD_IN_PIXELS`, and `tabListRef`) are constants or refs declared outside the functional component and do not change.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-09-30T20:49:32.508Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-10-08T15:52:50.753Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/logFileStore.ts:72-79
Timestamp: 2025-05-09T22:39:04.485Z
Learning: In the log viewer application, silently ignoring calls to `handleExportChunk` when `logExportManager` is null is intentional, as the page is reloaded in this scenario anyway.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/contexts/NotificationContextProvider/index.tsx:0-0
Timestamp: 2024-10-11T19:38:16.041Z
Learning: The `NotificationContextProvider` component in the yscope-log-viewer project is always mounted and does not unmount during the application's lifecycle.
🪛 LanguageTool
docs/src/dev-guide/coding-guidelines.md
[misspelling] ~123-~123: This word is normally spelled as one.
Context: ... actions in one object) - it's a common anti-pattern. ## Store access patterns ### Inside ...
(EN_COMPOUNDS_ANTI_PATTERN)
[uncategorized] ~180-~180: Possible missing comma found.
Context: ...act components Always use non-reactive access since reactive subscriptions do not wor...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 markdownlint-cli2 (0.17.2)
docs/src/dev-guide/coding-guidelines.md
56-56: Multiple top-level headings in the same document
null
(MD025, single-title, single-h1)
72-72: Lists should be surrounded by blank lines
null
(MD032, blanks-around-lists)
148-148: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
160-160: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
170-170: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
🔇 Additional comments (1)
docs/src/dev-guide/coding-guidelines.md (1)
122-123: Consider using the standard spelling “antipattern”Minor wording nitpick for consistency.
-values / actions in one object) - it's a common anti-pattern. +values / actions in one object) – it's a common antipattern.⛔ Skipped due to learnings
Learnt from: junhaoliao PR: y-scope/yscope-log-viewer#101 File: README.md:40-47 Timestamp: 2024-10-24T01:18:43.327Z Learning: Preference for precise language over simplified wording in documentation.Learnt from: junhaoliao PR: y-scope/yscope-log-viewer#265 File: src/stores/logFileStore.ts:125-125 Timestamp: 2025-05-17T05:13:39.947Z Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.Learnt from: zzxthehappiest PR: y-scope/yscope-log-viewer#286 File: src/components/Editor/index.tsx:138-139 Timestamp: 2025-06-01T13:40:12.222Z Learning: In the yscope-log-viewer codebase, when using Zustand stores in React components, the preferred pattern is to use `getState()` for static setters that never change (like `setLogEventNum`) to avoid unnecessary subscriptions, while using hooks for actions that do more than just setting values. All store state variables should be declared at the beginning of the component for consistency and clear dependency overview.Learnt from: junhaoliao PR: y-scope/yscope-log-viewer#286 File: src/stores/viewStore.ts:46-46 Timestamp: 2025-05-21T20:08:19.249Z Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).Learnt from: junhaoliao PR: y-scope/yscope-log-viewer#286 File: src/stores/viewStore.ts:46-46 Timestamp: 2025-05-21T20:08:19.249Z Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).Learnt from: junhaoliao PR: y-scope/yscope-log-viewer#224 File: src/contexts/states/viewStore.ts:108-109 Timestamp: 2025-05-09T01:07:32.803Z Learning: For the viewStore in the yscope-log-viewer project, it's more appropriate to suppress the max-lines-per-function ESLint rule than to split the store into slices due to the tight coupling and cohesive nature of its functions.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
docs/src/dev-guide/coding-guidelines.md (3)
56-60: Demote the heading & keep a single H1
# Zustandis the fourth top-level heading in this document and still violates MD025. Please demote it to## Zustand(and shift all nested headings down one level) as previously suggested.
132-141: Parameter order inconsistent with other examples & Zustand docsThis snippet uses
(get, set)whereas every other example (and the official API) uses(set, get). Flip the order for consistency.-const useLogExportStore = create<LogExportState>((get, set) => ({ +const useLogExportStore = create<LogExportState>((set, get) => ({
182-184: Missing comma after introductory clauseAdd a comma for clarity.
-Always use non-reactive access since reactive subscriptions do not work outside components. +Always use non-reactive access, since reactive subscriptions do not work outside components.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docs/src/dev-guide/coding-guidelines.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-09-30T20:49:32.508Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-10-08T15:52:50.753Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/Editor/index.tsx:138-139
Timestamp: 2025-06-01T13:40:12.222Z
Learning: In the yscope-log-viewer codebase, when using Zustand stores in React components, the preferred pattern is to use `getState()` for static setters that never change (like `setLogEventNum`) to avoid unnecessary subscriptions, while using hooks for actions that do more than just setting values. All store state variables should be declared at the beginning of the component for consistency and clear dependency overview.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#265
File: src/stores/logFileStore.ts:125-125
Timestamp: 2025-05-17T05:13:39.947Z
Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/logFileStore/loadSlice.ts:1-1
Timestamp: 2025-04-28T08:21:28.568Z
Learning: When importing from npm packages, use the public API entrypoint (e.g., `import {StateCreator} from "zustand"`) rather than deep-linking into package internals (e.g., `import {StateCreator} from "zustand/index"`), as internal file structures may change between package versions.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
docs/src/dev-guide/coding-guidelines.md (24)
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/Editor/index.tsx:138-139
Timestamp: 2025-06-01T13:40:12.222Z
Learning: In the yscope-log-viewer codebase, when using Zustand stores in React components, the preferred pattern is to use `getState()` for static setters that never change (like `setLogEventNum`) to avoid unnecessary subscriptions, while using hooks for actions that do more than just setting values. All store state variables should be declared at the beginning of the component for consistency and clear dependency overview.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/logFileStore/loadSlice.ts:1-1
Timestamp: 2025-04-28T08:21:28.568Z
Learning: When importing from npm packages, use the public API entrypoint (e.g., `import {StateCreator} from "zustand"`) rather than deep-linking into package internals (e.g., `import {StateCreator} from "zustand/index"`), as internal file structures may change between package versions.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#265
File: src/stores/logFileStore.ts:125-125
Timestamp: 2025-05-17T05:13:39.947Z
Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/viewStore.ts:108-109
Timestamp: 2025-05-09T01:07:32.803Z
Learning: For the viewStore in the yscope-log-viewer project, it's more appropriate to suppress the max-lines-per-function ESLint rule than to split the store into slices due to the tight coupling and cohesive nature of its functions.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
Learnt from: jackluo923
PR: y-scope/yscope-log-viewer#301
File: tools/deployment/k8s/s3/sample-resource-files/deploy-yscope-log-viewer.yaml:0-0
Timestamp: 2025-05-29T04:48:35.145Z
Learning: In the y-scope/yscope-log-viewer project, extra spaces inside YAML array brackets (e.g., `[ "sh", "-c", "command" ]`) are acceptable and preferred over strict YAML lint compliance.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/SidebarTabs/CustomListItem.tsx:31-31
Timestamp: 2024-10-08T15:52:50.753Z
Learning: In this project, avoid using `React.FC` due to concerns highlighted in https://github.com/facebook/create-react-app/pull/8177.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/SidebarTabs/CustomListItem.tsx:31-31
Timestamp: 2024-09-28T01:15:34.533Z
Learning: In this project, avoid using `React.FC` due to concerns highlighted in https://github.com/facebook/create-react-app/pull/8177.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#129
File: src/components/PopUps/PopUpMessageBox.tsx:119-131
Timestamp: 2024-11-27T16:16:28.173Z
Learning: In `src/components/PopUps/PopUpMessageBox.tsx`, future enhancements may include adding multiple actions to the pop-up message box. Suggestions should account for the possibility of multiple actions rather than just a single one.
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#107
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/Result.tsx:1-7
Timestamp: 2024-10-28T18:40:29.816Z
Learning: In the y-scope/yscope-log-viewer project, importing components via destructuring from '@mui/joy' is acceptable, as the package size impact has been analyzed and found acceptable.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#77
File: new-log-viewer/src/components/StatusBar/LogLevelSelect/index.tsx:99-105
Timestamp: 2024-09-26T08:12:44.589Z
Learning: Using `Object.values(LOG_LEVEL)` returns both the enum's numeric values and key names, so extra filtering is needed to obtain only the numeric values.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#77
File: new-log-viewer/src/components/StatusBar/LogLevelSelect/index.tsx:99-105
Timestamp: 2024-10-08T15:52:50.753Z
Learning: Using `Object.values(LOG_LEVEL)` returns both the enum's numeric values and key names, so extra filtering is needed to obtain only the numeric values.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#94
File: new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx:99-118
Timestamp: 2024-10-19T03:33:29.578Z
Learning: In `new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx`, when using `useEffect` to register window resize event handlers, it's acceptable to have an empty dependency array because the variables and functions used within the effect (`getPanelWidth`, `PANEL_CLIP_THRESHOLD_IN_PIXELS`, and `tabListRef`) are constants or refs declared outside the functional component and do not change.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-09-30T20:49:32.508Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-10-08T15:52:50.753Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/logFileStore.ts:72-79
Timestamp: 2025-05-09T22:39:04.485Z
Learning: In the log viewer application, silently ignoring calls to `handleExportChunk` when `logExportManager` is null is intentional, as the page is reloaded in this scenario anyway.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/contexts/NotificationContextProvider/index.tsx:0-0
Timestamp: 2024-10-11T19:38:16.041Z
Learning: The `NotificationContextProvider` component in the yscope-log-viewer project is always mounted and does not unmount during the application's lifecycle.
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#76
File: new-log-viewer/src/utils/actions.ts:105-133
Timestamp: 2024-09-25T21:12:28.732Z
Learning: In the `actions.ts` file, the `PAGE_TOP` and `PAGE_BOTTOM` actions are not intended to be handled in the `getPageNumCursorArgs` function.
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#76
File: new-log-viewer/src/utils/actions.ts:105-133
Timestamp: 2024-10-08T15:52:50.753Z
Learning: In the `actions.ts` file, the `PAGE_TOP` and `PAGE_BOTTOM` actions are not intended to be handled in the `getPageNumCursorArgs` function.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/contexts/NotificationContextProvider/index.tsx:0-0
Timestamp: 2024-10-11T19:37:56.597Z
Learning: In our TypeScript codebase with strong typing, variables are strictly typed and cannot be `undefined` where `null` is expected, so checking for `undefined` in null checks is unnecessary.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#83
File: new-log-viewer/src/components/theme.tsx:59-61
Timestamp: 2024-10-18T01:13:02.946Z
Learning: In 'new-log-viewer/src/components/theme.tsx', a 1px focus ring is acceptable due to sufficient color contrast as per project preferences.
🪛 LanguageTool
docs/src/dev-guide/coding-guidelines.md
[misspelling] ~124-~124: This word is normally spelled as one.
Context: ... actions in one object) - it's a common anti-pattern. ## Store access patterns ### Inside ...
(EN_COMPOUNDS_ANTI_PATTERN)
[uncategorized] ~184-~184: Possible missing comma found.
Context: ...act components Always use non-reactive access since reactive subscriptions do not wor...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 markdownlint-cli2 (0.17.2)
docs/src/dev-guide/coding-guidelines.md
56-56: Multiple top-level headings in the same document
null
(MD025, single-title, single-h1)
🔇 Additional comments (1)
docs/src/dev-guide/coding-guidelines.md (1)
123-124: Spell “antipattern” as one word
anti-patternis normally written without the hyphen.-values / actions in one object) - it's a common anti-pattern. +values / actions in one object) – it's a common antipattern.⛔ Skipped due to learnings
Learnt from: junhaoliao PR: y-scope/yscope-log-viewer#265 File: src/stores/logFileStore.ts:125-125 Timestamp: 2025-05-17T05:13:39.947Z Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.Learnt from: zzxthehappiest PR: y-scope/yscope-log-viewer#286 File: src/components/Editor/index.tsx:138-139 Timestamp: 2025-06-01T13:40:12.222Z Learning: In the yscope-log-viewer codebase, when using Zustand stores in React components, the preferred pattern is to use `getState()` for static setters that never change (like `setLogEventNum`) to avoid unnecessary subscriptions, while using hooks for actions that do more than just setting values. All store state variables should be declared at the beginning of the component for consistency and clear dependency overview.Learnt from: junhaoliao PR: y-scope/yscope-log-viewer#286 File: src/stores/viewStore.ts:46-46 Timestamp: 2025-05-21T20:08:19.249Z Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).Learnt from: junhaoliao PR: y-scope/yscope-log-viewer#286 File: src/stores/viewStore.ts:46-46 Timestamp: 2025-05-21T20:08:19.249Z Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).Learnt from: junhaoliao PR: y-scope/yscope-log-viewer#224 File: src/contexts/states/viewStore.ts:108-109 Timestamp: 2025-05-09T01:07:32.803Z Learning: For the viewStore in the yscope-log-viewer project, it's more appropriate to suppress the max-lines-per-function ESLint rule than to split the store into slices due to the tight coupling and cohesive nature of its functions.
junhaoliao
requested changes
Jun 30, 2025
Member
junhaoliao
left a comment
junhaoliao
left a comment
There was a problem hiding this comment.
my final batch of suggestions -
- one of the examples is a bit confusing. can we replace it with a better one?
- let's add caption to the code blocks. and highlight lines of code if necessary
Co-authored-by: Junhao Liao <[email protected]>
junhaoliao
reviewed
Jun 30, 2025
Co-authored-by: Junhao Liao <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
docs/src/dev-guide/coding-guidelines.md (2)
56-66: Demote the “Zustand” heading to fix MD025 (single-H1 violation)There are already top-level headings (
# Coding guidelines,# Web workers,# Naming,# React). Keeping “# Zustand” creates a second-level hierarchy clash and continues to break the docs build.-# Zustand +## ZustandAll sub-headings under this section must be shifted down by one level accordingly (
##→###,###→####, etc.).
200-203: Insert comma for readability-Always use non-reactive access since reactive subscriptions do not work outside components. +Always use non-reactive access, since reactive subscriptions do not work outside components.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docs/src/dev-guide/coding-guidelines.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-10-08T15:52:50.753Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-09-30T20:49:32.508Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/Editor/index.tsx:138-139
Timestamp: 2025-06-01T13:40:12.222Z
Learning: In the yscope-log-viewer codebase, when using Zustand stores in React components, the preferred pattern is to use `getState()` for static setters that never change (like `setLogEventNum`) to avoid unnecessary subscriptions, while using hooks for actions that do more than just setting values. All store state variables should be declared at the beginning of the component for consistency and clear dependency overview.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#265
File: src/stores/logFileStore.ts:125-125
Timestamp: 2025-05-17T05:13:39.947Z
Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/logFileStore/loadSlice.ts:1-1
Timestamp: 2025-04-28T08:21:28.568Z
Learning: When importing from npm packages, use the public API entrypoint (e.g., `import {StateCreator} from "zustand"`) rather than deep-linking into package internals (e.g., `import {StateCreator} from "zustand/index"`), as internal file structures may change between package versions.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
docs/src/dev-guide/coding-guidelines.md (27)
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/Editor/index.tsx:138-139
Timestamp: 2025-06-01T13:40:12.222Z
Learning: In the yscope-log-viewer codebase, when using Zustand stores in React components, the preferred pattern is to use `getState()` for static setters that never change (like `setLogEventNum`) to avoid unnecessary subscriptions, while using hooks for actions that do more than just setting values. All store state variables should be declared at the beginning of the component for consistency and clear dependency overview.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/logFileStore/loadSlice.ts:1-1
Timestamp: 2025-04-28T08:21:28.568Z
Learning: When importing from npm packages, use the public API entrypoint (e.g., `import {StateCreator} from "zustand"`) rather than deep-linking into package internals (e.g., `import {StateCreator} from "zustand/index"`), as internal file structures may change between package versions.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#265
File: src/stores/logFileStore.ts:125-125
Timestamp: 2025-05-17T05:13:39.947Z
Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/viewStore.ts:108-109
Timestamp: 2025-05-09T01:07:32.803Z
Learning: For the viewStore in the yscope-log-viewer project, it's more appropriate to suppress the max-lines-per-function ESLint rule than to split the store into slices due to the tight coupling and cohesive nature of its functions.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
Learnt from: jackluo923
PR: y-scope/yscope-log-viewer#301
File: tools/deployment/k8s/s3/sample-resource-files/deploy-yscope-log-viewer.yaml:0-0
Timestamp: 2025-05-29T04:48:35.145Z
Learning: In the y-scope/yscope-log-viewer project, extra spaces inside YAML array brackets (e.g., `[ "sh", "-c", "command" ]`) are acceptable and preferred over strict YAML lint compliance.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/SidebarTabs/CustomListItem.tsx:31-31
Timestamp: 2024-09-28T01:15:34.533Z
Learning: In this project, avoid using `React.FC` due to concerns highlighted in https://github.com/facebook/create-react-app/pull/8177.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/SidebarTabs/CustomListItem.tsx:31-31
Timestamp: 2024-10-08T15:52:50.753Z
Learning: In this project, avoid using `React.FC` due to concerns highlighted in https://github.com/facebook/create-react-app/pull/8177.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#129
File: src/components/PopUps/PopUpMessageBox.tsx:119-131
Timestamp: 2024-11-27T16:16:28.173Z
Learning: In `src/components/PopUps/PopUpMessageBox.tsx`, future enhancements may include adding multiple actions to the pop-up message box. Suggestions should account for the possibility of multiple actions rather than just a single one.
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#107
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/Result.tsx:1-7
Timestamp: 2024-10-28T18:40:29.816Z
Learning: In the y-scope/yscope-log-viewer project, importing components via destructuring from '@mui/joy' is acceptable, as the package size impact has been analyzed and found acceptable.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#94
File: new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx:99-118
Timestamp: 2024-10-19T03:33:29.578Z
Learning: In `new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx`, when using `useEffect` to register window resize event handlers, it's acceptable to have an empty dependency array because the variables and functions used within the effect (`getPanelWidth`, `PANEL_CLIP_THRESHOLD_IN_PIXELS`, and `tabListRef`) are constants or refs declared outside the functional component and do not change.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#77
File: new-log-viewer/src/components/StatusBar/LogLevelSelect/index.tsx:99-105
Timestamp: 2024-09-26T08:12:44.589Z
Learning: Using `Object.values(LOG_LEVEL)` returns both the enum's numeric values and key names, so extra filtering is needed to obtain only the numeric values.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#77
File: new-log-viewer/src/components/StatusBar/LogLevelSelect/index.tsx:99-105
Timestamp: 2024-10-08T15:52:50.753Z
Learning: Using `Object.values(LOG_LEVEL)` returns both the enum's numeric values and key names, so extra filtering is needed to obtain only the numeric values.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-09-30T20:49:32.508Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-10-08T15:52:50.753Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/logFileStore.ts:72-79
Timestamp: 2025-05-09T22:39:04.485Z
Learning: In the log viewer application, silently ignoring calls to `handleExportChunk` when `logExportManager` is null is intentional, as the page is reloaded in this scenario anyway.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/contexts/NotificationContextProvider/index.tsx:0-0
Timestamp: 2024-10-11T19:38:16.041Z
Learning: The `NotificationContextProvider` component in the yscope-log-viewer project is always mounted and does not unmount during the application's lifecycle.
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#76
File: new-log-viewer/src/utils/actions.ts:105-133
Timestamp: 2024-09-25T21:12:28.732Z
Learning: In the `actions.ts` file, the `PAGE_TOP` and `PAGE_BOTTOM` actions are not intended to be handled in the `getPageNumCursorArgs` function.
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#76
File: new-log-viewer/src/utils/actions.ts:105-133
Timestamp: 2024-10-08T15:52:50.753Z
Learning: In the `actions.ts` file, the `PAGE_TOP` and `PAGE_BOTTOM` actions are not intended to be handled in the `getPageNumCursorArgs` function.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/contexts/NotificationContextProvider/index.tsx:0-0
Timestamp: 2024-10-11T19:37:56.597Z
Learning: In our TypeScript codebase with strong typing, variables are strictly typed and cannot be `undefined` where `null` is expected, so checking for `undefined` in null checks is unnecessary.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#83
File: new-log-viewer/src/components/theme.tsx:59-61
Timestamp: 2024-10-18T01:13:02.946Z
Learning: In 'new-log-viewer/src/components/theme.tsx', a 1px focus ring is acceptable due to sufficient color contrast as per project preferences.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#101
File: docs/dev-guide/validation.md:6-15
Timestamp: 2024-10-24T01:17:07.255Z
Learning: In `docs/dev-guide/validation.md`, the feature "Exporting all logs to a file" includes the download functionality.
Learnt from: jackluo923
PR: y-scope/yscope-log-viewer#301
File: tools/deployment/k8s/s3/sample-resource-files/aws-credential.yaml:8-8
Timestamp: 2025-05-26T05:24:17.896Z
Learning: For the y-scope/yscope-log-viewer project, sample and reference implementation files (like those in tools/deployment/k8s/s3/sample-resource-files/) are intended to be basic examples rather than production-ready configurations. The maintainer jackluo923 prefers to keep these simple and functional rather than incorporating advanced security practices.
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#196
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx:55-58
Timestamp: 2025-03-03T18:10:02.451Z
Learning: In the y-scope/yscope-log-viewer repository, the team prefers using the `!` operator to flip boolean values (e.g., `!isRegex`) rather than the pattern `false == <expression>` that's mentioned in the coding guidelines.
🪛 LanguageTool
docs/src/dev-guide/coding-guidelines.md
[uncategorized] ~77-~77: Possible missing article found.
Context: ...te- union of values and actions When store gets too large, we slice{Name}State` ...
(AI_HYDRA_LEO_MISSING_A)
[misspelling] ~133-~133: This word is normally spelled as one.
Context: ... actions in one object) - it's a common anti-pattern. ## Store access patterns ### Inside ...
(EN_COMPOUNDS_ANTI_PATTERN)
[uncategorized] ~202-~202: Possible missing comma found.
Context: ...act components Always use non-reactive access since reactive subscriptions do not wor...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 markdownlint-cli2 (0.17.2)
docs/src/dev-guide/coding-guidelines.md
56-56: Multiple top-level headings in the same document
(MD025, single-title, single-h1)
80-80: Multiple consecutive blank lines
Expected: 1; Actual: 2
(MD012, no-multiple-blanks)
junhaoliao
reviewed
Jul 8, 2025
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
docs/src/dev-guide/coding-guidelines.md (1)
56-60: Duplicate: multiple#-level headings
# Zustandcreates a second top-level heading and still violates MD025. Please demote it to## Zustandand shift all subordinate headings down one level as already suggested.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docs/src/dev-guide/coding-guidelines.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-09-30T20:49:32.508Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-10-08T15:52:50.753Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/Editor/index.tsx:138-139
Timestamp: 2025-06-01T13:40:12.222Z
Learning: In the yscope-log-viewer codebase, when using Zustand stores in React components, the preferred pattern is to use `getState()` for static setters that never change (like `setLogEventNum`) to avoid unnecessary subscriptions, while using hooks for actions that do more than just setting values. All store state variables should be declared at the beginning of the component for consistency and clear dependency overview.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/ResizeHandle.tsx:77-82
Timestamp: 2024-09-28T02:32:08.882Z
Learning: Accessibility improvements, such as adding ARIA attributes to components like `ResizeHandle`, require broader discussion and are considered out of scope for small PRs.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/ResizeHandle.tsx:77-82
Timestamp: 2024-10-08T15:52:50.753Z
Learning: Accessibility improvements, such as adding ARIA attributes to components like `ResizeHandle`, require broader discussion and are considered out of scope for small PRs.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#265
File: src/stores/logFileStore.ts:125-125
Timestamp: 2025-05-17T05:13:39.947Z
Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/logFileStore/loadSlice.ts:1-1
Timestamp: 2025-04-28T08:21:28.568Z
Learning: When importing from npm packages, use the public API entrypoint (e.g., `import {StateCreator} from "zustand"`) rather than deep-linking into package internals (e.g., `import {StateCreator} from "zustand/index"`), as internal file structures may change between package versions.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
docs/src/dev-guide/coding-guidelines.md (27)
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/Editor/index.tsx:138-139
Timestamp: 2025-06-01T13:40:12.222Z
Learning: In the yscope-log-viewer codebase, when using Zustand stores in React components, the preferred pattern is to use `getState()` for static setters that never change (like `setLogEventNum`) to avoid unnecessary subscriptions, while using hooks for actions that do more than just setting values. All store state variables should be declared at the beginning of the component for consistency and clear dependency overview.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/logFileStore/loadSlice.ts:1-1
Timestamp: 2025-04-28T08:21:28.568Z
Learning: When importing from npm packages, use the public API entrypoint (e.g., `import {StateCreator} from "zustand"`) rather than deep-linking into package internals (e.g., `import {StateCreator} from "zustand/index"`), as internal file structures may change between package versions.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#265
File: src/stores/logFileStore.ts:125-125
Timestamp: 2025-05-17T05:13:39.947Z
Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/viewStore.ts:108-109
Timestamp: 2025-05-09T01:07:32.803Z
Learning: For the viewStore in the yscope-log-viewer project, it's more appropriate to suppress the max-lines-per-function ESLint rule than to split the store into slices due to the tight coupling and cohesive nature of its functions.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/stores/uiStore.ts:19-19
Timestamp: 2025-05-22T00:14:36.349Z
Learning: Type definitions in the codebase should use PascalCase naming convention for consistency.
Learnt from: jackluo923
PR: y-scope/yscope-log-viewer#301
File: tools/deployment/k8s/s3/sample-resource-files/deploy-yscope-log-viewer.yaml:0-0
Timestamp: 2025-05-29T04:48:35.145Z
Learning: In the y-scope/yscope-log-viewer project, extra spaces inside YAML array brackets (e.g., `[ "sh", "-c", "command" ]`) are acceptable and preferred over strict YAML lint compliance.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/SidebarTabs/CustomListItem.tsx:31-31
Timestamp: 2024-10-08T15:52:50.753Z
Learning: In this project, avoid using `React.FC` due to concerns highlighted in https://github.com/facebook/create-react-app/pull/8177.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/SidebarTabs/CustomListItem.tsx:31-31
Timestamp: 2024-09-28T01:15:34.533Z
Learning: In this project, avoid using `React.FC` due to concerns highlighted in https://github.com/facebook/create-react-app/pull/8177.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#129
File: src/components/PopUps/PopUpMessageBox.tsx:119-131
Timestamp: 2024-11-27T16:16:28.173Z
Learning: In `src/components/PopUps/PopUpMessageBox.tsx`, future enhancements may include adding multiple actions to the pop-up message box. Suggestions should account for the possibility of multiple actions rather than just a single one.
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#107
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/Result.tsx:1-7
Timestamp: 2024-10-28T18:40:29.816Z
Learning: In the y-scope/yscope-log-viewer project, importing components via destructuring from '@mui/joy' is acceptable, as the package size impact has been analyzed and found acceptable.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#77
File: new-log-viewer/src/components/StatusBar/LogLevelSelect/index.tsx:99-105
Timestamp: 2024-10-08T15:52:50.753Z
Learning: Using `Object.values(LOG_LEVEL)` returns both the enum's numeric values and key names, so extra filtering is needed to obtain only the numeric values.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#77
File: new-log-viewer/src/components/StatusBar/LogLevelSelect/index.tsx:99-105
Timestamp: 2024-09-26T08:12:44.589Z
Learning: Using `Object.values(LOG_LEVEL)` returns both the enum's numeric values and key names, so extra filtering is needed to obtain only the numeric values.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#94
File: new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx:99-118
Timestamp: 2024-10-19T03:33:29.578Z
Learning: In `new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx`, when using `useEffect` to register window resize event handlers, it's acceptable to have an empty dependency array because the variables and functions used within the effect (`getPanelWidth`, `PANEL_CLIP_THRESHOLD_IN_PIXELS`, and `tabListRef`) are constants or refs declared outside the functional component and do not change.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-09-30T20:49:32.508Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-10-08T15:52:50.753Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/logFileStore.ts:72-79
Timestamp: 2025-05-09T22:39:04.485Z
Learning: In the log viewer application, silently ignoring calls to `handleExportChunk` when `logExportManager` is null is intentional, as the page is reloaded in this scenario anyway.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/contexts/NotificationContextProvider/index.tsx:0-0
Timestamp: 2024-10-11T19:38:16.041Z
Learning: The `NotificationContextProvider` component in the yscope-log-viewer project is always mounted and does not unmount during the application's lifecycle.
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#76
File: new-log-viewer/src/utils/actions.ts:105-133
Timestamp: 2024-10-08T15:52:50.753Z
Learning: In the `actions.ts` file, the `PAGE_TOP` and `PAGE_BOTTOM` actions are not intended to be handled in the `getPageNumCursorArgs` function.
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#76
File: new-log-viewer/src/utils/actions.ts:105-133
Timestamp: 2024-09-25T21:12:28.732Z
Learning: In the `actions.ts` file, the `PAGE_TOP` and `PAGE_BOTTOM` actions are not intended to be handled in the `getPageNumCursorArgs` function.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/contexts/NotificationContextProvider/index.tsx:0-0
Timestamp: 2024-10-11T19:37:56.597Z
Learning: In our TypeScript codebase with strong typing, variables are strictly typed and cannot be `undefined` where `null` is expected, so checking for `undefined` in null checks is unnecessary.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#83
File: new-log-viewer/src/components/theme.tsx:59-61
Timestamp: 2024-10-18T01:13:02.946Z
Learning: In 'new-log-viewer/src/components/theme.tsx', a 1px focus ring is acceptable due to sufficient color contrast as per project preferences.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#101
File: docs/dev-guide/validation.md:6-15
Timestamp: 2024-10-24T01:17:07.255Z
Learning: In `docs/dev-guide/validation.md`, the feature "Exporting all logs to a file" includes the download functionality.
Learnt from: jackluo923
PR: y-scope/yscope-log-viewer#301
File: tools/deployment/k8s/s3/sample-resource-files/aws-credential.yaml:8-8
Timestamp: 2025-05-26T05:24:17.896Z
Learning: For the y-scope/yscope-log-viewer project, sample and reference implementation files (like those in tools/deployment/k8s/s3/sample-resource-files/) are intended to be basic examples rather than production-ready configurations. The maintainer jackluo923 prefers to keep these simple and functional rather than incorporating advanced security practices.
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#196
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx:55-58
Timestamp: 2025-03-03T18:10:02.451Z
Learning: In the y-scope/yscope-log-viewer repository, the team prefers using the `!` operator to flip boolean values (e.g., `!isRegex`) rather than the pattern `false == <expression>` that's mentioned in the coding guidelines.
🪛 LanguageTool
docs/src/dev-guide/coding-guidelines.md
[misspelling] ~132-~132: This word is normally spelled as one.
Context: ... actions in one object) - it's a common anti-pattern. ## Store access patterns ### Inside ...
(EN_COMPOUNDS_ANTI_PATTERN)
[uncategorized] ~201-~201: Possible missing comma found.
Context: ...act components Always use non-reactive access since reactive subscriptions do not wor...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 markdownlint-cli2 (0.17.2)
docs/src/dev-guide/coding-guidelines.md
56-56: Multiple top-level headings in the same document
(MD025, single-title, single-h1)
⏰ 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: build (macos-latest)
Henry8192
commented
Jul 10, 2025
Member
|
@kirkrodrigues i think this is good to go. let me know if you want to take a final look or we can merge now |
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (3)
docs/src/dev-guide/coding-guidelines.md (3)
56-66: Demote “Zustand” to H2 to fix MD025; shift child headings down one levelYou already have an H1 at Line 1. Make “Zustand” an H2 and demote its subsections accordingly.
-# Zustand +## Zustand -## File Naming +### File Naming -## Store structure +### Store structure -### Type definitions +#### Type definitions -### Default values +#### Default values -### Action naming +#### Action naming -## Feature-based slicing +### Feature-based slicing -## Store access patterns +### Store access patterns -### Inside store creation +#### Inside store creation -### Inside React components +#### Inside React components -#### State values +##### State values -#### Actions +##### Actions -### Outside React components +#### Outside React components
212-214: Comma improves readabilityAdd a comma after the introductory clause.
-Always use non-reactive access since reactive subscriptions do not work outside components. +Always use non-reactive access, since reactive subscriptions do not work outside components.
133-146: Clarify “too large” threshold for slicingMake the trigger explicit (as discussed): reference the ESLint max-lines-per-function limit or a concrete heuristic.
-When a Zustand store grows too large, split it into slices based on features such as functional -areas. +When a Zustand store grows too large, split it into slices based on features such as functional +areas. For example, if the store exceeds the ESLint `max-lines-per-function` limit (120 lines) or +accumulates more than ~5–7 values and actions in a single unit, prefer slicing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docs/src/dev-guide/coding-guidelines.md(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/Editor/index.tsx:138-139
Timestamp: 2025-06-01T13:40:12.222Z
Learning: In the yscope-log-viewer codebase, when using Zustand stores in React components, the preferred pattern is to use `getState()` for static setters that never change (like `setLogEventNum`) to avoid unnecessary subscriptions, while using hooks for actions that do more than just setting values. All store state variables should be declared at the beginning of the component for consistency and clear dependency overview.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
📚 Learning: 2025-06-01T13:40:12.222Z
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/Editor/index.tsx:138-139
Timestamp: 2025-06-01T13:40:12.222Z
Learning: In the yscope-log-viewer codebase, when using Zustand stores in React components, the preferred pattern is to use `getState()` for static setters that never change (like `setLogEventNum`) to avoid unnecessary subscriptions, while using hooks for actions that do more than just setting values. All store state variables should be declared at the beginning of the component for consistency and clear dependency overview.
Applied to files:
docs/src/dev-guide/coding-guidelines.md
📚 Learning: 2025-05-21T20:08:19.249Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Applied to files:
docs/src/dev-guide/coding-guidelines.md
📚 Learning: 2025-05-17T05:13:39.947Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#265
File: src/stores/logFileStore.ts:125-125
Timestamp: 2025-05-17T05:13:39.947Z
Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.
Applied to files:
docs/src/dev-guide/coding-guidelines.md
📚 Learning: 2024-10-28T18:40:29.816Z
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#107
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/Result.tsx:1-7
Timestamp: 2024-10-28T18:40:29.816Z
Learning: In the y-scope/yscope-log-viewer project, importing components via destructuring from 'mui/joy' is acceptable, as the package size impact has been analyzed and found acceptable.
Applied to files:
docs/src/dev-guide/coding-guidelines.md
📚 Learning: 2025-09-03T18:22:30.594Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#371
File: src/components/MenuBar/index.tsx:42-46
Timestamp: 2025-09-03T18:22:30.594Z
Learning: The development team has discussed that fire-and-forget async IIFE patterns (like `(async () => { ... })().catch(handleErrorWithNotification)`) don't cause synchronization issues in the yscope-log-viewer codebase due to serialized service worker communications, but they acknowledge it's still an error-prone pattern that should be refactored for better code clarity and maintainability.
Applied to files:
docs/src/dev-guide/coding-guidelines.md
📚 Learning: 2025-05-09T01:07:32.803Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/viewStore.ts:108-109
Timestamp: 2025-05-09T01:07:32.803Z
Learning: For the viewStore in the yscope-log-viewer project, it's more appropriate to suppress the max-lines-per-function ESLint rule than to split the store into slices due to the tight coupling and cohesive nature of its functions.
Applied to files:
docs/src/dev-guide/coding-guidelines.md
📚 Learning: 2025-04-28T08:21:28.568Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/logFileStore/loadSlice.ts:1-1
Timestamp: 2025-04-28T08:21:28.568Z
Learning: When importing from npm packages, use the public API entrypoint (e.g., `import {StateCreator} from "zustand"`) rather than deep-linking into package internals (e.g., `import {StateCreator} from "zustand/index"`), as internal file structures may change between package versions.
Applied to files:
docs/src/dev-guide/coding-guidelines.md
🪛 markdownlint-cli2 (0.17.2)
docs/src/dev-guide/coding-guidelines.md
56-56: Multiple top-level headings in the same document
(MD025, single-title, single-h1)
78-78: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
136-136: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
188-188: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
junhaoliao
changed the title
quinntaylormitchell
requested changes
Sep 18, 2025
Contributor
quinntaylormitchell
left a comment
quinntaylormitchell
left a comment
There was a problem hiding this comment.
It's coming along; a few comments to address.
A high-level comment: this document uses multiple instances of heading level 1 (#), which should really only be used in the title. level, and the usual is to start with heading 2.
(note: a few of my comments apply to multiple instances of a similar issue; I've only placed comments on the first instance of the issue, and I've indicated that comment applies elsewhere.)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
docs/src/dev-guide/coding-guidelines.md (4)
56-56: Single H1 only (MD025): demote “Zustand” section and its sub‑headingsKeep a single H1 per document. Demote this section and shift its children down one level to clear the lint error and improve hierarchy.
-# Zustand +## Zustand @@ -## File naming conventions +### File naming conventions @@ -## Store conventions +### Store conventions @@ -### Type definitions +#### Type definitions @@ -### Default values +#### Default values @@ -### Action naming +#### Action naming @@ -## Feature-based slicing +### Feature-based slicing @@ -## Store access patterns +### Store access patterns @@ -### Inside store creation +#### Inside store creation @@ -### Inside React components +#### Inside React components @@ -#### State values +##### State values @@ -#### Actions +##### Actions @@ -### Outside React components +#### Outside React componentsAlso applies to: 61-71, 75-86, 100-104, 112-136, 138-151, 151-176, 177-209, 221-239
79-85: Fix type terminology: it’s an intersection, not a unionThe examples use TypeScript’s “&” intersection. Update the wording to avoid confusion.
-* `{Name}State`: union of values and actions +* `{Name}State`: intersection of values and actions (TypeScript `&`) @@ -For large stores, split `{Name}State` into feature-specific slices (`{Name}{Feature}Slice`) and -combine them back into `{Name}State` using TypeScript intersection types. +For large stores, split `{Name}State` into feature‑specific slices (`{Name}{Feature}Slice`) and +combine them back into `{Name}State` using TypeScript intersection types.
65-70: Add import guideline: use public API entrypointsReinforce the team practice to avoid deep imports (e.g., “zustand/shallow” is OK; “zustand/index” is not).
* `index.ts` - main store file that combines slices and exports the main store hook. * `types.ts` - defines all types / interfaces for the store. +* Imports: always use package public API entrypoints (e.g., `import { create } from "zustand"`), not internal paths.
211-219: Document “static setters via getState()” conventionReflect the repo practice to avoid unnecessary subscriptions for stable actions.
-Actions usually do not change after initialization, so always access them non-reactively: +Actions usually do not change after initialization, so always access them non‑reactively. For +static setters that never change, prefer `useXxxStore.getState()` to avoid unnecessary subscriptions:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docs/src/dev-guide/coding-guidelines.md(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/Editor/index.tsx:138-139
Timestamp: 2025-06-01T13:40:12.222Z
Learning: In the yscope-log-viewer codebase, when using Zustand stores in React components, the preferred pattern is to use `getState()` for static setters that never change (like `setLogEventNum`) to avoid unnecessary subscriptions, while using hooks for actions that do more than just setting values. All store state variables should be declared at the beginning of the component for consistency and clear dependency overview.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
📚 Learning: 2025-06-01T13:40:12.222Z
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/Editor/index.tsx:138-139
Timestamp: 2025-06-01T13:40:12.222Z
Learning: In the yscope-log-viewer codebase, when using Zustand stores in React components, the preferred pattern is to use `getState()` for static setters that never change (like `setLogEventNum`) to avoid unnecessary subscriptions, while using hooks for actions that do more than just setting values. All store state variables should be declared at the beginning of the component for consistency and clear dependency overview.
Applied to files:
docs/src/dev-guide/coding-guidelines.md
📚 Learning: 2025-05-21T20:08:19.249Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#286
File: src/stores/viewStore.ts:46-46
Timestamp: 2025-05-21T20:08:19.249Z
Learning: For Zustand store actions, use `setX` for functions that simply change the value of a state, and use `updateX` for functions that do more than just set a value (e.g., perform additional logic, make API calls, update multiple states).
Applied to files:
docs/src/dev-guide/coding-guidelines.md
📚 Learning: 2025-05-17T05:13:39.947Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#265
File: src/stores/logFileStore.ts:125-125
Timestamp: 2025-05-17T05:13:39.947Z
Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.
Applied to files:
docs/src/dev-guide/coding-guidelines.md
📚 Learning: 2024-10-28T18:40:29.816Z
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#107
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/Result.tsx:1-7
Timestamp: 2024-10-28T18:40:29.816Z
Learning: In the y-scope/yscope-log-viewer project, importing components via destructuring from 'mui/joy' is acceptable, as the package size impact has been analyzed and found acceptable.
Applied to files:
docs/src/dev-guide/coding-guidelines.md
📚 Learning: 2025-09-03T18:22:30.594Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#371
File: src/components/MenuBar/index.tsx:42-46
Timestamp: 2025-09-03T18:22:30.594Z
Learning: The development team has discussed that fire-and-forget async IIFE patterns (like `(async () => { ... })().catch(handleErrorWithNotification)`) don't cause synchronization issues in the yscope-log-viewer codebase due to serialized service worker communications, but they acknowledge it's still an error-prone pattern that should be refactored for better code clarity and maintainability.
Applied to files:
docs/src/dev-guide/coding-guidelines.md
📚 Learning: 2025-05-09T01:07:32.803Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/viewStore.ts:108-109
Timestamp: 2025-05-09T01:07:32.803Z
Learning: For the viewStore in the yscope-log-viewer project, it's more appropriate to suppress the max-lines-per-function ESLint rule than to split the store into slices due to the tight coupling and cohesive nature of its functions.
Applied to files:
docs/src/dev-guide/coding-guidelines.md
📚 Learning: 2025-04-28T08:21:28.568Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/logFileStore/loadSlice.ts:1-1
Timestamp: 2025-04-28T08:21:28.568Z
Learning: When importing from npm packages, use the public API entrypoint (e.g., `import {StateCreator} from "zustand"`) rather than deep-linking into package internals (e.g., `import {StateCreator} from "zustand/index"`), as internal file structures may change between package versions.
Applied to files:
docs/src/dev-guide/coding-guidelines.md
🪛 markdownlint-cli2 (0.17.2)
docs/src/dev-guide/coding-guidelines.md
145-145: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
145-145: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
146-146: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
🔇 Additional comments (4)
docs/src/dev-guide/coding-guidelines.md (4)
223-223: Tighten wordingMinor grammar: clarify the scope and remove “for”.
[raise_nitpick_refactor]
-Always use non-reactive access since reactive subscriptions do not work for outside components. +Always use non‑reactive access, since reactive subscriptions do not work outside React components.
103-104: Finish the sentence with punctuationAdd a period at the end of the bullet for consistency.
[raise_nitpick_refactor]
-* Use uppercase constant naming with `_DEFAULT` suffix +* Use uppercase constant naming with `_DEFAULT` suffix.
162-174: Sample slice creation looks goodParameter order
(set, get)matches Zustand docs, early‑return guard is sensible.If you want stronger signalling, we can add a brief comment noting why the guard prevents redundant updates.
120-136: Caption now matches content — thanksThe action naming example and caption are aligned and clear.
Comment on lines
+143
to
+147
| :::{warning} | ||
| Follow the principle of separation of concerns: | ||
| - Do: Slice by feature. e.g., query configuration, query results, or query controller. | ||
| - Don't: Slice by type. e.g., one file for all values or one file for all actions | ||
| ::: |
There was a problem hiding this comment.
🧹 Nitpick
Fix list style and spacing inside admonition (MD004, MD032)
Use asterisks to match the rest of the doc and add a blank line before the list.
-:::{warning}
-Follow the principle of separation of concerns:
-- Do: Slice by feature. e.g., query configuration, query results, or query controller.
-- Don't: Slice by type. e.g., one file for all values or one file for all actions
-:::
+:::{warning}
+Follow the principle of separation of concerns:
+
+* Do: Slice by feature — e.g., query configuration, query results, or query controller.
+* Don't: Slice by type — e.g., one file for all values or one file for all actions.
+:::📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| :::{warning} | |
| Follow the principle of separation of concerns: | |
| - Do: Slice by feature. e.g., query configuration, query results, or query controller. | |
| - Don't: Slice by type. e.g., one file for all values or one file for all actions | |
| ::: | |
| :::{warning} | |
| Follow the principle of separation of concerns: | |
| * Do: Slice by feature — e.g., query configuration, query results, or query controller. | |
| * Don't: Slice by type — e.g., one file for all values or one file for all actions. | |
| ::: |
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
145-145: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
145-145: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
146-146: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
🤖 Prompt for AI Agents
In docs/src/dev-guide/coding-guidelines.md around lines 143 to 147, the
admonition uses hyphens and no blank line before the list which triggers
MD004/MD032; replace the hyphen list markers with asterisk markers to match the
rest of the document and insert a single blank line between the admonition intro
and the list so the list is properly separated and spaced.
Comment on lines
+149
to
+151
| Each slice should be self-contained and represent a coherent unit of application functionality. | ||
|
|
||
| ## Store access patterns |
There was a problem hiding this comment.
🧹 Nitpick
Call out the known exception (viewStore) to avoid confusion
Earlier decisions favoured suppressing max‑lines for a tightly coupled viewStore rather than slicing. Add a short note so readers don’t over‑split cohesive stores.
Each slice should be self-contained and represent a coherent unit of application functionality.
+
+:::{note}
+Exception: for highly cohesive stores (e.g., `viewStore`) where functions are tightly coupled,
+prefer suppressing the ESLint `max-lines-per-function` rule over forced slicing.
+:::If you want, I can add the exact link to the prior decision for traceability.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Each slice should be self-contained and represent a coherent unit of application functionality. | |
| ## Store access patterns | |
| Each slice should be self-contained and represent a coherent unit of application functionality. | |
| :::{note} | |
| Exception: for highly cohesive stores (e.g., `viewStore`) where functions are tightly coupled, | |
| prefer suppressing the ESLint `max-lines-per-function` rule over forced slicing. | |
| ::: | |
| ## Store access patterns |
🤖 Prompt for AI Agents
In docs/src/dev-guide/coding-guidelines.md around lines 149 to 151, add a short
note after the sentence about slices being self-contained to call out the known
exception for tightly coupled "viewStore" instances: state that viewStore may be
kept unsliced (suppressing max-lines) when it represents a cohesive, tightly
coupled read-model rather than forcing artificial slices, and optionally link to
the prior design decision for traceability; keep the note brief and directive so
readers don’t over-split cohesive stores.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This pull request adds comprehensive guidelines for using Zustand in the codebase. The new documentation covers file naming conventions, store structure, recommended patterns for state and action definitions, feature-based slicing, and best practices for accessing store values and actions both inside and outside React components.
Zustand usage guidelines:
Checklist
breaking change.
Validation performed
run
task docs:site&task docs:serve, the updated docs are shown.Summary by CodeRabbit