Skip to content

⚡ Bolt: Optimize LogsView performance #142

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

Draft
Dexploarer wants to merge 1 commit into
base: develop
Choose a base branch
from
Draft

⚡ Bolt: Optimize LogsView performance #142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
112 changes: 61 additions & 51 deletions apps/app/src/components/LogsView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* Logs view component — logs viewer with filtering.
*/

import { useEffect, useMemo, useState } from "react";
import { memo, useEffect, useMemo, useState } from "react";
import { useApp } from "../AppContext";
import type { LogEntry } from "../api-client";
import { formatTime } from "./shared/format";
Expand All @@ -18,6 +18,63 @@ const TAG_COLORS: Record<string, { bg: string; fg: string }> = {
websocket: { bg: "rgba(20, 184, 166, 0.15)", fg: "rgb(20, 184, 166)" },
};

const LogEntryItem = memo(({ entry }: { entry: LogEntry }) => {
return (
<div
className="font-mono text-xs px-2 py-1 border-b border-border flex gap-2 items-baseline"
data-testid="log-entry"
>
{/* Timestamp */}
<span className="text-muted whitespace-nowrap">
{formatTime(entry.timestamp, { fallback: "—" })}
</span>

{/* Level */}
<span
className={`font-semibold w-[44px] uppercase text-[11px] ${
entry.level === "error"
? "text-danger"
: entry.level === "warn"
? "text-warn"
: "text-muted"
}`}
>
{entry.level}
</span>

{/* Source */}
<span className="text-muted w-16 overflow-hidden text-ellipsis whitespace-nowrap text-[11px]">
[{entry.source}]
</span>

{/* Tag badges */}
<span className="inline-flex gap-0.5 shrink-0">
{(entry.tags ?? []).map((t: string) => {
const c = TAG_COLORS[t];
return (
<span
key={t}
className="inline-block text-[10px] px-1.5 py-px rounded-lg mr-0.5"
style={{
background: c ? c.bg : "var(--bg-muted)",
color: c ? c.fg : "var(--muted)",
fontFamily: "var(--font-body, sans-serif)",
}}
>
{t}
</span>
);
})}
</span>

{/* Message */}
<span className="flex-1 break-all">{entry.message}</span>
Copy link

@codereviewbot-ai codereviewbot-ai bot Feb 25, 2026

Choose a reason for hiding this comment

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

⚠️ Security Issue: Log messages are rendered directly via {entry.message}. If log messages can contain HTML or script content from untrusted sources, this could expose the application to XSS vulnerabilities.

Recommendation: Ensure that log messages are sanitized or escaped before rendering. If you are certain that log messages are always plain text, this risk is mitigated, but if not, consider using a sanitization library or rendering with dangerouslySetInnerHTML only after proper sanitization.

</div>
);
});

LogEntryItem.displayName = "LogEntryItem";

export function LogsView() {
const [searchQuery, setSearchQuery] = useState("");

Expand Down Expand Up @@ -163,57 +220,10 @@ export function LogsView() {
</div>
) : (
filteredLogs.map((entry: LogEntry) => (
<div
<LogEntryItem
key={`${entry.timestamp}-${entry.source}-${entry.level}-${entry.message}`}
Copy link

@codereviewbot-ai codereviewbot-ai bot Feb 25, 2026

Choose a reason for hiding this comment

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

⚠️ Performance/Correctness Issue: The key for each LogEntryItem is constructed as ${entry.timestamp}-${entry.source}-${entry.level}-${entry.message}. If entry.message is long or not unique, this can lead to inefficient React reconciliation or even key collisions, which may cause rendering bugs.

Recommendation: If possible, use a unique, stable identifier for each log entry (such as an id field). If not available, ensure that the combination used is guaranteed to be unique and not excessively long.

Copy link

@gemini-code-assist gemini-code-assist bot Feb 25, 2026

Choose a reason for hiding this comment

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

medium

Using the full log message for the key prop can be inefficient, especially for long messages, as it increases memory usage and slows down React's reconciliation process. Since this pull request is focused on performance, it would be beneficial to optimize this.

I suggest truncating the message to a reasonable length to create a more performant key while still maintaining a high degree of uniqueness.

Suggested change
key={`${entry.timestamp}-${entry.source}-${entry.level}-${entry.message}`}
key={`${entry.timestamp}-${entry.source}-${entry.level}-${entry.message.slice(0, 100)}`}

className="font-mono text-xs px-2 py-1 border-b border-border flex gap-2 items-baseline"
data-testid="log-entry"
>
{/* Timestamp */}
<span className="text-muted whitespace-nowrap">
{formatTime(entry.timestamp, { fallback: "—" })}
</span>

{/* Level */}
<span
className={`font-semibold w-[44px] uppercase text-[11px] ${
entry.level === "error"
? "text-danger"
: entry.level === "warn"
? "text-warn"
: "text-muted"
}`}
>
{entry.level}
</span>

{/* Source */}
<span className="text-muted w-16 overflow-hidden text-ellipsis whitespace-nowrap text-[11px]">
[{entry.source}]
</span>

{/* Tag badges */}
<span className="inline-flex gap-0.5 shrink-0">
{(entry.tags ?? []).map((t: string) => {
const c = TAG_COLORS[t];
return (
<span
key={t}
className="inline-block text-[10px] px-1.5 py-px rounded-lg mr-0.5"
style={{
background: c ? c.bg : "var(--bg-muted)",
color: c ? c.fg : "var(--muted)",
fontFamily: "var(--font-body, sans-serif)",
}}
>
{t}
</span>
);
})}
</span>

{/* Message */}
<span className="flex-1 break-all">{entry.message}</span>
</div>
entry={entry}
/>
))
)}
</div>
Expand Down
85 changes: 85 additions & 0 deletions apps/app/test/components/LogsView.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import React from "react";
import { act, create, type ReactTestRenderer } from "react-test-renderer";
import { describe, expect, it, vi } from "vitest";
import * as AppContext from "../../src/AppContext";
import { LogsView } from "../../src/components/LogsView";

// Mock the AppContext
vi.mock("../../src/AppContext", () => ({
useApp: vi.fn(),
}));

describe("LogsView", () => {
it("renders logs correctly", async () => {
const mockUseApp = {
logs: [
{
timestamp: 1234567890000,
source: "test-source",
level: "info",
message: "Test log message",
tags: ["test-tag"],
},
],
logSources: ["test-source"],
logTags: ["test-tag"],
logTagFilter: "",
logLevelFilter: "",
logSourceFilter: "",
loadLogs: vi.fn(),
setState: vi.fn(),
};

// @ts-expect-error - partial mock
vi.spyOn(AppContext, "useApp").mockReturnValue(mockUseApp);
Comment on lines +33 to +34
Copy link

@gemini-code-assist gemini-code-assist bot Feb 25, 2026

Choose a reason for hiding this comment

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

medium

Using @ts-expect-error indicates a type mismatch because mockUseApp is only a partial mock of the AppContextValue type. This can make tests more brittle and harder to maintain.

A better approach would be to create a test utility function that generates a complete, type-safe mock object for useApp. This function could accept overrides for specific properties needed by each test.

For example:

import type { AppContextValue } from '../../src/AppContext';

const createMockUseApp = (overrides: Partial<AppContextValue> = {}): AppContextValue => {
  const defaultMock: AppContextValue = {
    // ... all properties with default mock values
  };
  return { ...defaultMock, ...overrides };
}

// in test
vi.spyOn(AppContext, 'useApp').mockReturnValue(createMockUseApp({ logs: mockLogs }));

This would remove the need for @ts-expect-error and improve the overall quality and maintainability of the tests.


let testRenderer: ReactTestRenderer | undefined;
await act(async () => {
testRenderer = create(<LogsView />);
});

if (!testRenderer) throw new Error("Renderer not initialized");

const root = testRenderer.root;
// Check if log entry exists
const logEntries = root.findAllByProps({ "data-testid": "log-entry" });
expect(logEntries.length).toBe(1);

// Check content
// Check for text content in the rendered tree
const treeJson = JSON.stringify(testRenderer.toJSON());
expect(treeJson).toContain("Test log message");
expect(treeJson).toContain("info");
expect(treeJson).toContain("test-source");
Comment on lines +50 to +53
Copy link

@gemini-code-assist gemini-code-assist bot Feb 25, 2026

Choose a reason for hiding this comment

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

medium

Asserting against a stringified JSON of the entire component tree is brittle. Minor changes to the component's structure, class names, or unrelated text could break this test unexpectedly.

For more robust assertions, it's better to query for specific elements or text content. With react-test-renderer, you can traverse the rendered tree to find specific nodes and check their content. This makes the test more focused on what it's trying to verify and less prone to breaking from unrelated changes.

For example, you could find the specific span containing the message and assert on its content:

const messageSpan = root.find(
  (node) => node.type === 'span' && node.props.className?.includes('flex-1')
);
expect(messageSpan.children).toEqual(['Test log message']);

});

it("renders 'No log entries' when empty", async () => {
const mockUseApp = {
logs: [],
logSources: [],
logTags: [],
logTagFilter: "",
logLevelFilter: "",
logSourceFilter: "",
loadLogs: vi.fn(),
setState: vi.fn(),
};

// @ts-expect-error - partial mock
vi.spyOn(AppContext, "useApp").mockReturnValue(mockUseApp);

let testRenderer: ReactTestRenderer | undefined;
await act(async () => {
testRenderer = create(<LogsView />);
});

if (!testRenderer) throw new Error("Renderer not initialized");

const root = testRenderer.root;
const logEntries = root.findAllByProps({ "data-testid": "log-entry" });
expect(logEntries.length).toBe(0);

const treeJson = JSON.stringify(testRenderer.toJSON());
expect(treeJson).toContain("No log entries");
});
});
Loading