refactor: Create async function loadPageByCursor for code reduction; Centralize uiState change in loadPageByCursor (fixes #355).
#367
Conversation
…HashParams; Centralize uiState by setting it in loadPage
WalkthroughCentralizes page-loading into a new loadPageByCursor API on the view store, moves FAST_LOADING→READY UI transitions into that method, removes UI_STATE side-effects from StatusBar and urlHash, and updates components to call loadFile() then loadPageByCursor(...) with centralized error notifications. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant StatusBar
participant UrlHash
participant ViewStore
participant LogMgr
participant Notification
User->>StatusBar: Toggle prettify / Navigate
StatusBar->>UrlHash: updateViewHashParams()
UrlHash->>ViewStore: setIsPrettified(...) / loadPageByCursor(cursor)
ViewStore->>ViewStore: set UI_STATE = FAST_LOADING
ViewStore->>LogMgr: logFileManagerProxy.loadPage(cursor)
LogMgr-->>ViewStore: PageData
ViewStore->>ViewStore: updatePageData(PageData)
ViewStore->>ViewStore: set UI_STATE = READY
alt error
Any->>Notification: handleErrorWithNotification(err)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Out-of-scope changes
Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}⚙️ CodeRabbit Configuration File
Files:
🧠 Learnings (1)📚 Learning: 2025-06-01T13:41:12.938ZApplied to files:
🧬 Code Graph Analysis (1)src/components/AppController.tsx (7)
🔇 Additional comments (3)
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Henry8192
changed the title
loadPageByCursor for code reduction; Centralize uiState change in loadPageByCursor.loadPageByCursor for code reduction; Centralize uiState change in loadPageByCursor (Fixes #356).
Henry8192
changed the title
loadPageByCursor for code reduction; Centralize uiState change in loadPageByCursor (Fixes #356).loadPageByCursor for code reduction; Centralize uiState change in loadPageByCursor (fixes #356).
Henry8192
changed the title
loadPageByCursor for code reduction; Centralize uiState change in loadPageByCursor (fixes #356).loadPageByCursor for code reduction; Centralize uiState change in loadPageByCursor (fixes #355).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (3)
src/stores/viewStore/createViewPageSlice.ts (2)
17-19: Name consistency nit: store alias differs across files.Here the import alias is useLogFileManagerStore, while urlHash.ts uses useLogFileManagerProxyStore for the same module. Consider aligning the alias for readability and consistency.
95-112: Reload branch sanity check is good; consider using notifications over thrown errors.Throwing here bubbles to the caller; if uncaught, it can be noisy. Using handleErrorWithNotification would align with the rest of the error handling approach (and keep the app responsive).
- throw new Error( - `Unexpected fileSrc=${JSON.stringify( - fileSrc - )}, logEventNum=${logEventNum} when reloading.` - ); + handleErrorWithNotification( + new Error( + `Unexpected fileSrc=${JSON.stringify(fileSrc)}, logEventNum=${logEventNum} when reloading.` + ) + ); + return;src/utils/url/urlHash.ts (1)
87-102: Action required — audit direct callers of updatePageData and logFileManagerProxy.loadPageSearch shows direct uses outside the view page slice that may bypass the new loadPageByCursor / URL-hash flow. Please review and either route navigation flows through loadPageByCursor or document/justify the direct calls.
Files to inspect:
- src/stores/logFileStore.ts — updatePageData at lines 132–133 and 162; logFileManagerProxy.loadPage at line 161.
- src/stores/viewStore/createViewFilterSlice.ts — updatePageData at lines 42–43.
- src/stores/viewStore/createViewPageSlice.ts — logFileManagerProxy.loadPage at line 134; updatePageData usage/definition at ~135–140 (expected in the view slice).
- src/stores/viewStore/types.ts — updatePageData type declaration at line 20 (informational).
Recommended fix: for navigation flows, replace direct updatePageData/logFileManagerProxy.loadPage calls with useViewStore.getState().loadPageByCursor(cursor) so URL-hash and navigation are canonical. If a direct call is intentional (e.g., internal file loads or preloads), add a brief comment explaining why it must bypass loadPageByCursor.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
src/components/StatusBar/index.tsx(1 hunks)src/stores/viewStore/createViewPageSlice.ts(1 hunks)src/stores/viewStore/types.ts(2 hunks)src/utils/url/urlHash.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit Configuration File
- Prefer
false == <expression>rather than!<expression>.
Files:
src/components/StatusBar/index.tsxsrc/stores/viewStore/types.tssrc/stores/viewStore/createViewPageSlice.tssrc/utils/url/urlHash.ts
🧠 Learnings (4)
📚 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:
src/components/StatusBar/index.tsxsrc/utils/url/urlHash.ts
📚 Learning: 2024-10-08T15:52:50.753Z
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.
Applied to files:
src/stores/viewStore/types.tssrc/stores/viewStore/createViewPageSlice.ts
📚 Learning: 2024-10-08T15:52:50.753Z
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.
Applied to files:
src/stores/viewStore/createViewPageSlice.ts
📚 Learning: 2025-06-01T13:41:12.938Z
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.
Applied to files:
src/utils/url/urlHash.ts
🧬 Code Graph Analysis (3)
src/stores/viewStore/types.ts (2)
src/services/LogFileManager/index.ts (1)
cursor(443-477)src/typings/worker.ts (1)
CursorType(108-108)
src/stores/viewStore/createViewPageSlice.ts (5)
src/typings/states.ts (1)
UI_STATE(106-106)src/services/LogFileManager/index.ts (1)
cursor(443-477)src/stores/notificationStore.ts (1)
handleErrorWithNotification(76-76)src/typings/worker.ts (2)
CursorType(108-108)PageData(111-111)src/utils/url/index.ts (1)
updateWindowUrlHashParams(385-385)
src/utils/url/urlHash.ts (3)
src/services/LogFileManager/index.ts (3)
numEvents(131-133)setIsPrettified(199-201)cursor(443-477)src/services/LogFileManagerProxy.ts (1)
setIsPrettified(60-63)src/stores/notificationStore.ts (1)
handleErrorWithNotification(76-76)
🔇 Additional comments (3)
src/components/StatusBar/index.tsx (1)
17-17: LGTM: UI state manipulation correctly removed from StatusBar.Importing only UI_ELEMENT and delegating UI state transitions to the centralised flow reduces duplication and side-effects in the component.
src/utils/url/urlHash.ts (2)
100-101: LGTM: Centralised loading via loadPageByCursor.This replaces scattered data/UI updates with a single store action. The catch(handler) maintains consistent error notifications.
1-3: Remove .ts extensions from imports for consistency — confirm with your toolchainrg found .ts extensions in multiple imports. If your bundler/TS config does not require explicit extensions, drop them for consistency; otherwise keep them.
Files to update:
- src/utils/url/urlHash.ts — lines 1–3
- src/stores/queryStore/createQueryControllerSlice.ts — line 5
- src/stores/queryStore/index.ts — line 3
Suggested diffs:
-import useLogFileManagerProxyStore from "../../stores/logFileManagerProxyStore.ts"; -import useLogFileStore from "../../stores/logFileStore.ts"; -import {handleErrorWithNotification} from "../../stores/notificationStore.ts"; +import useLogFileManagerProxyStore from "../../stores/logFileManagerProxyStore"; +import useLogFileStore from "../../stores/logFileStore"; +import {handleErrorWithNotification} from "../../stores/notificationStore";-import {QUERY_CONFIG_DEFAULT} from "./createQueryConfigSlice.ts"; +import {QUERY_CONFIG_DEFAULT} from "./createQueryConfigSlice";-import {createQueryConfigSlice} from "./createQueryConfigSlice.ts"; +import {createQueryConfigSlice} from "./createQueryConfigSlice";
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/stores/viewStore/createViewPageSlice.ts (1)
150-158: Keep updatePageData pure (move URL mutation next to navigation) — optionalThis keeps state updates side-effect free and centralizes URL changes inside the navigation flow. Prior comment suggested the same.
Apply this diff to updatePageData:
updatePageData: (pageData: PageData) => { set({ beginLineNumToLogEventNum: pageData.beginLineNumToLogEventNum, logData: pageData.logs, logEventNum: pageData.logEventNum, numPages: pageData.numPages, pageNum: pageData.pageNum, }); - updateWindowUrlHashParams({logEventNum: pageData.logEventNum}); },And add the URL update after updatePageData inside loadPageByCursor (leveraging the try/finally you already have):
try { const {logFileManagerProxy} = useLogFileManagerStore.getState(); const pageData = await logFileManagerProxy.loadPage(cursor); const {updatePageData} = get(); updatePageData(pageData); + updateWindowUrlHashParams({logEventNum: pageData.logEventNum}); } finally { setUiState(UI_STATE.READY); }Note: Per our team learning, updateWindowUrlHashParams does not throw, so no extra error handling needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/stores/viewStore/createViewPageSlice.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit Configuration File
- Prefer
false == <expression>rather than!<expression>.
Files:
src/stores/viewStore/createViewPageSlice.ts
🧠 Learnings (3)
📚 Learning: 2025-06-01T13:41:12.938Z
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.
Applied to files:
src/stores/viewStore/createViewPageSlice.ts
📚 Learning: 2024-10-08T15:52:50.753Z
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.
Applied to files:
src/stores/viewStore/createViewPageSlice.ts
📚 Learning: 2024-10-08T15:52:50.753Z
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.
Applied to files:
src/stores/viewStore/createViewPageSlice.ts
🔇 Additional comments (3)
src/stores/viewStore/createViewPageSlice.ts (3)
84-85: Single-flight guard introduced — good callThe module-level in-flight promise is a pragmatic way to de-duplicate concurrent loads. This addresses the overlapping-loads risk that bypassed the READY gate.
136-144: UI_STATE reset via try/finally — exactly what was neededWrapping the load in try/finally ensures READY is restored even on failures. Callers can keep catching for notifications without risking a stuck FAST_LOADING state.
125-149: Confirm intended behaviour: subsequent load requests are ignored while one is in flightThe function returns the existing
pageLoadInFlightpromise when truthy, so additional calls (even with a different cursor) are de-duplicated and effectively dropped until the in‑flight promise resolves.
- Location: src/stores/viewStore/createViewPageSlice.ts — loadPageByCursor (lines 125–149)
- If this “drop while loading” behaviour is intentional, add an inline comment documenting it for future maintainers.
- If you prefer “latest-wins”, I can implement a compact follow-up load: store a single pendingCursor and trigger it once the current load completes (no parallel loads).
Would you like me to propose/submit that compact “latest-wins” patch?
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/stores/viewStore/createViewPageSlice.ts (3)
120-126: Guard navigation when no pages are available (numPages <= 0)Avoid constructing cursors when there are zero pages; depending on clamp’s behaviour, you could compute an invalid page. Add an early return.
Apply this diff:
- const {numPages, pageNum, loadPageByCursor} = get(); + const {numPages, pageNum, loadPageByCursor} = get(); + if (numPages <= 0) { + console.warn("Cannot navigate: no pages available.", {navAction, numPages}); + return; + } const cursor = getPageNumCursor(navAction, pageNum, numPages); if (null === cursor) { console.error(`Error with nav action ${navAction.code}.`); return; }
129-141: De-duplicate concurrent loads inside loadPageByCursor (return same in-flight promise)updateViewHashParams now calls this directly, bypassing the READY gate. Without in-flight dedupe, concurrent calls can race, causing out-of-order page data and flickery UI. Add a lightweight in-flight guard that returns the same promise to concurrent callers, and always clears on settle. Keep the try/finally to guarantee READY is restored.
Apply this diff:
- loadPageByCursor: async (cursor: CursorType) => { - const {setUiState} = useUiStore.getState(); - setUiState(UI_STATE.FAST_LOADING); - - try { - const {logFileManagerProxy} = useLogFileManagerStore.getState(); - const pageData = await logFileManagerProxy.loadPage(cursor); - const {updatePageData} = get(); - updatePageData(pageData); - } finally { - setUiState(UI_STATE.READY); - } - }, + loadPageByCursor: (cursor: CursorType) => { + if (false == (null === pageLoadInFlight)) { + console.warn("Page load already in flight, ignoring new request.", cursor); + return pageLoadInFlight as Promise<void>; + } + + const run = (async () => { + const {setUiState} = useUiStore.getState(); + setUiState(UI_STATE.FAST_LOADING); + try { + const {logFileManagerProxy} = useLogFileManagerStore.getState(); + const pageData = await logFileManagerProxy.loadPage(cursor); + const {updatePageData} = get(); + updatePageData(pageData); + } finally { + setUiState(UI_STATE.READY); + } + })(); + + pageLoadInFlight = run; + return run.finally(() => { + pageLoadInFlight = null; + }); + },Add this module-scoped variable near the top of the file (outside the slice) to track the in-flight promise:
let pageLoadInFlight: Promise<void> | null = null;Run this to verify urlHash and other callers currently invoke loadPageByCursor directly (bypassing the READY gate) and benefit from the dedupe:
#!/bin/bash set -euo pipefail echo "== Call sites of loadPageByCursor ==" rg -n -S "loadPageByCursor\\s*\\(" -g '!**/node_modules/**' -g '!**/dist/**' || true echo echo "== updateViewHashParams usage (should delegate to view store) ==" rg -n -S "updateViewHashParams|loadPageByCursor|loadPageByAction" src/utils/url || true
142-150: Keep updatePageData pure; move URL mutation to the navigation/load pathSide-effects (mutating the URL) inside a state updater make it harder to reuse in isolation and to test. Move updateWindowUrlHashParams to the load path right after updatePageData.
Apply this diff to make updatePageData pure:
updatePageData: (pageData: PageData) => { set({ beginLineNumToLogEventNum: pageData.beginLineNumToLogEventNum, logData: pageData.logs, logEventNum: pageData.logEventNum, numPages: pageData.numPages, pageNum: pageData.pageNum, }); - updateWindowUrlHashParams({logEventNum: pageData.logEventNum}); },Then, add the URL update immediately after updatePageData in loadPageByCursor (inside the try block), e.g.:
// inside loadPageByCursor, after updatePageData(pageData): updateWindowUrlHashParams({logEventNum: pageData.logEventNum});
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/stores/viewStore/createViewPageSlice.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit Configuration File
- Prefer
false == <expression>rather than!<expression>.
Files:
src/stores/viewStore/createViewPageSlice.ts
🧠 Learnings (3)
📚 Learning: 2025-06-01T13:41:12.938Z
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.
Applied to files:
src/stores/viewStore/createViewPageSlice.ts
📚 Learning: 2024-10-08T15:52:50.753Z
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.
Applied to files:
src/stores/viewStore/createViewPageSlice.ts
📚 Learning: 2024-10-08T15:52:50.753Z
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.
Applied to files:
src/stores/viewStore/createViewPageSlice.ts
🧬 Code Graph Analysis (1)
src/stores/viewStore/createViewPageSlice.ts (5)
src/typings/states.ts (1)
UI_STATE(106-106)src/services/LogFileManager/index.ts (1)
cursor(443-477)src/stores/notificationStore.ts (1)
handleErrorWithNotification(76-76)src/typings/worker.ts (2)
CursorType(108-108)PageData(111-111)src/utils/url/index.ts (1)
updateWindowUrlHashParams(385-385)
🔇 Additional comments (1)
src/stores/viewStore/createViewPageSlice.ts (1)
114-120: READY gate to prevent overlapping navigation looks goodThis local guard prevents re-entrancy from UI-triggered navigation paths. Keep it — it’s the right place to avoid stacking actions when a load is already in progress.
|
@hoophalab I plan to extract the loadPage & startQuery logic of loadFile out. I remember in an offline discussion weeks ago, we believe that loadFile should be kept clean and only loads the file. It is the caller's responsibilty to call the subsequent loadPage and startQuery. Leave a thumbs up if you agree, or correct me below. |
Are we only splitting the logic for loadPage and startQuery away from loadFile, but still keeping all the functions within Zustand? If that's the plan, I think we're all set. If we're also planning to move the loadFile, loadPage, and startQuery functions completely out of Zustand, we should double-check with Junhao to make sure we haven't overlooked any potential issues. |
… it is now the caller's responsibility to loadPage after loadFile.
|
I temporarily disabled the max lines for loadFile (21 statements, max 20). We need to figure out how to refactor that. |
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (4)
src/stores/logFileStore.ts (2)
111-163: Avoid leaving UI stuck in FILE_LOADING on errors: reset to READY in catchIf
logFileManagerProxy.loadFile(or any await) throws, UI remains inFILE_LOADING. Reset toREADYon error and rethrow so centralized error notification still fires.Apply this diff:
- // eslint-disable-next-line max-statements - loadFile: async (fileSrc: FileSrcType) => { - const {setUiState} = useUiStore.getState(); - setUiState(UI_STATE.FILE_LOADING); + // eslint-disable-next-line max-statements + loadFile: async (fileSrc: FileSrcType) => { + const {setUiState} = useUiStore.getState(); + setUiState(UI_STATE.FILE_LOADING); + try { set({ fileName: "Loading...", fileSrc: fileSrc, metadata: LOG_FILE_STORE_DEFAULT.metadata, onDiskFileSizeInBytes: LOG_FILE_STORE_DEFAULT.onDiskFileSizeInBytes, }); if ("string" !== typeof fileSrc) { updateWindowUrlSearchParams({[SEARCH_PARAM_NAMES.FILE_PATH]: null}); } @@ - if (0 === decoderOptions.formatString.length && fileInfo.fileTypeInfo.isStructured) { - const {postPopUp} = useNotificationStore.getState(); - postPopUp(FORMAT_POP_UP_MESSAGE); - } - }, + if (0 === decoderOptions.formatString.length && fileInfo.fileTypeInfo.isStructured) { + const {postPopUp} = useNotificationStore.getState(); + postPopUp(FORMAT_POP_UP_MESSAGE); + } + } catch (error) { + // Restore READY on error; loadPageByCursor will transition to READY on success. + setUiState(UI_STATE.READY); + throw error; + } + },
138-141: Remove duplicate state/URL updates (fileSrc and FILE_PATH cleared twice)
fileSrcis set on Line 117 and the FILE_PATH search param is cleared on Lines 122–124. Lines 138–141 repeat both. Drop the second block.Apply this diff:
- set({fileSrc}); - if ("string" !== typeof fileSrc) { - updateWindowUrlSearchParams({[SEARCH_PARAM_NAMES.FILE_PATH]: null}); - }src/stores/viewStore/createViewPageSlice.ts (1)
17-17: Naming consistency: align store name with file name (optional)Elsewhere (e.g., logFileStore.ts) the import is
useLogFileManagerProxyStore. Here it’suseLogFileManagerStore. Consider standardizing the identifier for readability.Proposed change:
-import useLogFileManagerStore from "../logFileManagerProxyStore"; +import useLogFileManagerProxyStore from "../logFileManagerProxyStore"; @@ - const {logFileManagerProxy} = useLogFileManagerStore.getState(); + const {logFileManagerProxy} = useLogFileManagerProxyStore.getState();src/components/AppController.tsx (1)
85-86: Avoid self-triggered hashchange race on initYou normalise the hash (Lines 95–101) right after registering the handler. That can fire
hashchangeand causeupdateViewHashParams()to run (which callsloadPageByCursor) before the file is loaded. Register the handler after initial normalisation/load scheduling.Apply this diff:
- window.addEventListener("hashchange", handleHashChange); + // Defer registration to avoid handling our own initial normalisation below.And add the listener after the initialisation block, just before returning the cleanup:
+ window.addEventListener("hashchange", handleHashChange); return () => { window.removeEventListener("hashchange", handleHashChange); };
♻️ Duplicate comments (4)
src/stores/logFileStore.ts (1)
129-136: Calling updatePageData here mutates URL hash during “Loading…” placeholder (undesirable side-effect)
updatePageDatacurrently updates the URL hash, which can clobber incoming hash params and cause early hashchange-driven navigation. Use a pure updater for the placeholder or makeupdatePageDatapure and move URL updates toloadPageByCursor.Minimal direction:
- Remove URL mutation from
updatePageData.- Immediately after
updatePageData(pageData)inloadPageByCursor, callupdateWindowUrlHashParams({logEventNum: pageData.logEventNum}).See the suggested change in
createViewPageSlice.ts(comment on Lines 146–155).src/stores/viewStore/createViewPageSlice.ts (3)
146-155: Keep updatePageData pure; move URL updates to loadPageByCursorSide-effects in
updatePageDatamake it harder to reuse when rendering placeholders (e.g., “Loading…” in logFileStore) and can unintentionally clobber URL state.Apply this minimal reshuffle:
- updatePageData: (pageData: PageData) => { + updatePageData: (pageData: PageData) => { set({ beginLineNumToLogEventNum: pageData.beginLineNumToLogEventNum, logData: pageData.logs, logEventNum: pageData.logEventNum, numPages: pageData.numPages, pageNum: pageData.pageNum, }); - updateWindowUrlHashParams({logEventNum: pageData.logEventNum}); },Then, right after
updatePageData(pageData)inloadPageByCursor:- const {updatePageData} = get(); - updatePageData(pageData); + const {updatePageData} = get(); + updatePageData(pageData); + updateWindowUrlHashParams({logEventNum: pageData.logEventNum});
118-132: Guard nav when there are no pages; and detach the promise (nitpick)
- If
numPages <= 0, navigation is meaningless; guard early.- Prefix the detached promise with
voidto appease strict linters.Apply this diff:
- const {numPages, pageNum, loadPageByCursor} = get(); + const {numPages, pageNum, loadPageByCursor} = get(); + if (numPages <= 0) { + console.warn("Cannot navigate: no pages available.", {navAction, numPages}); + return; + } @@ - loadPageByCursor(cursor).catch(handleErrorWithNotification); + void loadPageByCursor(cursor).catch(handleErrorWithNotification);
133-145: De-duplicate concurrent page loads: add an in-flight guard in loadPageByCursor
updateViewHashParamscallsloadPageByCursordirectly and bypasses the READY-gate used inloadPageByAction. Without a guard, calls can overlap (race conditions, stale updates).Apply this diff to add a lightweight in-flight promise:
@@ -const createViewPageSlice: StateCreator< +let pageLoadInFlight: Promise<void> | null = null; + +const createViewPageSlice: StateCreator< @@ - loadPageByCursor: async (cursor: CursorType) => { - const {setUiState} = useUiStore.getState(); - setUiState(UI_STATE.FAST_LOADING); - - try { - const {logFileManagerProxy} = useLogFileManagerStore.getState(); - const pageData = await logFileManagerProxy.loadPage(cursor); - const {updatePageData} = get(); - updatePageData(pageData); - } finally { - setUiState(UI_STATE.READY); - } - }, + loadPageByCursor: async (cursor: CursorType) => { + if (pageLoadInFlight) { + console.warn("Page load already in flight, ignoring new request.", cursor); + return pageLoadInFlight; + } + + const {setUiState} = useUiStore.getState(); + setUiState(UI_STATE.FAST_LOADING); + + pageLoadInFlight = (async () => { + try { + const {logFileManagerProxy} = useLogFileManagerStore.getState(); + const pageData = await logFileManagerProxy.loadPage(cursor); + const {updatePageData} = get(); + updatePageData(pageData); + } finally { + setUiState(UI_STATE.READY); + } + })(); + + return pageLoadInFlight.finally(() => { pageLoadInFlight = null; }); + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
src/components/AppController.tsx(3 hunks)src/components/DropFileContainer/index.tsx(2 hunks)src/components/MenuBar/index.tsx(2 hunks)src/stores/logFileStore.ts(4 hunks)src/stores/viewStore/createViewPageSlice.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit Configuration File
- Prefer
false == <expression>rather than!<expression>.
Files:
src/components/DropFileContainer/index.tsxsrc/stores/viewStore/createViewPageSlice.tssrc/components/AppController.tsxsrc/components/MenuBar/index.tsxsrc/stores/logFileStore.ts
🧠 Learnings (4)
📚 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:
src/components/DropFileContainer/index.tsx
📚 Learning: 2025-06-01T13:41:12.938Z
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.
Applied to files:
src/stores/viewStore/createViewPageSlice.ts
📚 Learning: 2024-10-08T15:52:50.753Z
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.
Applied to files:
src/stores/viewStore/createViewPageSlice.ts
📚 Learning: 2025-08-14T15:27:32.397Z
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#367
File: src/utils/url/urlHash.ts:39-51
Timestamp: 2025-08-14T15:27:32.397Z
Learning: The logFileManagerProxy methods (including setIsPrettified) are asynchronous and return Promises, requiring await when called and proper async handling.
Applied to files:
src/stores/logFileStore.ts
🧬 Code Graph Analysis (5)
src/components/DropFileContainer/index.tsx (3)
src/services/LogFileManagerProxy.ts (1)
loadFile(17-43)src/typings/worker.ts (1)
CURSOR_CODE(101-101)src/stores/notificationStore.ts (1)
handleErrorWithNotification(76-76)
src/stores/viewStore/createViewPageSlice.ts (6)
src/services/LogFileManagerProxy.ts (1)
loadFile(17-43)src/typings/worker.ts (3)
CURSOR_CODE(101-101)CursorType(108-108)PageData(111-111)src/stores/notificationStore.ts (1)
handleErrorWithNotification(76-76)src/typings/states.ts (1)
UI_STATE(106-106)src/services/LogFileManager/index.ts (1)
cursor(443-477)src/utils/url/index.ts (1)
updateWindowUrlHashParams(385-385)
src/components/AppController.tsx (7)
src/typings/url.ts (1)
UrlHashParams(43-43)src/services/LogFileManager/index.ts (2)
cursor(443-477)startQuery(307-337)src/typings/worker.ts (2)
CursorType(108-108)CURSOR_CODE(101-101)src/utils/url/index.ts (1)
URL_HASH_PARAMS_DEFAULT(387-387)src/services/LogFileManagerProxy.ts (2)
loadFile(17-43)startQuery(70-73)src/stores/notificationStore.ts (1)
handleErrorWithNotification(76-76)src/utils/url/urlHash.ts (1)
updateQueryHashParams(137-137)
src/components/MenuBar/index.tsx (3)
src/services/LogFileManagerProxy.ts (1)
loadFile(17-43)src/typings/worker.ts (1)
CURSOR_CODE(101-101)src/stores/notificationStore.ts (1)
handleErrorWithNotification(76-76)
src/stores/logFileStore.ts (3)
src/typings/worker.ts (1)
FileSrcType(109-109)src/utils/config.ts (1)
getConfig(192-192)src/typings/config.ts (1)
CONFIG_KEY(45-45)
🔇 Additional comments (2)
src/stores/logFileStore.ts (1)
156-158: Good: awaiting proxy method setIsPrettified
logFileManagerProxy.setIsPrettifiedis async; awaiting it is correct and prevents race conditions with subsequent loads or UI state.src/stores/viewStore/createViewPageSlice.ts (1)
137-144: LGTM: try/finally ensures UI_STATE resets to READYGood defensive pattern: even if
loadPagefails, UI will not get stuck in a loading state.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (3)
src/stores/logFileStore.ts (3)
111-158: Handle errors: UI can get stuck in FILE_LOADING and user gets no feedback on load failuresIf
getConfig,logFileManagerProxy.loadFile, orsetIsPrettifiedthrows, the UI remains inFILE_LOADING, page shows “Loading…”, and no error notification is posted. This is a regression from the previous explicit error handling.Apply a try/catch around the awaited section and reset UI state + notify on failure:
loadFile: async (fileSrc: FileSrcType) => { const {setUiState} = useUiStore.getState(); setUiState(UI_STATE.FILE_LOADING); set({ fileName: "Loading...", fileSrc: fileSrc, metadata: LOG_FILE_STORE_DEFAULT.metadata, onDiskFileSizeInBytes: LOG_FILE_STORE_DEFAULT.onDiskFileSizeInBytes, }); if ("string" !== typeof fileSrc) { updateWindowUrlSearchParams({[SEARCH_PARAM_NAMES.FILE_PATH]: null}); } const {setExportProgress} = useLogExportStore.getState(); setExportProgress(LOG_EXPORT_STORE_DEFAULT.exportProgress); const {updatePageData} = useViewStore.getState(); updatePageData({ beginLineNumToLogEventNum: VIEW_PAGE_DEFAULT.beginLineNumToLogEventNum, logEventNum: useViewStore.getState().logEventNum, logs: "Loading...", numPages: VIEW_PAGE_DEFAULT.numPages, pageNum: VIEW_PAGE_DEFAULT.pageNum, }); - const {logFileManagerProxy} = useLogFileManagerProxyStore.getState(); - const decoderOptions = getConfig(CONFIG_KEY.DECODER_OPTIONS); - const fileInfo = await logFileManagerProxy.loadFile( - { - decoderOptions: decoderOptions, - fileSrc: fileSrc, - pageSize: getConfig(CONFIG_KEY.PAGE_SIZE), - }, - Comlink.proxy(handleExportChunk), - Comlink.proxy(handleQueryResults) - ); - - set(fileInfo); - - const {isPrettified} = useViewStore.getState(); - await logFileManagerProxy.setIsPrettified(isPrettified); - - if (0 === decoderOptions.formatString.length && fileInfo.fileTypeInfo.isStructured) { - const {postPopUp} = useNotificationStore.getState(); - postPopUp(FORMAT_POP_UP_MESSAGE); - } + try { + const {logFileManagerProxy} = useLogFileManagerProxyStore.getState(); + const decoderOptions = getConfig(CONFIG_KEY.DECODER_OPTIONS); + const fileInfo = await logFileManagerProxy.loadFile( + { + decoderOptions: decoderOptions, + fileSrc: fileSrc, + pageSize: getConfig(CONFIG_KEY.PAGE_SIZE), + }, + Comlink.proxy(handleExportChunk), + Comlink.proxy(handleQueryResults) + ); + + set(fileInfo); + + const {isPrettified} = useViewStore.getState(); + await logFileManagerProxy.setIsPrettified(isPrettified); + + if (0 === decoderOptions.formatString.length && fileInfo.fileTypeInfo.isStructured) { + const {postPopUp} = useNotificationStore.getState(); + postPopUp(FORMAT_POP_UP_MESSAGE); + } + } catch (e) { + // Avoid being stuck in FILE_LOADING and surface the error to the user. + setUiState(UI_STATE.UNOPENED); + const {postPopUp} = useNotificationStore.getState(); + postPopUp({ + level: LOG_LEVEL.ERROR, + title: "Failed to load file", + message: e instanceof Error ? e.message : String(e), + timeoutMillis: LONG_AUTO_DISMISS_TIMEOUT_MILLIS, + }); + } },
128-135: Avoid double store reads for logEventNum (micro-nit)Slightly cleaner and avoids reading the store twice.
- updatePageData({ - beginLineNumToLogEventNum: VIEW_PAGE_DEFAULT.beginLineNumToLogEventNum, - logEventNum: useViewStore.getState().logEventNum, - logs: "Loading...", - numPages: VIEW_PAGE_DEFAULT.numPages, - pageNum: VIEW_PAGE_DEFAULT.pageNum, - }); + const {logEventNum} = useViewStore.getState(); + updatePageData({ + beginLineNumToLogEventNum: VIEW_PAGE_DEFAULT.beginLineNumToLogEventNum, + logEventNum, + logs: "Loading...", + numPages: VIEW_PAGE_DEFAULT.numPages, + pageNum: VIEW_PAGE_DEFAULT.pageNum, + });
111-158: Mitigate race conditions on rapid consecutive loadsTwo overlapping
loadFilecalls can interleave; the slower one may overwrite state from the newer one. Consider a monotonically increasingloadIdtoken stored in state, checked before applying results, or a cancellable worker-side load.If helpful, I can sketch a lightweight token-based guard inside this store.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/stores/logFileStore.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit Configuration File
- Prefer
false == <expression>rather than!<expression>.
Files:
src/stores/logFileStore.ts
🧠 Learnings (1)
📚 Learning: 2025-08-14T15:27:32.397Z
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#367
File: src/utils/url/urlHash.ts:39-51
Timestamp: 2025-08-14T15:27:32.397Z
Learning: The logFileManagerProxy methods (including setIsPrettified) are asynchronous and return Promises, requiring await when called and proper async handling.
Applied to files:
src/stores/logFileStore.ts
🧬 Code Graph Analysis (1)
src/stores/logFileStore.ts (3)
src/typings/worker.ts (1)
FileSrcType(109-109)src/utils/config.ts (1)
getConfig(192-192)src/typings/config.ts (1)
CONFIG_KEY(45-45)
🔇 Additional comments (5)
src/stores/logFileStore.ts (5)
19-19: API surface aligns with refactor: FileSrcType-only import looks correctDropping CursorType from this store is consistent with delegating paging to viewStore. No issues.
24-24: Import of useNotificationStore is appropriate and usedThis import is required for the format popup; usage is correct later in the file.
151-153: Good: Awaiting async proxy call prevents race with subsequent actionsAwaiting
setIsPrettifiedis correct per the async proxy contract; avoids subtle timing issues.
40-41: Signature change is breaking — manual verification neededI ran a preliminary search and didn’t find any two-argument
loadFile(...)calls, but absence of matches isn’t proof of absence. Please manually confirm:
- All
loadFileinvocations now take onlyfileSrc: FileSrcType(no cursor).- There are no remaining
CursorTypereferences outside of your.d.tsfiles.- Each file load is followed by the new paging call (
loadPageByCursoror equivalent).
121-123: Confirm URL-param behaviour: set FILE_PATH when fileSrc is a string?
We only see the clearing call in src/stores/logFileStore.ts (line 122):
•updateWindowUrlSearchParams({ [SEARCH_PARAM_NAMES.FILE_PATH]: null });There’s no matching setter for a string
fileSrc. If we never explicitly set it, a staleFILE_PATHmay persist in the URL across loads.
Please verify that, in the branch wherefileSrcis a string, you invoke:updateWindowUrlSearchParams({ [SEARCH_PARAM_NAMES.FILE_PATH]: fileSrc });— for example, in
logFileStoreright after assigning a string source.
# Conflicts: # src/components/AppController.tsx # src/stores/logFileStore.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/components/AppController.tsx (3)
105-113: startQuery sequencing fix is correctStarting the query only after loadFile and loadPageByCursor completes avoids races with an uninitialised LogFileManager. Good change.
42-66: Don’t pre-emptively mutate view state in getInitialCursor; simplify and add explicit return typeLet loadPageByCursor update logEventNum from authoritative page data to avoid transient mismatches. While here, return early and annotate the return type for clarity.
Apply:
-/** - * Returns the initial load file cursor based on the URL hash parameters. - * - * @param hashParams - * @return - */ -const getInitialCursor = (hashParams: UrlHashParams) => { - let cursor: CursorType = {code: CURSOR_CODE.LAST_EVENT, args: null}; - - if (URL_HASH_PARAMS_DEFAULT.timestamp !== hashParams.timestamp) { - cursor = { - code: CURSOR_CODE.TIMESTAMP, - args: {timestamp: hashParams.timestamp}, - }; - } else if (URL_HASH_PARAMS_DEFAULT.logEventNum !== hashParams.logEventNum) { - const {setLogEventNum} = useViewStore.getState(); - setLogEventNum(hashParams.logEventNum); - cursor = { - code: CURSOR_CODE.EVENT_NUM, - args: {eventNum: hashParams.logEventNum}, - }; - } - - return cursor; -}; +/** + * Returns the initial load file cursor based on the URL hash parameters. + */ +const getInitialCursor = (hashParams: UrlHashParams): CursorType => { + if (URL_HASH_PARAMS_DEFAULT.timestamp !== hashParams.timestamp) { + return { + code: CURSOR_CODE.TIMESTAMP, + args: {timestamp: hashParams.timestamp}, + }; + } + if (URL_HASH_PARAMS_DEFAULT.logEventNum !== hashParams.logEventNum) { + return { + code: CURSOR_CODE.EVENT_NUM, + args: {eventNum: hashParams.logEventNum}, + }; + } + return {code: CURSOR_CODE.LAST_EVENT, args: null}; +};
105-113: Explicitly detach the async IIFE to satisfy no-floating-promisesPrefix with void to avoid linter warnings while preserving behaviour.
- (async () => { + void (async () => { await loadFile(searchParams.filePath); const {loadPageByCursor} = useViewStore.getState(); await loadPageByCursor(getInitialCursor(hashParams)); if (updateQueryHashParams()) { const {startQuery} = useQueryStore.getState(); startQuery(); } })().catch(handleErrorWithNotification);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/components/AppController.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit Configuration File
- Prefer
false == <expression>rather than!<expression>.
Files:
src/components/AppController.tsx
🧬 Code Graph Analysis (1)
src/components/AppController.tsx (7)
src/typings/url.ts (1)
UrlHashParams(43-43)src/services/LogFileManager/index.ts (2)
cursor(443-477)startQuery(307-337)src/typings/worker.ts (2)
CursorType(108-108)CURSOR_CODE(101-101)src/utils/url/index.ts (1)
URL_HASH_PARAMS_DEFAULT(387-387)src/services/LogFileManagerProxy.ts (2)
loadFile(17-43)startQuery(70-73)src/utils/url/urlHash.ts (1)
updateQueryHashParams(137-137)src/stores/notificationStore.ts (1)
handleErrorWithNotification(76-76)
🔇 Additional comments (2)
src/components/AppController.tsx (2)
7-7: Centralized error handling addition looks goodImporting handleErrorWithNotification and using it for async flows is consistent with the new centralised error reporting.
34-40: Ignore wrappinghandleHashChangein an async IIFE—updateViewHashParamsis already synchronous and handles its own errors
updateViewHashParamsdoesn’t return aPromise(it invokesloadPageByCursor(...).catch(...)internally), so:
- There are no unhandled rejections to catch externally.
- Awaiting its (void) return value in
handleHashChangewould have no effect.- Sequencing
startQuery()after the view update would require changingupdateViewHashParamsto return theloadPageByCursorpromise (and potentially removing its internal.catch).No changes needed in
AppController.tsxunless you explicitly refactorupdateViewHashParamsto expose a promise for external sequencing.Likely an incorrect or invalid review comment.
| try { | ||
| const {logFileManagerProxy} = useLogFileManagerStore.getState(); | ||
| const pageData = await logFileManagerProxy.loadPage(cursor); | ||
| const {updatePageData} = get(); | ||
| updatePageData(pageData); | ||
| } finally { | ||
| setUiState(UI_STATE.READY); | ||
| })().catch(handleErrorWithNotification); | ||
| } |
There was a problem hiding this comment.
probably should remove this try - finally
There was a problem hiding this comment.
This one is actually coderabbitai's suggestion:
if the current loadPage fails, the uiState will remain in FAST_LOADING state, which not what we want, right?
There was a problem hiding this comment.
make sense. Even though I feel there might be better solutions than try-finally, let's merge this for now.
|
Validations:
|
kirkrodrigues
changed the title
loadPageByCursor for code reduction; Centralize uiState change in loadPageByCursor (fixes #355).loadPageByCursor for code reduction; Centralize uiState change in loadPageByCursor (fixes #355).
Description
Fixes #355.
This pull request refactors how page loading and UI state management are handled in the log viewer application. The main changes involve moving the responsibility for setting UI loading states and fetching page data into a new asynchronous
loadPageaction within the view store, which simplifies navigation logic and improves code organization.Refactoring and code organization:
loadPagemethod toViewPageActionsinsrc/stores/viewStore/types.ts, centralizing page loading logic and UI state management.createViewPageSliceinsrc/stores/viewStore/createViewPageSlice.tsto useloadPagefor both direct page loads and navigation actions, removing duplicated UI state handling and page data updates.updateViewHashParamsand related logic insrc/utils/url/urlHash.tsto use the newloadPagemethod, further simplifying async page loading and UI state transitions.Simplification and cleanup:
src/components/StatusBar/index.tsx, relying on the new centralized logic.src/utils/url/urlHash.tsby updating state and proxy settings in a more streamlined way.Checklist
breaking change.
Validation performed
Open log viewer with link 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
loadPageByCursortestOnly
updateViewHashParamsandloadPageByActioncallsloadPageByCursor, by verifying these two functions we can confirmloadPageByCursoris working.uiStatetestIn
uiStore.ts, replace the code with this snippet:where 3 is READY state and 2 is FAST_LOADING state.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor