8000 fix: prevent invalid render output for build logs (#17233) · coder/coder@c938bfe · GitHub
[go: up one dir, main page]
Skip to content

Commit c938bfe

Browse files
ParkreinerParkreiner
authored
fix: prevent invalid render output for build logs (#17233)
## Changes made - Updated `Line` type in `LogLine.tsx` to support an ID value to prevent key conflicts during React rendering. Also deleted the `LineWithID` type, which became redundant after the change - Updated the `Logs` component to use the ID to avoid render key conflicts - Updated any component calls to add the ID as a prop ## Notes - This does prevent a bunch of extra `console.error` calls that React will automatically spit out, so this should help us a good bit in the future - Beyond being a little annoying, there was a chance (that was tiny for now) that React could accidentally mix up component instances during re-renders. That wasn't my main goal with this PR (I just wanted less noisy logs), but that should now be impossible
1 parent 0fe7346 commit c938bfe

File tree

9 files changed

+17
-20
lines changed

9 files changed

+17
-20
lines changed

‎site/src/components/Logs/LogLine.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { MONOSPACE_FONT_FAMILY } from "theme/constants";
66
export const DEFAULT_LOG_LINE_SIDE_PADDING = 24;
77

88
export interface Line {
9+
id: number;
910
time: string;
1011
output: string;
1112
level: LogLevel;

‎site/src/components/Logs/Logs.stories.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,16 @@
11
import type { Meta, StoryObj } from "@storybook/react";
22
import { chromatic } from "testHelpers/chromatic";
33
import { MockWorkspaceBuildLogs } from "testHelpers/entities";
4+
import type { Line } from "./LogLine";
45
import { Logs } from "./Logs";
56

67
const meta: Meta<typeof Logs> = {
78
title: "components/Logs",
89
parameters: { chromatic },
910
component: Logs,
1011
args: {
11-
lines: MockWorkspaceBuildLogs.map((log) => ({
12+
lines: MockWorkspaceBuildLogs.map<Line>((log) => ({
13+
id: log.id,
1214
level: log.log_level,
1315
time: log.created_at,
1416
output: log.output,

‎site/src/components/Logs/Logs.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ export const Logs: FC<LogsProps> = ({
2020
<div css={styles.root} className={`${className} logs-container`}>
2121
<div css={{ minWidth: "fit-content" }}>
2222
{lines.map((line) => (
23-
<LogLine key={line.output} level={line.level}>
23+
<LogLine key={line.id} level={line.level}>
2424
{!hideTimestamps && (
2525
<LogLinePrefix>
2626
{dayjs(line.time).format("HH:mm:ss.SSS")}

‎site/src/modules/resources/AgentLogs/AgentLogLine.tsx

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,6 @@ import AnsiToHTML from "ansi-to-html";
33
import { type Line, LogLine, LogLinePrefix } from "components/Logs/LogLine";
44
import { type FC, type ReactNode, useMemo } from "react";
55

6-
// Logs are stored as the Line interface to make rendering
7-
// much more efficient. Instead of mapping objects each time, we're
8-
// able to just pass the array of logs to the component.
9-
export interface LineWithID extends Line {
10-
id: number;
11-
}
12-
136
// Approximate height of a log line. Used to control virtualized list height.
147
export const AGENT_LOG_LINE_HEIGHT = 20;
158

‎site/src/modules/resources/AgentLogs/AgentLogs.tsx

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,16 @@
11
import type { Interpolation, Theme } from "@emotion/react";
22
import Tooltip from "@mui/material/Tooltip";
33
import type { WorkspaceAgentLogSource } from "api/typesGenerated";
4+
import type { Line } from "components/Logs/LogLine";
45
import { type ComponentProps, forwardRef, useMemo } from "react";
56
import { FixedSizeList as List } from "react-window";
6-
import {
7-
AGENT_LOG_LINE_HEIGHT,
8-
AgentLogLine,
9-
type LineWithID,
10-
} from "./AgentLogLine";
7+
import { AGENT_LOG_LINE_HEIGHT, AgentLogLine } from "./AgentLogLine";
118

129
type AgentLogsProps = Omit<
1310
ComponentProps<typeof List>,
1411
"children" | "itemSize" | "itemCount"
1512
> & {
16-
logs: readonly LineWithID[];
13+
logs: readonly Line[];
1714
sources: readonly WorkspaceAgentLogSource[];
1815
};
1916

‎site/src/modules/resources/AgentLogs/mocks.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// Those mocks are fetched from the Coder API in dev.coder.com
22

3-
import type { LineWithID } from "./AgentLogLine";
3+
import type { Line } from "components/Logs/LogLine";
44

55
export const MockSources = [
66
{
@@ -1128,4 +1128,4 @@ export const MockLogs = [
11281128
time: "2024-03-14T11:31:10.859531Z",
11291129
sourceId: "d9475581-8a42-4bce-b4d0-e4d2791d5c98",
11301130
},
1131-
] satisfies LineWithID[];
1131+
] satisfies Line[];

‎site/src/modules/resources/AgentRow.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import type {
1212
WorkspaceAgentMetadata,
1313
} from "api/typesGenerated";
1414
import { DropdownArrow } from "components/DropdownArrow/DropdownArrow";
15+
import type { Line } from "components/Logs/LogLine";
1516
import { Stack } from "components/Stack/Stack";
1617
import { useProxy } from "contexts/ProxyContext";
1718
import {
@@ -318,7 +319,7 @@ export const AgentRow: FC<AgentRowProps> = ({
318319
width={width}
319320
css={styles.startupLogs}
320321
onScroll={handleLogScroll}
321-
logs={startupLogs.map((l) => ({
322+
logs={startupLogs.map<Line>((l) => ({
322323
id: l.id,
323324
level: l.level,
324325
output: l.output,

‎site/src/modules/workspaces/WorkspaceBuildLogs/WorkspaceBuildLogs.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { type Interpolation, type Theme, useTheme } from "@emotion/react";
22
import type { ProvisionerJobLog } from "api/typesGenerated";
3+
import type { Line } from "components/Logs/LogLine";
34
import { DEFAULT_LOG_LINE_SIDE_PADDING, Logs } from "components/Logs/Logs";
45
import dayjs from "dayjs";
56
import { type FC, Fragment, type HTMLAttributes } from "react";
@@ -63,7 +64,8 @@ export const WorkspaceBuildLogs: FC<WorkspaceBuildLogsProps> = ({
6364
>
6465
{Object.entries(groupedLogsByStage).map(([stage, logs]) => {
6566
const isEmpty = logs.every((log) => log.output === "");
66-
const lines = logs.map((log) => ({
67+
const lines = logs.map<Line>((log) => ({
68+
id: log.id,
6769
time: log.created_at,
6870
output: log.output,
6971
level: log.log_level,

‎site/src/pages/WorkspaceBuildPage/WorkspaceBuildPageView.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import type {
77
import { Alert } from "components/Alert/Alert";
88
import { ErrorAlert } from "components/Alert/ErrorAlert";
99
import { Loader } from "components/Loader/Loader";
10+
import type { Line } from "components/Logs/LogLine";
1011
import { Margins } from "components/Margins/Margins";
1112
import {
1213
FullWidthPageHeader,
@@ -302,7 +303,7 @@ const AgentLogsContent: FC<{ workspaceId: string; agent: WorkspaceAgent }> = ({
302303
return (
303304
<AgentLogs
304305
sources={agent.log_sources}
305-
logs={logs.map((l) => ({
306+
logs={logs.map<Line>((l) => ({
306307
id: l.id,
307308
output: l.output,
308309
time: l.created_at,

0 commit comments

Comments
 (0)
0