Skip to content

refactor: Move logEventNum update handling from AppController to viewStore; Split viewStore into slices. #316

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.

Sign up for GitHub

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

Merged
merged 14 commits into from
Jun 23, 2025
Merged

refactor: Move logEventNum update handling from AppController to viewStore; Split viewStore into slices. #316

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
6986c36
convert setLogEventNum to updateLogEventNum
Henry8192 Jun 12, 2025
73d20e0
Merge branch 'main' into updateLogEventNum
Henry8192 Jun 12, 2025
5fea528
finish merging from main
Henry8192 Jun 12, 2025
6ea6074
enable updateUrlIfEventOnPage; changing logEventNum in the url should…
Henry8192 Jun 13, 2025
589b114
Merge branch 'main' into updateLogEventNum
Henry8192 Jun 17, 2025
7911caf
slice viewStore with update and utility slices
Henry8192 Jun 18, 2025
c0fe6ad
Merge branch 'main' into updateLogEventNum
junhaoliao Jun 18, 2025
5084b21
refactor viewStore based on features
Henry8192 Jun 19, 2025
1bfbfbe
Apply suggestions from code review
Henry8192 Jun 19, 2025
92bf12c
update handleHashChange
Henry8192 Jun 19, 2025
0d96f19
Apply suggestions from code review
Henry8192 Jun 23, 2025
11b25f2
resolve changes
Henry8192 Jun 23, 2025
745e75d
remove duplicate zustand calls for updatePageData & fix lint
Henry8192 Jun 23, 2025
b4cf71a
update default name
Henry8192 Jun 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 16 additions & 105 deletions src/components/AppController.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,92 +3,42 @@ import React, {
useRef,
} from "react";

import useLogFileManagerStore from "../stores/logFileManagerProxyStore";
import useLogFileStore from "../stores/logFileStore";
import {handleErrorWithNotification} from "../stores/notificationStore";
import useQueryStore from "../stores/queryStore";
import useUiStore from "../stores/uiStore";
import useViewStore from "../stores/viewStore";
import {Nullable} from "../typings/common";
import {UI_STATE} from "../typings/states";
import {UrlHashParams} from "../typings/url";
import {
CURSOR_CODE,
CursorType,
} from "../typings/worker";
import {
findNearestLessThanOrEqualElement,
isWithinBounds,
} from "../utils/data";
import {clamp} from "../utils/math";
import {
getWindowUrlHashParams,
getWindowUrlSearchParams,
updateWindowUrlHashParams,
URL_HASH_PARAMS_DEFAULT,
URL_SEARCH_PARAMS_DEFAULT,
} from "../utils/url";


/**
* Updates the log event number in the URL to `logEventNum` if it's within the bounds of
* `logEventNumsOnPage`.
*
* @param logEventNum
* @param logEventNumsOnPage
* @return Whether `logEventNum` is within the bounds of `logEventNumsOnPage`.
*/
const updateUrlIfEventOnPage = (
logEventNum: number,
logEventNumsOnPage: number[]
): boolean => {
if (false === isWithinBounds(logEventNumsOnPage, logEventNum)) {
return false;
}

const nearestIdx = findNearestLessThanOrEqualElement(
logEventNumsOnPage,
logEventNum
);

// Since `isWithinBounds` returned `true`, then:
// - `logEventNumsOnPage` must bound `logEventNum`.
// - `logEventNumsOnPage` cannot be empty.
// - `nearestIdx` cannot be `null`.
//
// Therefore, we can safely cast:
// - `nearestIdx` from `Nullable<number>` to `number`.
// - `logEventNumsOnPage[nearestIdx]` from `number | undefined` to `number`.
const nearestLogEventNum = logEventNumsOnPage[nearestIdx as number] as number;

updateWindowUrlHashParams({
logEventNum: nearestLogEventNum,
});

return true;
};

/**
* Updates view-related parameters from URL hash.
*
* @param hashParams
* Updates view-related states from URL hash parameters.
* NOTE: this may modify the URL parameters.
*/
const updateViewHashParams = (hashParams: UrlHashParams): void => {
const {isPrettified, logEventNum} = hashParams;
const {updateIsPrettified, setLogEventNum} = useViewStore.getState();
const updateViewHashParams = () => {
const {isPrettified, logEventNum} = getWindowUrlHashParams();
const {updateIsPrettified, updateLogEventNum} = useViewStore.getState();

updateIsPrettified(isPrettified);
setLogEventNum(logEventNum);
updateLogEventNum(logEventNum);
};

/**
* Updates query-related parameters from URL hash.
* Updates query-related states from URL hash parameters.
* NOTE: this may modify the URL parameters.
*
* @param hashParams
* @return Whether any query parameters were modified.
* @return Whether any query-related parameters were modified.
*/
const updateQueryHashParams = (hashParams: UrlHashParams): boolean => {
const {queryIsCaseSensitive, queryIsRegex, queryString} = hashParams;
const updateQueryHashParams = () => {
const {queryIsCaseSensitive, queryIsRegex, queryString} = getWindowUrlHashParams();
const {
queryIsCaseSensitive: currentQueryIsCaseSensitive,
queryIsRegex: currentQueryIsRegex,
Expand All @@ -115,22 +65,17 @@ const updateQueryHashParams = (hashParams: UrlHashParams): boolean => {
* Handles hash change events by updating the application state based on the URL hash parameters.
*
* @param [ev] The hash change event, or `null` when called on application initialization.
* @return The parsed URL hash parameters.
*/
const handleHashChange = (ev: Nullable<HashChangeEvent>): UrlHashParams => {
const hashParams = getWindowUrlHashParams();
updateViewHashParams(hashParams);
const isQueryModified = updateQueryHashParams(hashParams);
const handleHashChange = (ev: Nullable<HashChangeEvent>) => {
updateViewHashParams();
const isTriggeredByHashChange = null !== ev;
if (isTriggeredByHashChange && isQueryModified) {
if (isTriggeredByHashChange && updateQueryHashParams()) {
const {startQuery} = useQueryStore.getState();
startQuery();
}

// eslint-disable-next-line no-warning-comments
// TODO: Remove empty or falsy parameters.

return hashParams;
};

interface AppControllerProps {
Expand All @@ -145,9 +90,6 @@ interface AppControllerProps {
* @return
*/
const AppController = ({children}: AppControllerProps) => {
// States
const logEventNum = useViewStore((state) => state.logEventNum);

// Refs
const isInitialized = useRef<boolean>(false);

Expand All @@ -162,7 +104,8 @@ const AppController = ({children}: AppControllerProps) => {
isInitialized.current = true;

// Handle initial page load and maintain full URL state
const hashParams = handleHashChange(null);
handleHashChange(null);
const hashParams = getWindowUrlHashParams();
const searchParams = getWindowUrlSearchParams();
if (URL_SEARCH_PARAMS_DEFAULT.filePath !== searchParams.filePath) {
let cursor: CursorType = {code: CURSOR_CODE.LAST_EVENT, args: null};
Expand All @@ -182,38 +125,6 @@ const AppController = ({children}: AppControllerProps) => {
};
}, []);

// On `logEventNum` update, clamp it then switch page if necessary or simply update the URL.
useEffect(() => {
const {numEvents} = useLogFileStore.getState();
if (0 === numEvents || URL_HASH_PARAMS_DEFAULT.logEventNum === logEventNum) {
return;
}

const clampedLogEventNum = clamp(logEventNum, 1, numEvents);
const {beginLineNumToLogEventNum} = useViewStore.getState();
const logEventNumsOnPage: number [] = Array.from(beginLineNumToLogEventNum.values());
if (updateUrlIfEventOnPage(clampedLogEventNum, logEventNumsOnPage)) {
// No need to request a new page since the log event is on the current page.
return;
}

// If the log event is not on the current page, request a new page.
const {setUiState} = useUiStore.getState();
setUiState(UI_STATE.FAST_LOADING);
(async () => {
const {logFileManagerProxy} = useLogFileManagerStore.getState();
const cursor: CursorType = {
code: CURSOR_CODE.EVENT_NUM,
args: {eventNum: clampedLogEventNum},
};
const {isPrettified} = useViewStore.getState();

const pageData = await logFileManagerProxy.loadPage(cursor, isPrettified);
const {updatePageData} = useViewStore.getState();
updatePageData(pageData);
})().catch(handleErrorWithNotification);
}, [logEventNum]);

return children;
};

Expand Down
4 changes: 2 additions & 2 deletions src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/Result.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ const Result = ({logEventNum, message, matchRange}: ResultProps) => {

const handleResultButtonClick = useCallback(() => {
updateWindowUrlHashParams({logEventNum});
const {setLogEventNum} = useViewStore.getState();
setLogEventNum(logEventNum);
const {updateLogEventNum} = useViewStore.getState();
updateLogEventNum(logEventNum);
}, [logEventNum]);

return (
Expand Down
4 changes: 2 additions & 2 deletions src/components/Editor/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,8 @@ const Editor = () => {
return;
}
updateWindowUrlHashParams({logEventNum: newLogEventNum});
const {setLogEventNum} = useViewStore.getState();
setLogEventNum(newLogEventNum);
const {updateLogEventNum} = useViewStore.getState();
updateLogEventNum(newLogEventNum);
}, []);
Comment on lines 264 to 267
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Same redundancy as in Result.tsx

handleCursorExplicitPosChange updates the hash, then hands the same value to updateLogEventNum, which updates it again. This can bloat the browser history and cause an extra render.

Apply the same one-liner fix as suggested for Result.tsx.

🤖 Prompt for AI Agents 
In src/components/Editor/index.tsx around lines 264 to 267, the function updates
the URL hash and then redundantly updates the same value in the state, causing
extra renders and bloating browser history. Refactor the code to update the
state first and then update the URL hash in a single step or combine the updates
into one call to avoid duplication and unnecessary renders, following the
one-liner fix applied in Result.tsx.


// Synchronize `beginLineNumToLogEventNumRef` with `beginLineNumToLogEventNum`.
Expand Down
16 changes: 13 additions & 3 deletions src/stores/logFileStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ import useNotificationStore, {handleErrorWithNotification} from "./notificationS
import useQueryStore from "./queryStore";
import useUiStore from "./uiStore";
import useViewStore from "./viewStore";
import {VIEW_EVENT_DEFAULT} from "./viewStore/createViewEventSlice";
import {VIEW_PAGE_DEFAULT} from "./viewStore/createViewPageSlice";


interface LogFileValues {
Expand Down Expand Up @@ -97,6 +99,7 @@ const handleQueryResults = (progress: number, results: QueryResults) => {
mergeQueryResults(results);
};

// eslint-disable-next-line max-lines-per-function
const useLogFileStore = create<LogFileState>((set, get) => ({
...LOG_FILE_STORE_DEFAULT,
loadFile: (fileSrc: FileSrcType, cursor: CursorType) => {
Expand All @@ -110,8 +113,15 @@ const useLogFileStore = create<LogFileState>((set, get) => ({
const {setExportProgress} = useLogExportStore.getState();
setExportProgress(LOG_EXPORT_STORE_DEFAULT.exportProgress);

const {setLogData} = useViewStore.getState();
setLogData("Loading...");
const {updatePageData} = useViewStore.getState();
updatePageData({
beginLineNumToLogEventNum: VIEW_PAGE_DEFAULT.beginLineNumToLogEventNum,
cursorLineNum: 1,
logEventNum: VIEW_EVENT_DEFAULT.logEventNum,
logs: "Loading...",
numPages: VIEW_PAGE_DEFAULT.numPages,
pageNum: VIEW_PAGE_DEFAULT.pageNum,
});

set({fileSrc});
if ("string" !== typeof fileSrc) {
Expand All @@ -133,7 +143,7 @@ const useLogFileStore = create<LogFileState>((set, get) => ({

set(fileInfo);

const {isPrettified, updatePageData} = useViewStore.getState();
const {isPrettified} = useViewStore.getState();
const pageData = await logFileManagerProxy.loadPage(cursor, isPrettified);
updatePageData(pageData);

Expand Down
Loading