From a604b7f773d9807d160bda144150e0b50efd4caf Mon Sep 17 00:00:00 2001 From: Luke Parker <10430890+Hona@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:58:24 +1000 Subject: [PATCH] fix(ui): calm diff word highlights with @pierre/diffs 1.5.1 (#51236) --- bun.lock | 16 ++++-------- package.json | 2 +- .../session-ui/component-tests/file.spec.ts | 8 ++---- .../highlighting/plain.correctness.ts | 2 +- .../performance/highlighting/viewer.tsx | 4 +-- .../session-ui/src/components/file-ssr.tsx | 15 ++++++----- packages/session-ui/src/components/file.tsx | 16 ++++++------ .../src/components/session-review.tsx | 6 ++--- packages/session-ui/src/context/data.tsx | 2 +- packages/session-ui/src/pierre/index.ts | 25 +++++++++++-------- packages/session-ui/src/pierre/worker.ts | 6 ++--- 11 files changed, 48 insertions(+), 54 deletions(-) diff --git a/bun.lock b/bun.lock index e97962020ab..f36aea209a4 100644 --- a/bun.lock +++ b/bun.lock @@ -1119,7 +1119,7 @@ "@opentui/core": "0.5.12", "@opentui/keymap": "0.5.12", "@opentui/solid": "0.5.12", - "@pierre/diffs": "1.2.10", + "@pierre/diffs": "1.5.1", "@playwright/test": "1.59.1", "@sentry/solid": "10.71.0", "@sentry/vite-plugin": "5.4.0", @@ -2530,11 +2530,11 @@ "@peculiar/webcrypto": ["@peculiar/webcrypto@1.7.1", "", { "dependencies": { "@peculiar/asn1-schema": "^2.7.0", "@peculiar/json-schema": "^1.1.12", "@peculiar/utils": "^2.0.2", "tslib": "^2.8.1", "webcrypto-core": "^1.9.2" } }, "sha512-ODOov0sGMJMf3jPonOkgGqPknTsu+DdQ7kD++gz8aI+aFMOMHFbWAA2taqXXVTdP+OTOQR/znGvSpmkeI0WTYQ=="], - "@pierre/diffs": ["@pierre/diffs@1.2.10", "", { "dependencies": { "@pierre/theme": "1.0.3", "@pierre/theming": "0.0.1", "@shikijs/transformers": "^3.0.0 || ^4.0.0", "diff": "8.0.3", "hast-util-to-html": "9.0.5", "lru_map": "0.4.1", "shiki": "^3.0.0 || ^4.0.0" }, "peerDependencies": { "react": "^18.3.1 || ^19.0.0", "react-dom": "^18.3.1 || ^19.0.0" } }, "sha512-rPeAmDWarxFVTQpaf4y6wTxjZxU44xKJKoJti2zU21P06DVd9nRHZX+xSIObLB307Qjpaesyb1x/j0z94t7vLw=="], + "@pierre/diffs": ["@pierre/diffs@1.5.1", "", { "dependencies": { "@pierre/theme": "2.0.0", "@pierre/theming": "1.0.1", "@shikijs/transformers": "^3.0.0 || ^4.0.0", "diff": "9.0.0", "hast-util-to-html": "9.0.5", "lru_map": "0.4.1", "shiki": "^3.0.0 || ^4.0.0" }, "peerDependencies": { "react": "^18.3.1 || ^19.0.0", "react-dom": "^18.3.1 || ^19.0.0" }, "optionalPeers": ["react", "react-dom"] }, "sha512-+EXNfz4ZXI6FHr7P6ToWEORwyYiPVfAmzhA70xF+0y/oNvj7gbNTLJJ6jNzkBjuMbRlu564TAmgYZLc5OyHURA=="], - "@pierre/theme": ["@pierre/theme@1.0.3", "", {}, "sha512-sWHv11TMoqKxKDgTIk5VbhQjdPhs8DCcBxbjh3mRlS3YOM/OcrWoGX6MM8eBGn9cUu3M46Py0JnxsG2nJaFTuA=="], + "@pierre/theme": ["@pierre/theme@2.0.0", "", {}, "sha512-yNDd9GYLQl1mEUJR8AneJ5e4ohLIHQd/wZLWr4fagt78vS2RwwZNW530vVgHqXFAyFVcFlRmGUD5ramXH46OXw=="], - "@pierre/theming": ["@pierre/theming@0.0.1", "", { "peerDependencies": { "@pierre/theme": "^1.0.0", "@shikijs/themes": "^3.0.0 || ^4.0.0", "react": "^18.3.1 || ^19.0.0", "react-dom": "^18.3.1 || ^19.0.0", "shiki": "^3.0.0 || ^4.0.0" }, "optionalPeers": ["@pierre/theme", "@shikijs/themes", "react", "react-dom", "shiki"] }, "sha512-1thlEtJbqdyLzc1ZS2KQa1q7FzDGHT4dTEdKHoyQjOMeWWOmbVG5/ndEfOKfAb5Fzkz8cNJrOjFLiZoDH/A03A=="], + "@pierre/theming": ["@pierre/theming@1.0.1", "", { "peerDependencies": { "@pierre/theme": "^1.1.0 || ^2.0.0", "@shikijs/themes": "^3.0.0 || ^4.0.0", "react": "^18.3.1 || ^19.0.0", "react-dom": "^18.3.1 || ^19.0.0", "shiki": "^3.0.0 || ^4.0.0" }, "optionalPeers": ["@pierre/theme", "@shikijs/themes", "react", "react-dom", "shiki"] }, "sha512-WCI5Qd7iprDpISL9fBYOLe8RV53+b7mFNA3bPzl60/2CKCSrsKN8zEcep6Y3BAzvARlmca50zGjDodqPGiTUKA=="], "@pierre/trees": ["@pierre/trees@1.0.0-beta.4", "", { "dependencies": { "preact": "11.0.0-beta.0", "preact-render-to-string": "6.6.5" }, "peerDependencies": { "react": "^18.3.1 || ^19.0.0", "react-dom": "^18.3.1 || ^19.0.0" } }, "sha512-OfT1yk9ne8Te5+GB5zUY8yqE6B8BqjBHQJleH4lu8ltwNpoocZl4vXt1AzlEExpxI/pp+AFX5QG+lR3JjtTEag=="], @@ -6258,11 +6258,7 @@ "@parcel/watcher/detect-libc": ["detect-libc@1.0.3", "", { "bin": { "detect-libc": "./bin/detect-libc.js" } }, "sha512-pGjwhsmsp4kL2RTz08wcOlGN83otlqHeD/Z5T8GXZB+/YcpQ/dgo+lbU8ZsGxV0HIvqqxo9l7mqYwyYMD9bKDg=="], - "@pierre/diffs/diff": ["diff@8.0.3", "", {}, "sha512-qejHi7bcSD4hQAZE0tNAawRK1ZtafHDmMTMkrrIGgSLl7hTnQHmKCeB45xAcbfTqK2zowkM3j3bHt/4b/ARbYQ=="], - - "@pierre/diffs/react": ["react@19.2.8", "", {}, "sha512-PWaYA1L/q9u2u7xYQi+Y3L3Yfnie7XyLeaJICV1MGD6LprsBxcAqGjYyr0eY3p+QdsA+x/Irkt4Qif8D63+Sbw=="], - - "@pierre/diffs/react-dom": ["react-dom@19.2.8", "", { "dependencies": { "scheduler": "^0.27.0" }, "peerDependencies": { "react": "^19.2.8" } }, "sha512-rVprimfGBG3DR+Tq0IQG2DT5PxKth1WIGDmj5yPmlzr4YBe7uyE+Du4oVqTDXZSHGGGXRtTJEGSSePyQCMBglQ=="], + "@pierre/diffs/diff": ["diff@9.0.0", "", {}, "sha512-svtcdpS8CgJyqAjEQIXdb3OjhFVVYjzGAPO8WGCmRbrml64SPw/jJD4GoE98aR7r25A0XcgrK3F02yw9R/vhQw=="], "@pierre/trees/react": ["react@19.2.8", "", {}, "sha512-PWaYA1L/q9u2u7xYQi+Y3L3Yfnie7XyLeaJICV1MGD6LprsBxcAqGjYyr0eY3p+QdsA+x/Irkt4Qif8D63+Sbw=="], @@ -7160,8 +7156,6 @@ "@oxc-resolver/binding-wasm32-wasi/@emnapi/core/@emnapi/wasi-threads": ["@emnapi/wasi-threads@1.2.2", "", { "dependencies": { "tslib": "^2.4.0" } }, "sha512-c95qOXkHdydNKhscBTebqEC1CVAZpyqOfVfBzQ1qgzyl3gfeldUjIggDbIZgDKsHLgnsM+igH7TJ/eAasaVuMA=="], - "@pierre/diffs/react-dom/scheduler": ["scheduler@0.27.0", "", {}, "sha512-eNv+WrVbKu1f3vbYJT/xtiF5syA5HPIMtf9IgY/nKg0sWqzAUEvqY/xm7OcZc/qafLx/iO9FgOmeSAp4v5ti/Q=="], - "@pierre/trees/react-dom/scheduler": ["scheduler@0.27.0", "", {}, "sha512-eNv+WrVbKu1f3vbYJT/xtiF5syA5HPIMtf9IgY/nKg0sWqzAUEvqY/xm7OcZc/qafLx/iO9FgOmeSAp4v5ti/Q=="], "@puppeteer/browsers/yargs/cliui": ["cliui@9.0.1", "", { "dependencies": { "string-width": "^7.2.0", "strip-ansi": "^7.1.0", "wrap-ansi": "^9.0.0" } }, "sha512-k7ndgKhwoQveBL+/1tqGJYNz097I7WOvwbmmU2AR5+magtbjPWQTS1C5vzGkBC8Ym8UWRzfKUzUUqFLypY4Q+w=="], diff --git a/package.json b/package.json index ecc574473fa..4ab7bc77744 100644 --- a/package.json +++ b/package.json @@ -68,7 +68,7 @@ "@tsconfig/bun": "1.0.9", "@cloudflare/workers-types": "4.20251008.0", "@openauthjs/openauth": "0.0.0-20250322224806", - "@pierre/diffs": "1.2.10", + "@pierre/diffs": "1.5.1", "opentui-spinner": "0.0.7", "@solid-primitives/event-listener": "2.4.6", "@solid-primitives/media": "2.3.6", diff --git a/packages/session-ui/component-tests/file.spec.ts b/packages/session-ui/component-tests/file.spec.ts index c64ae01e88a..c1cd85a8d71 100644 --- a/packages/session-ui/component-tests/file.spec.ts +++ b/packages/session-ui/component-tests/file.spec.ts @@ -13,10 +13,7 @@ for (const split of [false, true]) { await expect(additions).toHaveText(["select-text"]) await expect(root.locator('[data-line-type="context"] [data-diff-span]')).toHaveCount(0) await expect(root.locator('[data-line-type="change-deletion"] [data-diff-span]')).toHaveText([ - '"http" in', - "? error.reason.", - "response?.", - ": undefined", + '"http" in error.reason ? error.reason.http?.response?.headers["x-should-retry"] : undefined', ]) await expect(additions).not.toHaveCSS("background-color", "rgba(0, 0, 0, 0)") }) @@ -27,8 +24,7 @@ for (const split of [false, true]) { const root = await mount("components-session-review--large-file", { args: { split, source } }) const line = root.locator('[data-line][data-line-type="change-addition"]') await expect(line).toHaveText("export const value = 'after'") - // Plain first paint is not proof that the worker kept inline diffs disabled. - await expect(line.locator('[style*="--syntax-"]')).not.toHaveCount(0) + await expect(line.locator('[style*="--syntax-"]')).toHaveCount(0) await expect(root.locator("[data-diff-span]")).toHaveCount(0) }) } diff --git a/packages/session-ui/performance/highlighting/plain.correctness.ts b/packages/session-ui/performance/highlighting/plain.correctness.ts index f7d27082282..f69e78ec6a3 100644 --- a/packages/session-ui/performance/highlighting/plain.correctness.ts +++ b/packages/session-ui/performance/highlighting/plain.correctness.ts @@ -13,7 +13,7 @@ test("renders large diffs as plain text without changing ordinary highlighting", await page.goto("/") await page.waitForFunction(() => !!window.highlighting) const ordinary = await page.evaluate(() => window.highlighting.mount(1)) - expect(ordinary.options).toMatchObject({ lineDiffType: "word-alt", maxLineDiffLength: 1000, tokenizeMaxLineLength: 1000 }) + expect(ordinary.options).toMatchObject({ lineDiffType: "word-line", maxLineDiffLength: 1000, tokenizeMaxLineLength: 1000 }) expect(ordinary.syntaxSpans).toBeGreaterThan(0) await expect(page.locator('[data-line="11"][data-line-type="change-addition"]')).toContainText("status: 201") }) diff --git a/packages/session-ui/performance/highlighting/viewer.tsx b/packages/session-ui/performance/highlighting/viewer.tsx index 1d0569db286..9bbb5d8725c 100644 --- a/packages/session-ui/performance/highlighting/viewer.tsx +++ b/packages/session-ui/performance/highlighting/viewer.tsx @@ -80,7 +80,7 @@ async function mount(revision: number) { const offset = messages.length const start = performance.now() active = normalize({ ...input, additions: large ? 120 : 12, deletions: large ? 120 : 12 }) - const pool = getWorkerPool(large ? "none" : "word-alt")! + const pool = getWorkerPool(large ? "none" : "word-line")! let firstReady = 0 let rendered = 0 let finish!: (value: Measurement) => void @@ -128,7 +128,7 @@ async function mount(revision: number) { export const highlighting = { mount, - configure(options: Partial) { return getWorkerPool(large ? "none" : "word-alt")!.setRenderOptions(options) }, + configure(options: Partial) { return getWorkerPool(large ? "none" : "word-line")!.setRenderOptions(options) }, unmount() { dispose?.(); dispose = undefined; host.scrollTop = 0 }, contents() { return active && { before: text(active, "deletions"), after: text(active, "additions") } }, input: { before, after: inputs.map((input) => input.after) }, diff --git a/packages/session-ui/src/components/file-ssr.tsx b/packages/session-ui/src/components/file-ssr.tsx index f192dbcfb42..6fc44d3d579 100644 --- a/packages/session-ui/src/components/file-ssr.tsx +++ b/packages/session-ui/src/components/file-ssr.tsx @@ -16,7 +16,7 @@ import { import { acquireVirtualizer, virtualMetrics } from "../pierre/virtualizer" import { File, type DiffFileProps, type FileProps } from "./file" -type DiffPreload = PreloadMultiFileDiffResult | PreloadFileDiffResult +type DiffPreload = PreloadMultiFileDiffResult | PreloadFileDiffResult type SSRDiffFileProps = DiffFileProps & { preloadedDiff: DiffPreload @@ -119,7 +119,7 @@ function DiffSSRViewer(props: SSRDiffFileProps) { // @ts-expect-error private field required for hydration fileDiffInstance.fileContainer = fileDiffRef fileDiffInstance.hydrate( - local.fileDiff + local.fileDiff || !local.before || !local.after ? { fileDiff: local.fileDiff, lineAnnotations: annotations, @@ -128,12 +128,11 @@ function DiffSSRViewer(props: SSRDiffFileProps) { prerenderedHTML: local.preloadedDiff.prerenderedHTML, } : { - oldFile: local.before - ? { ...local.before, contents: typeof local.before.contents === "string" ? local.before.contents : "" } - : local.before, - newFile: local.after - ? { ...local.after, contents: typeof local.after.contents === "string" ? local.after.contents : "" } - : local.after, + oldFile: { + ...local.before, + contents: typeof local.before.contents === "string" ? local.before.contents : "", + }, + newFile: { ...local.after, contents: typeof local.after.contents === "string" ? local.after.contents : "" }, lineAnnotations: annotations, fileContainer: fileDiffRef, containerWrapper: container, diff --git a/packages/session-ui/src/components/file.tsx b/packages/session-ui/src/components/file.tsx index 64c9f567942..2b36dd60afa 100644 --- a/packages/session-ui/src/components/file.tsx +++ b/packages/session-ui/src/components/file.tsx @@ -75,17 +75,17 @@ export type FileSearchControl = { register: (handle: FileSearchHandle | null) => void } -export type TextFileProps = FileOptions & +export type TextFileProps = FileOptions & SharedProps & { mode: "text" file: FileContents annotations?: LineAnnotation[] - preloadedDiff?: PreloadMultiFileDiffResult + preloadedDiff?: PreloadMultiFileDiffResult } -type DiffPreload = PreloadMultiFileDiffResult | PreloadFileDiffResult +type DiffPreload = PreloadMultiFileDiffResult | PreloadFileDiffResult -type DiffBaseProps = FileDiffOptions & +type DiffBaseProps = FileDiffOptions & SharedProps & { mode: "diff" annotations?: DiffLineAnnotation[] @@ -957,7 +957,7 @@ function DiffViewer(props: DiffFileProps) { let instance: FileDiff | undefined let instanceVirtualizer: Virtualizer | undefined let instanceWorkerPool: ReturnType - let instanceVirtualHunkSeparators: FileDiffOptions["hunkSeparators"] | undefined + let instanceVirtualHunkSeparators: FileDiffOptions["hunkSeparators"] | undefined let instanceFileDiff: FileDiffMetadata | undefined let instanceBefore: FileContents | undefined let instanceAfter: FileContents | undefined @@ -1066,7 +1066,7 @@ function DiffViewer(props: DiffFileProps) { lineDiffType: "none", maxLineDiffLength: 0, tokenizeMaxLineLength: 1, - } satisfies Pick, "lineDiffType" | "maxLineDiffLength" | "tokenizeMaxLineLength"> + } satisfies Pick, "lineDiffType" | "maxLineDiffLength" | "tokenizeMaxLineLength"> const lineCallbacks = createLineCallbacks({ viewer, @@ -1076,7 +1076,7 @@ function DiffViewer(props: DiffFileProps) { onLineNumberSelectionEnd: (range) => local.onLineNumberSelectionEnd?.(range), }) - const options = createMemo>(() => { + const options = createMemo>(() => { const base = { ...createDefaultOptions(props.diffStyle), ...others, @@ -1112,7 +1112,7 @@ function DiffViewer(props: DiffFileProps) { createEffect(() => { const opts = options() // Worker render options override per-viewer options, including the large-file fallback. - const workerPool = getWorkerPool(large() ? "none" : "word-alt") + const workerPool = getWorkerPool(large() ? "none" : "word-line") const virtualizer = virtuals.get() const beforeContents = typeof local.before?.contents === "string" ? local.before.contents : "" const afterContents = typeof local.after?.contents === "string" ? local.after.contents : "" diff --git a/packages/session-ui/src/components/session-review.tsx b/packages/session-ui/src/components/session-review.tsx index f3ce2cc067b..2a436093527 100644 --- a/packages/session-ui/src/components/session-review.tsx +++ b/packages/session-ui/src/components/session-review.tsx @@ -64,12 +64,12 @@ export type SessionReviewCommentActions = { export type SessionReviewFocus = { file: string; id: string } type RawReviewDiff = (PresentationFileDiff | FileDiffInfo) & { - preloaded?: PreloadMultiFileDiffResult + preloaded?: PreloadMultiFileDiffResult } type ReviewDiff = ((PresentationFileDiff & { file: string }) | FileDiffInfo) & { - preloaded?: PreloadMultiFileDiffResult + preloaded?: PreloadMultiFileDiffResult } -type Item = ViewDiff & { preloaded?: PreloadMultiFileDiffResult } +type Item = ViewDiff & { preloaded?: PreloadMultiFileDiffResult } function diff(value: unknown): value is ReviewDiff { if (!value || typeof value !== "object" || Array.isArray(value)) return false diff --git a/packages/session-ui/src/context/data.tsx b/packages/session-ui/src/context/data.tsx index d4343b3650a..0a96bf8be0b 100644 --- a/packages/session-ui/src/context/data.tsx +++ b/packages/session-ui/src/context/data.tsx @@ -32,7 +32,7 @@ type Data = { [sessionID: string]: FileDiffInfo[] } session_diff_preload?: { - [sessionID: string]: PreloadMultiFileDiffResult[] + [sessionID: string]: PreloadMultiFileDiffResult[] } } diff --git a/packages/session-ui/src/pierre/index.ts b/packages/session-ui/src/pierre/index.ts index 0c523e92e67..2b7b9fe38a2 100644 --- a/packages/session-ui/src/pierre/index.ts +++ b/packages/session-ui/src/pierre/index.ts @@ -2,7 +2,7 @@ import { DiffLineAnnotation, FileContents, FileDiffOptions, type SelectedLineRan import { ComponentProps } from "solid-js" import { lineCommentStyles } from "../components/line-comment-styles" -export type DiffProps = FileDiffOptions & { +export type DiffProps = FileDiffOptions & { before: FileContents after: FileContents annotations?: DiffLineAnnotation[] @@ -17,6 +17,11 @@ export type DiffProps = FileDiffOptions & { const unsafeCSS = ` :host { --diffs-bg: var(--opencode-diffs-bg, var(--color-background-stronger)); + /* Pierre mixes 15% in dark mode, which makes collapsed rows read as highlighted. */ + --diffs-bg-separator-override: light-dark( + color-mix(in lab, var(--diffs-bg) 98%, var(--diffs-mixer)), + color-mix(in lab, var(--diffs-bg) 95%, var(--diffs-mixer)) + ); } [data-diff], @@ -138,15 +143,15 @@ const unsafeCSS = ` } @media (max-width: 767px) { - /* File annotations share the code column; reclaim the measured number gutter. */ - [data-file] [data-line-annotation] { - margin-inline-start: calc(-1 * var(--diffs-column-number-width, 0px)); - z-index: 4; + /* File annotations share the code column; reclaim the number gutter. Pierre only measures column widths for + scrolling files with annotations, so derive the gutter from the code grid width instead. */ + [data-file][data-overflow='wrap'] [data-code] { + container-type: inline-size; } - [data-file] [data-annotation-content] { - width: var(--diffs-column-width, auto); - inset-inline-start: 0; + [data-file][data-overflow='wrap'] [data-line-annotation] { + margin-inline-start: calc(100% - 100cqi); + z-index: 4; } } @@ -198,7 +203,7 @@ ${lineCommentStyles} ` -export function createDefaultOptions(style: FileDiffOptions["diffStyle"]) { +export function createDefaultOptions(style: FileDiffOptions["diffStyle"]) { return { theme: "OpenCode", themeType: "system", @@ -210,7 +215,7 @@ export function createDefaultOptions(style: FileDiffOptions["diffStyle"]) disableBackground: false, expansionLineCount: 20, hunkSeparators: "line-info-basic", - lineDiffType: "word-alt", + lineDiffType: "word-line", maxLineDiffLength: 1000, maxLineLengthForHighlighting: 1000, disableFileHeader: true, diff --git a/packages/session-ui/src/pierre/worker.ts b/packages/session-ui/src/pierre/worker.ts index 4e2f14bf7c9..362dcb52c13 100644 --- a/packages/session-ui/src/pierre/worker.ts +++ b/packages/session-ui/src/pierre/worker.ts @@ -8,7 +8,7 @@ export function workerFactory(): Worker { return new Worker(ShikiWorkerUrl, { type: "module" }) } -function createPool(lineDiffType: "none" | "word-alt") { +function createPool(lineDiffType: "none" | "word-line") { const pool = new WorkerPoolManager( { workerFactory, @@ -36,7 +36,7 @@ function createPool(lineDiffType: "none" | "word-alt") { let plain: WorkerPoolManager | undefined let diff: WorkerPoolManager | undefined -export function getWorkerPool(lineDiffType: "none" | "word-alt" = "word-alt"): WorkerPoolManager | undefined { +export function getWorkerPool(lineDiffType: "none" | "word-line" = "word-line"): WorkerPoolManager | undefined { if (typeof window === "undefined") return if (lineDiffType === "none") { @@ -44,7 +44,7 @@ export function getWorkerPool(lineDiffType: "none" | "word-alt" = "word-alt"): W return plain } - if (!diff) diff = createPool("word-alt") + if (!diff) diff = createPool("word-line") return diff }