Skip to content

Commit b0da17c

Browse files
cdervclaude
andcommitted
Extract shared detectBrowser() and bound findCftExecutable walk
The browser detection priority chain (QUARTO_CHROMIUM → chrome-headless-shell → system Chrome) was duplicated in `getBrowserExecutablePath()` and `detectChromeForCheck()`. Extract the shared logic into `detectBrowser()` in puppeteer.ts, keeping only the context-specific legacy fallbacks in each caller. Also add a known-path check in `findCftExecutable` before falling back to `walkSync`, and bound the fallback walk to `maxDepth: 3`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 0218dbf commit b0da17c

3 files changed

Lines changed: 106 additions & 87 deletions

File tree

src/command/check/check.ts

Lines changed: 8 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,7 @@ import { typstBinaryPath } from "../../core/typst.ts";
3131
import { quartoCacheDir } from "../../core/appdirs.ts";
3232
import { isWindows } from "../../deno_ral/platform.ts";
3333
import { makeStringEnumTypeEnforcer } from "../../typing/dynamic.ts";
34-
import { findChrome } from "../../core/puppeteer.ts";
35-
import { safeExistsSync } from "../../core/path.ts";
36-
import {
37-
chromeHeadlessShellExecutablePath,
38-
chromeHeadlessShellInstallDir,
39-
readInstalledVersion,
40-
} from "../../tools/impl/chrome-headless-shell.ts";
34+
import { detectBrowser } from "../../core/puppeteer.ts";
4135
import { executionEngines } from "../../execute/engine.ts";
4236

4337
export function getTargets(): readonly string[] {
@@ -538,49 +532,18 @@ interface ChromeCheckInfo {
538532
}
539533

540534
async function detectChromeForCheck(): Promise<ChromeCheckInfo> {
541-
const result: ChromeCheckInfo = {};
542-
543-
// 1. QUARTO_CHROMIUM environment variable
544-
const envPath = Deno.env.get("QUARTO_CHROMIUM");
545-
if (envPath) {
546-
if (safeExistsSync(envPath)) {
547-
result.detected = {
548-
label: "Chrome from QUARTO_CHROMIUM",
549-
path: envPath,
550-
source: "QUARTO_CHROMIUM",
551-
};
552-
return result;
553-
}
554-
result.warning =
555-
`QUARTO_CHROMIUM is set to ${envPath} but the path does not exist.`;
556-
}
535+
const detection = await detectBrowser();
557536

558-
// 2. Chrome headless shell installed by Quarto
559-
const chromeHsPath = chromeHeadlessShellExecutablePath();
560-
if (chromeHsPath !== undefined) {
561-
const version = readInstalledVersion(chromeHeadlessShellInstallDir());
562-
result.detected = {
563-
label: "Chrome Headless Shell installed by Quarto",
564-
path: chromeHsPath,
565-
source: "quarto-chrome-headless-shell",
566-
version,
567-
};
568-
return result;
537+
if (detection.detected) {
538+
return { detected: detection.detected };
569539
}
570540

571-
// 3. System Chrome
572-
const chromeDetected = await findChrome();
573-
if (chromeDetected.path !== undefined) {
574-
result.detected = {
575-
label: "Chrome found on system",
576-
path: chromeDetected.path,
577-
source: chromeDetected.source ?? "system",
578-
displaySource: chromeDetected.source,
579-
};
580-
return result;
541+
const result: ChromeCheckInfo = {};
542+
if (detection.warning) {
543+
result.warning = detection.warning;
581544
}
582545

583-
// 4. Legacy chromium installed by Quarto
546+
// Legacy: chromium installed by Quarto
584547
const chromiumTool = installableTool("chromium");
585548
if (chromiumTool && await chromiumTool.installed()) {
586549
let path: string | undefined;
@@ -594,9 +557,7 @@ async function detectChromeForCheck(): Promise<ChromeCheckInfo> {
594557
source: "quarto",
595558
version,
596559
};
597-
return result;
598560
}
599561

600-
// 5. Not found
601562
return result;
602563
}

src/core/puppeteer.ts

Lines changed: 83 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,11 @@ import { UnreachableError } from "./lib/error.ts";
1212
import { quartoDataDir } from "./appdirs.ts";
1313
import { isMac, isWindows } from "../deno_ral/platform.ts";
1414
import puppeteer from "puppeteer";
15-
import { chromeHeadlessShellExecutablePath } from "../tools/impl/chrome-headless-shell.ts";
15+
import {
16+
chromeHeadlessShellExecutablePath,
17+
chromeHeadlessShellInstallDir,
18+
readInstalledVersion,
19+
} from "../tools/impl/chrome-headless-shell.ts";
1620

1721
// deno-lint-ignore no-explicit-any
1822
// let puppeteerImport: any = undefined;
@@ -273,59 +277,101 @@ export async function findChrome(): Promise<ChromeInfo> {
273277
return { path: path, source: source };
274278
}
275279

276-
export async function getBrowserExecutablePath() {
277-
// Cook up a new instance
278-
const browserFetcher = await fetcher();
279-
const availableRevisions = await browserFetcher.localRevisions();
280+
export interface BrowserDetection {
281+
path: string;
282+
source: string;
283+
label: string;
284+
version?: string;
285+
displaySource?: string;
286+
}
280287

281-
let executablePath: string | undefined = undefined;
288+
export interface BrowserDetectionResult {
289+
detected?: BrowserDetection;
290+
warning?: string;
291+
}
282292

283-
// Priority 1: QUARTO_CHROMIUM env var
293+
/**
294+
* Detect available Chrome/Chromium browser using the standard priority order:
295+
* 1. QUARTO_CHROMIUM env var
296+
* 2. Quarto-installed chrome-headless-shell
297+
* 3. System Chrome/Edge
298+
*
299+
* Does NOT check legacy fallbacks (puppeteer revisions, chromium tool) —
300+
* callers handle those based on their context.
301+
*/
302+
export async function detectBrowser(): Promise<BrowserDetectionResult> {
303+
const result: BrowserDetectionResult = {};
304+
305+
// 1. QUARTO_CHROMIUM environment variable
284306
const envPath = Deno.env.get("QUARTO_CHROMIUM");
285307
if (envPath) {
286308
if (safeExistsSync(envPath)) {
287-
debug(`[CHROMIUM] Using QUARTO_CHROMIUM: ${envPath}`);
288-
executablePath = envPath;
289-
} else {
290-
debug(`[CHROMIUM] QUARTO_CHROMIUM is set to ${envPath} but the path does not exist. Searching for another browser.`);
309+
result.detected = {
310+
path: envPath,
311+
source: "QUARTO_CHROMIUM",
312+
label: "Chrome from QUARTO_CHROMIUM",
313+
};
314+
return result;
291315
}
316+
result.warning =
317+
`QUARTO_CHROMIUM is set to ${envPath} but the path does not exist.`;
292318
}
293319

294-
// Priority 2: Quarto-installed chrome-headless-shell
295-
if (executablePath === undefined) {
296-
executablePath = chromeHeadlessShellExecutablePath();
297-
if (executablePath) {
298-
debug(`[CHROMIUM] Using chrome-headless-shell: ${executablePath}`);
299-
}
320+
// 2. Quarto-installed chrome-headless-shell
321+
const chromeHsPath = chromeHeadlessShellExecutablePath();
322+
if (chromeHsPath !== undefined) {
323+
const version = readInstalledVersion(chromeHeadlessShellInstallDir());
324+
result.detected = {
325+
path: chromeHsPath,
326+
source: "quarto-chrome-headless-shell",
327+
label: "Chrome Headless Shell installed by Quarto",
328+
version,
329+
};
330+
return result;
300331
}
301332

302-
// Priority 3: System Chrome/Edge
303-
if (executablePath === undefined) {
304-
const chromeInfo = await findChrome();
305-
if (chromeInfo.path) {
306-
debug(`[CHROMIUM] Using system Chrome from ${chromeInfo.source}: ${chromeInfo.path}`);
307-
executablePath = chromeInfo.path;
308-
}
333+
// 3. System Chrome/Edge
334+
const chromeInfo = await findChrome();
335+
if (chromeInfo.path) {
336+
result.detected = {
337+
path: chromeInfo.path,
338+
source: chromeInfo.source ?? "system",
339+
label: "Chrome found on system",
340+
displaySource: chromeInfo.source,
341+
};
342+
return result;
343+
}
344+
345+
return result;
346+
}
347+
348+
export async function getBrowserExecutablePath() {
349+
const detection = await detectBrowser();
350+
351+
if (detection.detected) {
352+
debug(`[CHROMIUM] Using ${detection.detected.label}: ${detection.detected.path}`);
353+
return detection.detected.path;
354+
}
355+
356+
if (detection.warning) {
357+
debug(`[CHROMIUM] ${detection.warning} Searching for another browser.`);
309358
}
310359

311-
// Priority 4: Legacy puppeteer-managed Chromium revisions
312-
if (executablePath === undefined && availableRevisions.length > 0) {
313-
// get the latest available revision
360+
// Legacy: puppeteer-managed Chromium revisions
361+
const browserFetcher = await fetcher();
362+
const availableRevisions = await browserFetcher.localRevisions();
363+
if (availableRevisions.length > 0) {
314364
availableRevisions.sort((a: string, b: string) => Number(b) - Number(a));
315365
const revision = availableRevisions[0];
316366
const revisionInfo = browserFetcher.revisionInfo(revision);
317-
executablePath = revisionInfo.executablePath;
318-
}
319-
320-
if (executablePath === undefined) {
321-
error("Chrome not found");
322-
info(
323-
"\nNo Chrome or Chromium installation was detected.\n\nPlease run 'quarto install chrome-headless-shell' to install a headless browser.\n",
324-
);
325-
throw new Error();
367+
return revisionInfo.executablePath;
326368
}
327369

328-
return executablePath;
370+
error("Chrome not found");
371+
info(
372+
"\nNo Chrome or Chromium installation was detected.\n\nPlease run 'quarto install chrome-headless-shell' to install a headless browser.\n",
373+
);
374+
throw new Error();
329375
}
330376

331377
async function fetchBrowser() {

src/tools/impl/chrome-for-testing.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@
88
* Copyright (C) 2026 Posit Software, PBC
99
*/
1010

11-
import { basename } from "../../deno_ral/path.ts";
12-
import { safeChmodSync, safeRemoveSync, walkSync } from "../../deno_ral/fs.ts";
11+
import { basename, join } from "../../deno_ral/path.ts";
12+
import { existsSync, safeChmodSync, safeRemoveSync, walkSync } from "../../deno_ral/fs.ts";
1313
import { debug } from "../../deno_ral/log.ts";
1414
import { arch, isWindows, os } from "../../deno_ral/platform.ts";
1515
import { unzip } from "../../core/zip.ts";
@@ -133,7 +133,19 @@ export function findCftExecutable(
133133
): string | undefined {
134134
const target = isWindows ? `${binaryName}.exe` : binaryName;
135135

136-
for (const entry of walkSync(extractedDir, { includeDirs: false })) {
136+
// CfT zips extract to {binaryName}-{platform}/{target}
137+
try {
138+
const { platform } = detectCftPlatform();
139+
const knownPath = join(extractedDir, `${binaryName}-${platform}`, target);
140+
if (existsSync(knownPath)) {
141+
return knownPath;
142+
}
143+
} catch {
144+
// Platform detection failed — fall through to walk
145+
}
146+
147+
// Fallback: bounded walk for unexpected directory structures
148+
for (const entry of walkSync(extractedDir, { includeDirs: false, maxDepth: 3 })) {
137149
if (basename(entry.path) === target) {
138150
return entry.path;
139151
}

0 commit comments

Comments
 (0)