-
Notifications
You must be signed in to change notification settings - Fork 18
refactor: Add useCallback
for functions inside react components; Enforce useState
for calling zustand actions.
#340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…State` for calling zustand actions
WalkthroughThis update refactors several React components to memoize event handler functions using Changes
Sequence Diagram(s)sequenceDiagram
participant UI_Component as React Component
participant Store as Zustand Store
UI_Component->>UI_Component: User triggers event (e.g., click)
UI_Component->>UI_Component: Memoized handler executes (via useCallback)
UI_Component->>Store: Access action via getState()
Store-->>UI_Component: Returns action
UI_Component->>Store: Invoke action
Store-->>UI_Component: State updated
Estimated code review effort2 (10–30 minutes) Possibly related PRs
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
📓 Path-based instructions (1)**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}⚙️ CodeRabbit Configuration File
Files:
🧠 Learnings (2)📓 Common learnings
src/components/CentralContainer/Sidebar/SidebarTabs/TabButton.tsx (6)Learnt from: junhaoliao Learnt from: junhaoliao Learnt from: junhaoliao Learnt from: junhaoliao Learnt from: junhaoliao Learnt from: junhaoliao 🧰 Additional context used📓 Path-based instructions (1)**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}⚙️ CodeRabbit Configuration File
Files:
🧠 Learnings (2)📓 Common learnings
src/components/CentralContainer/Sidebar/SidebarTabs/TabButton.tsx (6)Learnt from: junhaoliao Learnt from: junhaoliao Learnt from: junhaoliao Learnt from: junhaoliao Learnt from: junhaoliao Learnt from: junhaoliao 🔇 Additional comments (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/MenuBar/NavigationBar.tsx (1)
40-40
: Store access pattern correctly implemented.This change addresses the previous feedback to use
store.getState()
and aligns with the established pattern in this codebase.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
src/components/CentralContainer/Sidebar/ResizeHandle.tsx
(1 hunks)src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/ResultsGroup.tsx
(2 hunks)src/components/MenuBar/NavigationBar.tsx
(3 hunks)src/components/MenuBar/PageNumInput.tsx
(2 hunks)src/components/PopUps/PopUpMessageBox.tsx
(1 hunks)src/components/StatusBar/LogLevelSelect/index.tsx
(3 hunks)src/components/StatusBar/index.tsx
(1 hunks)src/stores/queryStore/createQueryControllerSlice.ts
(1 hunks)src/stores/queryStore/index.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧠 Learnings (9)
📓 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#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#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#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#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#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#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#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#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.
src/components/CentralContainer/Sidebar/ResizeHandle.tsx (7)
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#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#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/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#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx:23-26
Timestamp: 2024-09-28T02:35:29.384Z
Learning: In the `getPanelWidth` function in `Sidebar/index.tsx`, it's acceptable not to handle `NaN` explicitly since it would only occur if `--ylv-panel-width` is not set due to coding errors.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx:23-26
Timestamp: 2024-10-08T15:52:50.753Z
Learning: In the `getPanelWidth` function in `Sidebar/index.tsx`, it's acceptable not to handle `NaN` explicitly since it would only occur if `--ylv-panel-width` is not set due to coding errors.
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/ResultsGroup.tsx (5)
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#107
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/ResultsGroup.tsx:99-105
Timestamp: 2024-11-14T04:34:08.409Z
Learning: In the React component `ResultsGroup` in `src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/ResultsGroup.tsx`, it's acceptable to use array indices as keys when rendering `Result` components from the `results` array, since the results are inserted in order and items are not reordered or deleted.
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#74
File: new-log-viewer/src/components/SidebarContainer/index.tsx:107-107
Timestamp: 2024-10-08T15:52:50.753Z
Learning: The `PanelTabs` component is already wrapped with `React.forwardRef` and passes the `ref` to the appropriate child element.
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.
src/components/PopUps/PopUpMessageBox.tsx (4)
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: 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#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.
src/components/StatusBar/LogLevelSelect/index.tsx (9)
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: 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/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#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#77
File: new-log-viewer/src/components/StatusBar/LogLevelSelect/index.tsx:99-105
Timestamp: 2024-10-08T15:52:50.753Z
Learning: `LOG_LEVEL` is a numerical enum with values starting from 0, so using `index` in `LOG_LEVEL_NAMES.map((logLevelName, index) => ...)` is safe.
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: `LOG_LEVEL` is a numerical enum with values starting from 0, so using `index` in `LOG_LEVEL_NAMES.map((logLevelName, index) => ...)` is safe.
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.
src/stores/queryStore/index.ts (3)
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#123
File: src/services/decoders/ClpIrDecoder.ts:0-0
Timestamp: 2024-11-18T01:36:22.048Z
Learning: In JavaScript/TypeScript module imports, `index` is automatically resolved when importing from a directory, so specifying `index` in the import path is unnecessary.
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/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.
src/components/MenuBar/NavigationBar.tsx (16)
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: Henry8192
PR: y-scope/yscope-log-viewer#80
File: new-log-viewer/src/components/MenuBar/index.tsx:35-41
Timestamp: 2024-10-09T01:22:57.841Z
Learning: The search button in the `MenuBar` component (`new-log-viewer/src/components/MenuBar/index.tsx`) is temporary and will be deleted eventually.
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: 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#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#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: 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#74
File: new-log-viewer/src/components/SidebarContainer/index.tsx:107-107
Timestamp: 2024-10-08T15:52:50.753Z
Learning: The `PanelTabs` component is already wrapped with `React.forwardRef` and passes the `ref` to the appropriate child element.
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#76
File: new-log-viewer/src/utils/actions.ts:84-139
Timestamp: 2024-09-25T21:13:37.250Z
Learning: In the `getPageNumCursorArgs` function, variables `newPageNum` and `anchor` are always initialized when handling valid actions, so there is no risk of them being uninitialized.
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#76
File: new-log-viewer/src/utils/actions.ts:84-139
Timestamp: 2024-10-08T15:52:50.753Z
Learning: In the `getPageNumCursorArgs` function, variables `newPageNum` and `anchor` are always initialized when handling valid actions, so there is no risk of them being uninitialized.
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.
src/components/MenuBar/PageNumInput.tsx (9)
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#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: davemarco
PR: y-scope/yscope-log-viewer#76
File: new-log-viewer/src/utils/actions.ts:84-139
Timestamp: 2024-10-08T15:52:50.753Z
Learning: In the `getPageNumCursorArgs` function, variables `newPageNum` and `anchor` are always initialized when handling valid actions, so there is no risk of them being uninitialized.
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#76
File: new-log-viewer/src/utils/actions.ts:84-139
Timestamp: 2024-09-25T21:13:37.250Z
Learning: In the `getPageNumCursorArgs` function, variables `newPageNum` and `anchor` are always initialized when handling valid actions, so there is no risk of them being uninitialized.
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/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: Henry8192
PR: y-scope/yscope-log-viewer#80
File: new-log-viewer/src/components/MenuBar/index.tsx:35-41
Timestamp: 2024-10-09T01:22:57.841Z
Learning: The search button in the `MenuBar` component (`new-log-viewer/src/components/MenuBar/index.tsx`) is temporary and will be deleted eventually.
src/components/StatusBar/index.tsx (9)
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: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx:37-40
Timestamp: 2025-06-01T13:41:12.938Z
Learning: The `updateWindowUrlHashParams` function in `src/utils/url.ts` doesn't throw errors, so error handling is not needed when calling this function.
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#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/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#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.
🧬 Code Graph Analysis (1)
src/components/StatusBar/index.tsx (3)
src/utils/actions.ts (1)
ACTION_NAME
(107-107)src/utils/url.ts (1)
updateWindowUrlHashParams
(336-336)src/typings/url.ts (1)
HASH_PARAM_NAMES
(37-37)
🔇 Additional comments (9)
src/components/PopUps/PopUpMessageBox.tsx (1)
51-57
: LGTM! Proper useCallback implementation.The function is correctly memoized with appropriate dependencies. The dependency array includes both
handleCloseButtonClick
andprimaryAction
, which are the values the function depends on.src/components/StatusBar/LogLevelSelect/index.tsx (2)
161-175
: LGTM! Proper useCallback for render value function.The
handleRenderValue
function is correctly memoized with an empty dependency array since it only uses theselected
parameter passed to it.
221-223
: LGTM! Proper useCallback with correct dependencies.The
handleSelectClearButtonClick
function is correctly memoized withupdateFilter
as its dependency, which is the only external value it uses.src/stores/queryStore/createQueryControllerSlice.ts (1)
5-5
: LGTM! Consistent import path with explicit file extension.The import path change to
"./createQueryConfigSlice.ts"
improves consistency and makes the file reference explicit.src/stores/queryStore/index.ts (1)
3-3
: LGTM! Consistent import path standardization.The import path change aligns with the similar update in
createQueryControllerSlice.ts
, ensuring consistent file references across the query store module.src/components/CentralContainer/Sidebar/ResizeHandle.tsx (1)
36-39
: LGTM! Appropriate useCallback with empty dependencies.The
handleMouseDown
function is correctly memoized with an empty dependency array since it only uses stable references (setIsMouseDown
fromuseState
and the event parameter).src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/ResultsGroup.tsx (1)
3-3
: Good memoization of event handler.The
useCallback
wrapping ofhandleAccordionChange
with an empty dependency array is appropriate since the function only uses stable references (setIsExpanded
) and event parameters.Also applies to: 49-54
src/components/MenuBar/NavigationBar.tsx (1)
1-1
: Excellent memoization and store access pattern.The combination of
useCallback
with empty dependencies and accessingloadPageByAction
viagetState()
follows React best practices and the established codebase patterns for Zustand store usage.Also applies to: 30-43
src/components/MenuBar/PageNumInput.tsx (1)
2-2
: Excellent comprehensive memoization refactor.All event handlers are properly memoized with correct dependency arrays:
handleSubmit
depends on[isEditing]
✓handleBlur
depends on[handleSubmit]
✓handleInputClick
andadjustInputWidth
have empty dependencies ✓handleInputChange
depends on[adjustInputWidth]
✓useEffect
correctly includesadjustInputWidth
in dependencies ✓The store access pattern using
getState()
also follows the established codebase guidelines.Also applies to: 43-90
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change looks good, but do we miss the handleClick
in
components/CentralContainer/Sidebar/SidebarTabs/TabButton.tsx
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thanks to eslint, all dependencies should already be handled, and this change shouldn't affect the log viewer's behavior.
Validations:
- Do a quick run-through on common functionalities.
useCallback
for functions inside react components; enforce useState
for calling zustand actions.useCallback
for functions inside react components; Enforce useState
for calling zustand actions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deferring to @hoophalab's review.
Description
As title suggests.
Checklist
breaking change.
Validation performed
Open log viewer, enter http://localhost:3010/?filePath=https://yscope.s3.us-east-2.amazonaws.com/sample-logs/yarn-ubuntu-resourcemanager-ip-172-31-17-135.log.1.clp.zst in log viewer, make sure page size is 10000.
Navigation bar
Prettify button
Toggle prettify button, notice that it behaves as normal.
Summary by CodeRabbit
No changes to public interfaces or component functionality.