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
This commit is contained in:
Michael Smith
2025-04-02 17:32:49 -04:00
committed by GitHub
parent 0fe7346264
commit c938bfeaab
9 changed files with 17 additions and 20 deletions
+1
View File
@@ -6,6 +6,7 @@ import { MONOSPACE_FONT_FAMILY } from "theme/constants";
export const DEFAULT_LOG_LINE_SIDE_PADDING = 24;
export interface Line {
id: number;
time: string;
output: string;
level: LogLevel;
+3 -1
View File
@@ -1,6 +1,7 @@
import type { Meta, StoryObj } from "@storybook/react";
import { chromatic } from "testHelpers/chromatic";
import { MockWorkspaceBuildLogs } from "testHelpers/entities";
import type { Line } from "./LogLine";
import { Logs } from "./Logs";
const meta: Meta<typeof Logs> = {
@@ -8,7 +9,8 @@ const meta: Meta<typeof Logs> = {
parameters: { chromatic },
component: Logs,
args: {
lines: MockWorkspaceBuildLogs.map((log) => ({
lines: MockWorkspaceBuildLogs.map<Line>((log) => ({
id: log.id,
level: log.log_level,
time: log.created_at,
output: log.output,
+1 -1
View File
@@ -20,7 +20,7 @@ export const Logs: FC<LogsProps> = ({
<div css={styles.root} className={`${className} logs-container`}>
<div css={{ minWidth: "fit-content" }}>
{lines.map((line) => (
<LogLine key={line.output} level={line.level}>
<LogLine key={line.id} level={line.level}>
{!hideTimestamps && (
<LogLinePrefix>
{dayjs(line.time).format("HH:mm:ss.SSS")}
@@ -3,13 +3,6 @@ import AnsiToHTML from "ansi-to-html";
import { type Line, LogLine, LogLinePrefix } from "components/Logs/LogLine";
import { type FC, type ReactNode, useMemo } from "react";
// Logs are stored as the Line interface to make rendering
// much more efficient. Instead of mapping objects each time, we're
// able to just pass the array of logs to the component.
export interface LineWithID extends Line {
id: number;
}
// Approximate height of a log line. Used to control virtualized list height.
export const AGENT_LOG_LINE_HEIGHT = 20;
@@ -1,19 +1,16 @@
import type { Interpolation, Theme } from "@emotion/react";
import Tooltip from "@mui/material/Tooltip";
import type { WorkspaceAgentLogSource } from "api/typesGenerated";
import type { Line } from "components/Logs/LogLine";
import { type ComponentProps, forwardRef, useMemo } from "react";
import { FixedSizeList as List } from "react-window";
import {
AGENT_LOG_LINE_HEIGHT,
AgentLogLine,
type LineWithID,
} from "./AgentLogLine";
import { AGENT_LOG_LINE_HEIGHT, AgentLogLine } from "./AgentLogLine";
type AgentLogsProps = Omit<
ComponentProps<typeof List>,
"children" | "itemSize" | "itemCount"
> & {
logs: readonly LineWithID[];
logs: readonly Line[];
sources: readonly WorkspaceAgentLogSource[];
};
@@ -1,6 +1,6 @@
// Those mocks are fetched from the Coder API in dev.coder.com
import type { LineWithID } from "./AgentLogLine";
import type { Line } from "components/Logs/LogLine";
export const MockSources = [
{
@@ -1128,4 +1128,4 @@ export const MockLogs = [
time: "2024-03-14T11:31:10.859531Z",
sourceId: "d9475581-8a42-4bce-b4d0-e4d2791d5c98",
},
] satisfies LineWithID[];
] satisfies Line[];
+2 -1
View File
@@ -12,6 +12,7 @@ import type {
WorkspaceAgentMetadata,
} from "api/typesGenerated";
import { DropdownArrow } from "components/DropdownArrow/DropdownArrow";
import type { Line } from "components/Logs/LogLine";
import { Stack } from "components/Stack/Stack";
import { useProxy } from "contexts/ProxyContext";
import {
@@ -318,7 +319,7 @@ export const AgentRow: FC<AgentRowProps> = ({
width={width}
css={styles.startupLogs}
onScroll={handleLogScroll}
logs={startupLogs.map((l) => ({
logs={startupLogs.map<Line>((l) => ({
id: l.id,
level: l.level,
output: l.output,
@@ -1,5 +1,6 @@
import { type Interpolation, type Theme, useTheme } from "@emotion/react";
import type { ProvisionerJobLog } from "api/typesGenerated";
import type { Line } from "components/Logs/LogLine";
import { DEFAULT_LOG_LINE_SIDE_PADDING, Logs } from "components/Logs/Logs";
import dayjs from "dayjs";
import { type FC, Fragment, type HTMLAttributes } from "react";
@@ -63,7 +64,8 @@ export const WorkspaceBuildLogs: FC<WorkspaceBuildLogsProps> = ({
>
{Object.entries(groupedLogsByStage).map(([stage, logs]) => {
const isEmpty = logs.every((log) => log.output === "");
const lines = logs.map((log) => ({
const lines = logs.map<Line>((log) => ({
id: log.id,
time: log.created_at,
output: log.output,
level: log.log_level,
@@ -7,6 +7,7 @@ import type {
import { Alert } from "components/Alert/Alert";
import { ErrorAlert } from "components/Alert/ErrorAlert";
import { Loader } from "components/Loader/Loader";
import type { Line } from "components/Logs/LogLine";
import { Margins } from "components/Margins/Margins";
import {
FullWidthPageHeader,
@@ -302,7 +303,7 @@ const AgentLogsContent: FC<{ workspaceId: string; agent: WorkspaceAgent }> = ({
return (
<AgentLogs
sources={agent.log_sources}
logs={logs.map((l) => ({
logs={logs.map<Line>((l) => ({
id: l.id,
output: l.output,
time: l.created_at,