From f7d276b8420f8ca700de999d4e9a8965ba9ab58e Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 25 Apr 2026 10:01:33 +0100 Subject: [PATCH] perf: cache guard inventory checks --- scripts/check-architecture-smells.mjs | 42 +++++++---- .../check-extension-plugin-sdk-boundary.mjs | 14 +++- .../check-web-fetch-provider-boundaries.mjs | 73 +++++++++++-------- scripts/lib/guard-inventory-utils.mjs | 7 +- test/architecture-smells.test.ts | 12 +-- test/scripts/ts-topology.test.ts | 65 ++++++++--------- 6 files changed, 124 insertions(+), 89 deletions(-) diff --git a/scripts/check-architecture-smells.mjs b/scripts/check-architecture-smells.mjs index 69ee12e8023..29c96527faf 100644 --- a/scripts/check-architecture-smells.mjs +++ b/scripts/check-architecture-smells.mjs @@ -20,6 +20,7 @@ import { const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); const scanRoots = resolveSourceRoots(repoRoot, ["src/plugin-sdk", "src/plugins/runtime"]); +let architectureSmellsPromise; function compareEntries(left, right) { return ( @@ -195,21 +196,32 @@ function scanRuntimeServiceLocatorSmells(sourceFile, filePath) { } export async function collectArchitectureSmells() { - const files = (await collectTypeScriptFilesFromRoots(scanRoots)).toSorted((left, right) => - normalizeRepoPath(repoRoot, left).localeCompare(normalizeRepoPath(repoRoot, right)), - ); - return await collectTypeScriptInventory({ - ts, - files, - compareEntries, - collectEntries(sourceFile, filePath) { - return [ - ...scanPluginSdkExtensionFacadeSmells(sourceFile, filePath), - ...scanRuntimeTypeImplementationSmells(sourceFile, filePath), - ...scanRuntimeServiceLocatorSmells(sourceFile, filePath), - ]; - }, - }); + if (!architectureSmellsPromise) { + architectureSmellsPromise = (async () => { + const files = (await collectTypeScriptFilesFromRoots(scanRoots)).toSorted((left, right) => + normalizeRepoPath(repoRoot, left).localeCompare(normalizeRepoPath(repoRoot, right)), + ); + return await collectTypeScriptInventory({ + ts, + files, + compareEntries, + collectEntries(sourceFile, filePath) { + return [ + ...scanPluginSdkExtensionFacadeSmells(sourceFile, filePath), + ...scanRuntimeTypeImplementationSmells(sourceFile, filePath), + ...scanRuntimeServiceLocatorSmells(sourceFile, filePath), + ]; + }, + }); + })(); + try { + return await architectureSmellsPromise; + } catch (error) { + architectureSmellsPromise = undefined; + throw error; + } + } + return await architectureSmellsPromise; } function formatInventoryHuman(inventory) { diff --git a/scripts/check-extension-plugin-sdk-boundary.mjs b/scripts/check-extension-plugin-sdk-boundary.mjs index 199fdb3ee48..b32202f5d72 100644 --- a/scripts/check-extension-plugin-sdk-boundary.mjs +++ b/scripts/check-extension-plugin-sdk-boundary.mjs @@ -101,9 +101,12 @@ async function collectParsedExtensionSourceFiles() { if (!parsedExtensionSourceFilesPromise) { parsedExtensionSourceFilesPromise = (async () => { const files = await collectExtensionSourceFiles(extensionsRoot); - return await Promise.all( + const parsed = await Promise.all( files.map(async (filePath) => { const source = await fs.readFile(filePath, "utf8"); + if (!mayContainModuleSpecifier(source)) { + return null; + } const scriptKind = filePath.endsWith(".tsx") || filePath.endsWith(".jsx") ? ts.ScriptKind.TSX @@ -120,11 +123,20 @@ async function collectParsedExtensionSourceFiles() { }; }), ); + return parsed.filter(Boolean); })(); } return await parsedExtensionSourceFilesPromise; } +function mayContainModuleSpecifier(source) { + return ( + /\bfrom\s*["']/.test(source) || + /\bimport\s*(?:\(|["']|type\b|[\w*{])/.test(source) || + /\bexport\s*(?:type\s+)?(?:\*|{)[^;\n]*\bfrom\s*["']/.test(source) + ); +} + function resolveExtensionRoot(filePath) { const relativePath = normalizeRepoPath(repoRoot, filePath); const segments = relativePath.split("/"); diff --git a/scripts/check-web-fetch-provider-boundaries.mjs b/scripts/check-web-fetch-provider-boundaries.mjs index fe61519e99f..0ad906e5ae1 100644 --- a/scripts/check-web-fetch-provider-boundaries.mjs +++ b/scripts/check-web-fetch-provider-boundaries.mjs @@ -32,40 +32,53 @@ const suspiciousPatterns = [ /id:\s*"firecrawl"/, ]; +let webFetchProviderViolationsPromise; + export async function collectWebFetchProviderBoundaryViolations() { - const violations = []; - const files = await collectSourceFileContents({ - repoRoot, - scanRoots: ["src"], - scanExtensions, - ignoredDirNames, - }); - for (const { relativeFile, content } of files) { - if ( - allowedFiles.has(relativeFile) || - relativeFile.includes(".test.") || - relativeFile.includes("test-support") - ) { - continue; - } - const lines = content.split(/\r?\n/); - for (const [index, line] of lines.entries()) { - if (!line.includes("firecrawl") && !line.includes("Firecrawl")) { - continue; - } - if (!suspiciousPatterns.some((pattern) => pattern.test(line))) { - continue; - } - violations.push({ - file: relativeFile, - line: index + 1, - reason: "core web-fetch runtime/tooling contains Firecrawl-specific fetch logic", + if (!webFetchProviderViolationsPromise) { + webFetchProviderViolationsPromise = (async () => { + const violations = []; + const files = await collectSourceFileContents({ + repoRoot, + scanRoots: ["src"], + scanExtensions, + ignoredDirNames, }); + for (const { relativeFile, content } of files) { + if ( + allowedFiles.has(relativeFile) || + relativeFile.includes(".test.") || + relativeFile.includes("test-support") + ) { + continue; + } + const lines = content.split(/\r?\n/); + for (const [index, line] of lines.entries()) { + if (!line.includes("firecrawl") && !line.includes("Firecrawl")) { + continue; + } + if (!suspiciousPatterns.some((pattern) => pattern.test(line))) { + continue; + } + violations.push({ + file: relativeFile, + line: index + 1, + reason: "core web-fetch runtime/tooling contains Firecrawl-specific fetch logic", + }); + } + } + return violations.toSorted( + (left, right) => left.file.localeCompare(right.file) || left.line - right.line, + ); + })(); + try { + return await webFetchProviderViolationsPromise; + } catch (error) { + webFetchProviderViolationsPromise = undefined; + throw error; } } - return violations.toSorted( - (left, right) => left.file.localeCompare(right.file) || left.line - right.line, - ); + return await webFetchProviderViolationsPromise; } export async function main(argv = process.argv.slice(2), io) { diff --git a/scripts/lib/guard-inventory-utils.mjs b/scripts/lib/guard-inventory-utils.mjs index b279c95c96c..a6ee41f82f6 100644 --- a/scripts/lib/guard-inventory-utils.mjs +++ b/scripts/lib/guard-inventory-utils.mjs @@ -2,6 +2,7 @@ import { promises as fs } from "node:fs"; import path from "node:path"; const parsedTypeScriptSourceCache = new Map(); +const sourceTextCache = new Map(); export function normalizeRepoPath(repoRoot, filePath) { return path.relative(repoRoot, filePath).split(path.sep).join("/"); @@ -118,7 +119,11 @@ export async function collectTypeScriptInventory(params) { const cacheKey = `${scriptKind}:${filePath}`; let sourceFile = parsedTypeScriptSourceCache.get(cacheKey); if (!sourceFile) { - const source = await fs.readFile(filePath, "utf8"); + let source = sourceTextCache.get(filePath); + if (source === undefined) { + source = await fs.readFile(filePath, "utf8"); + sourceTextCache.set(filePath, source); + } if (params.shouldParseSource && !params.shouldParseSource(source, filePath)) { continue; } diff --git a/test/architecture-smells.test.ts b/test/architecture-smells.test.ts index 7a612e53672..11239ace5cb 100644 --- a/test/architecture-smells.test.ts +++ b/test/architecture-smells.test.ts @@ -2,14 +2,14 @@ import { describe, expect, it } from "vitest"; import { collectArchitectureSmells, main } from "../scripts/check-architecture-smells.mjs"; import { createCapturedIo } from "./helpers/captured-io.js"; +const smellsPromise = collectArchitectureSmells(); + describe("architecture smell inventory", () => { it("produces stable sorted output", async () => { - const first = await collectArchitectureSmells(); - const second = await collectArchitectureSmells(); + const smells = await smellsPromise; - expect(second).toEqual(first); expect( - [...first].toSorted( + [...smells].toSorted( (left, right) => left.category.localeCompare(right.category) || left.file.localeCompare(right.file) || @@ -18,7 +18,7 @@ describe("architecture smell inventory", () => { left.specifier.localeCompare(right.specifier) || left.reason.localeCompare(right.reason), ), - ).toEqual(first); + ).toEqual(smells); }); it("script json output matches the collector", async () => { @@ -27,6 +27,6 @@ describe("architecture smell inventory", () => { expect(exitCode).toBe(0); expect(captured.readStderr()).toBe(""); - expect(JSON.parse(captured.readStdout())).toEqual(await collectArchitectureSmells()); + expect(JSON.parse(captured.readStdout())).toEqual(await smellsPromise); }); }); diff --git a/test/scripts/ts-topology.test.ts b/test/scripts/ts-topology.test.ts index 7b698b673a6..fb07f1ac84a 100644 --- a/test/scripts/ts-topology.test.ts +++ b/test/scripts/ts-topology.test.ts @@ -16,14 +16,26 @@ function buildFixtureScope() { }); } +const fixtureScope = buildFixtureScope(); +const publicSurfaceEnvelope = analyzeTopology({ + repoRoot, + scope: fixtureScope, + report: "public-surface-usage", +}); +const singleOwnerEnvelope = analyzeTopology({ + repoRoot, + scope: fixtureScope, + report: "single-owner-shared", +}); +const unusedEnvelope = analyzeTopology({ + repoRoot, + scope: fixtureScope, + report: "unused-public-surface", +}); + describe("ts-topology", () => { it("collapses canonical symbols exported by multiple public subpaths", () => { - const envelope = analyzeTopology({ - repoRoot, - scope: buildFixtureScope(), - report: "public-surface-usage", - }); - const sharedThing = envelope.records.find((record) => + const sharedThing = publicSurfaceEnvelope.records.find((record) => record.exportNames.includes("sharedThing"), ); @@ -38,16 +50,13 @@ describe("ts-topology", () => { }); it("counts renamed imports, namespace imports, type-only imports, and test-only consumers", () => { - const envelope = analyzeTopology({ - repoRoot, - scope: buildFixtureScope(), - report: "public-surface-usage", - }); - const aliasedThing = envelope.records.find((record) => + const aliasedThing = publicSurfaceEnvelope.records.find((record) => record.exportNames.includes("aliasedThing"), ); - const sharedType = envelope.records.find((record) => record.exportNames.includes("SharedType")); - const testOnlyThing = envelope.records.find((record) => + const sharedType = publicSurfaceEnvelope.records.find((record) => + record.exportNames.includes("SharedType"), + ); + const testOnlyThing = publicSurfaceEnvelope.records.find((record) => record.exportNames.includes("testOnlyThing"), ); @@ -65,33 +74,17 @@ describe("ts-topology", () => { }); it("surfaces single-owner shared and unused reports correctly", () => { - const singleOwner = analyzeTopology({ - repoRoot, - scope: buildFixtureScope(), - report: "single-owner-shared", - }); - const unused = analyzeTopology({ - repoRoot, - scope: buildFixtureScope(), - report: "unused-public-surface", - }); - - expect(singleOwner.records.map((record) => record.exportNames[0])).toContain( + expect(singleOwnerEnvelope.records.map((record) => record.exportNames[0])).toContain( "singleOwnerHelper", ); - expect(singleOwner.records.map((record) => record.exportNames[0])).not.toContain("sharedThing"); - expect(unused.records.map((record) => record.exportNames[0])).toEqual(["unusedThing"]); + expect(singleOwnerEnvelope.records.map((record) => record.exportNames[0])).not.toContain( + "sharedThing", + ); + expect(unusedEnvelope.records.map((record) => record.exportNames[0])).toEqual(["unusedThing"]); }); it("renders stable text summaries for the public-surface report", () => { - const envelope = analyzeTopology({ - repoRoot, - scope: buildFixtureScope(), - report: "public-surface-usage", - limit: 3, - }); - - expect(renderTextReport(envelope, 3)).toMatchInlineSnapshot(` + expect(renderTextReport({ ...publicSurfaceEnvelope, limit: 3 }, 3)).toMatchInlineSnapshot(` "Scope: custom Public exports analyzed: 6 Production-used exports: 4