diff --git a/.changeset/tsconfig-path-aliases.md b/.changeset/tsconfig-path-aliases.md new file mode 100644 index 000000000..20368c346 --- /dev/null +++ b/.changeset/tsconfig-path-aliases.md @@ -0,0 +1,9 @@ +--- +"@qawolf/cli": patch +--- + +`qawolf flows run` now resolves the tsconfig path aliases a flow imports through. A flow that imported `@utilities/gpt-helpers.ts` used to fail with `Cannot find package '@utilities/gpt-helpers.ts'`, because the local run handed the alias straight to Node, which went looking for an npm package by that name. The staged copy of your project is now rewritten so every alias becomes the equivalent relative import, which is what the platform runner does before it runs a flow. The aliases come from `compilerOptions.paths` in your project's `tsconfig.json`, the same table and the same rules the platform reads. + +Overlapping alias patterns now pick the target TypeScript picks. An exact pattern beats a wildcard, the longest matching prefix wins among wildcards, and the text after the `*` has to match too, so `@utilities/email/*` no longer loses to `@utilities/*` depending on the order the two are written in. + +An alias that still does not resolve now says so. The failure used to tell you to declare `@utilities/gpt-helpers.ts` in `package.json` "dependencies" and run npm install, which could never work. It now points at `compilerOptions.paths` in your tsconfig. diff --git a/src/commands/flows/runStagedFlows.ts b/src/commands/flows/runStagedFlows.ts index 1d48ba56e..ed90bbc35 100644 --- a/src/commands/flows/runStagedFlows.ts +++ b/src/commands/flows/runStagedFlows.ts @@ -86,6 +86,8 @@ export async function runStagedFlows( runRoot: runStagingRoot(), onInstallStart: (depCount) => ctx.ui.info(runnerMessages.installingProjectDeps(depCount)), + onTsconfigUnparsed: (dir) => + ctx.ui.warn(runnerMessages.tsconfigUnparsedNotice(dir)), }); if (staged.outerHop.mode === "install") { diff --git a/src/core/aliasImports/collectAliasSpecifiers.ts b/src/core/aliasImports/collectAliasSpecifiers.ts new file mode 100644 index 000000000..040c22320 --- /dev/null +++ b/src/core/aliasImports/collectAliasSpecifiers.ts @@ -0,0 +1,70 @@ +import type ts from "typescript"; + +export type AliasSpecifier = { + end: number; + specifier: string; + start: number; +}; + +function isRelativeSpecifier(specifier: string): boolean { + return specifier.startsWith("./") || specifier.startsWith("../"); +} + +export function collectAliasSpecifiers(options: { + sourceFile: ts.SourceFile; + typescript: typeof ts; +}): AliasSpecifier[] { + const { sourceFile, typescript: compiler } = options; + const specifiers: AliasSpecifier[] = []; + + const addCandidate = (literal: ts.StringLiteralLike): void => { + if (isRelativeSpecifier(literal.text)) return; + specifiers.push({ + end: literal.getEnd(), + specifier: literal.text, + start: literal.getStart(sourceFile), + }); + }; + + for (const statement of sourceFile.statements) { + if (compiler.isImportDeclaration(statement)) { + const phase = statement.importClause?.phaseModifier; + if (phase === compiler.SyntaxKind.TypeKeyword) continue; + if (compiler.isStringLiteral(statement.moduleSpecifier)) { + addCandidate(statement.moduleSpecifier); + } + continue; + } + + if (compiler.isExportDeclaration(statement) && !statement.isTypeOnly) { + const { moduleSpecifier } = statement; + if ( + moduleSpecifier !== undefined && + compiler.isStringLiteral(moduleSpecifier) + ) { + addCandidate(moduleSpecifier); + } + } + } + + const visit = (node: ts.Node): void => { + if ( + compiler.isCallExpression(node) && + node.expression.kind === compiler.SyntaxKind.ImportKeyword + ) { + const [firstArgument] = node.arguments; + if ( + firstArgument !== undefined && + (compiler.isStringLiteral(firstArgument) || + compiler.isNoSubstitutionTemplateLiteral(firstArgument)) + ) { + addCandidate(firstArgument); + } + return; + } + compiler.forEachChild(node, visit); + }; + visit(sourceFile); + + return specifiers; +} diff --git a/src/core/aliasImports/filePathVariants.test.ts b/src/core/aliasImports/filePathVariants.test.ts new file mode 100644 index 000000000..e88d2093e --- /dev/null +++ b/src/core/aliasImports/filePathVariants.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, it } from "bun:test"; + +import { filePathVariants } from "./filePathVariants.js"; + +describe("filePathVariants", () => { + it("tries the other source extension for a path that names one", () => { + expect(filePathVariants("src/utilities/gptHelpers.ts")).toEqual([ + "src/utilities/gptHelpers.ts", + "src/utilities/gptHelpers.js", + ]); + }); + + it("resolves a .js specifier to its TypeScript source", () => { + expect(filePathVariants("src/pages/login.js")).toEqual([ + "src/pages/login.js", + "src/pages/login.ts", + ]); + }); + + it("adds index candidates only for an extensionless path", () => { + expect(filePathVariants("src/pages/login")).toEqual([ + "src/pages/login", + "src/pages/login.ts", + "src/pages/login.js", + "src/pages/login/index.ts", + "src/pages/login/index.js", + ]); + }); + it("prefers TypeScript when a project holds both spellings of a file", () => { + const projectFiles = new Set(["src/u/foo.ts", "src/u/foo.js"]); + expect( + filePathVariants("src/u/foo").find((candidate) => + projectFiles.has(candidate), + ), + ).toBe("src/u/foo.ts"); + }); + + it("still binds the JavaScript file when the import names it", () => { + const projectFiles = new Set(["src/u/foo.ts", "src/u/foo.js"]); + expect( + filePathVariants("src/u/foo.js").find((candidate) => + projectFiles.has(candidate), + ), + ).toBe("src/u/foo.js"); + }); +}); diff --git a/src/core/aliasImports/filePathVariants.ts b/src/core/aliasImports/filePathVariants.ts new file mode 100644 index 000000000..1dd41d662 --- /dev/null +++ b/src/core/aliasImports/filePathVariants.ts @@ -0,0 +1,14 @@ +const sourceExtensionPattern = /\.(js|ts)$/; +const sourceExtensions = [".ts", ".js"]; + +export function filePathVariants(importPath: string): string[] { + const hasSourceExtension = sourceExtensionPattern.test(importPath); + const base = importPath.replace(sourceExtensionPattern, ""); + + const siblings = sourceExtensions.map((extension) => base + extension); + const indexes = hasSourceExtension + ? [] + : sourceExtensions.map((extension) => `${base}/index${extension}`); + + return [...new Set([importPath, ...siblings, ...indexes])]; +} diff --git a/src/core/aliasImports/rewriteAliasImports.test.ts b/src/core/aliasImports/rewriteAliasImports.test.ts new file mode 100644 index 000000000..9b3de4a8a --- /dev/null +++ b/src/core/aliasImports/rewriteAliasImports.test.ts @@ -0,0 +1,163 @@ +import { describe, expect, it } from "bun:test"; +import typescript from "typescript"; + +import { rewriteAliasImports } from "./rewriteAliasImports.js"; +import type { TsconfigPaths } from "./tsconfigPaths.js"; + +const tsconfigPaths: TsconfigPaths = { + "@flows/*": ["src/flows/*"], + "@pages/*": ["src/pages/*"], + "@utilities/*": ["src/utilities/*"], +}; + +const projectFilePaths = new Set([ + "src/flows/checkout.flow.ts", + "src/flows/shared.ts", + "src/pages/login/index.ts", + "src/utilities/gptHelpers.ts", + "src/utilities/legacy.js", + "src/utilities/both.ts", + "src/utilities/both.js", +]); + +function rewrite( + code: string, + importingFilePath = "src/flows/checkout.flow.ts", +): string { + return rewriteAliasImports({ + code, + importingFilePath, + projectFilePaths, + tsconfigPaths, + typescript, + }); +} + +describe("rewriteAliasImports", () => { + it("rewrites an alias naming a source file to a relative specifier", () => { + expect(rewrite('import { ask } from "@utilities/gptHelpers.ts";')).toBe( + 'import { ask } from "../utilities/gptHelpers.ts";', + ); + }); + + it("rewrites an extensionless alias", () => { + expect(rewrite('import { ask } from "@utilities/gptHelpers";')).toBe( + 'import { ask } from "../utilities/gptHelpers.ts";', + ); + }); + + it("prefixes a same-directory target so it stays a relative specifier", () => { + expect(rewrite('import { shared } from "@flows/shared.ts";')).toBe( + 'import { shared } from "./shared.ts";', + ); + }); + + it("resolves a directory alias through its index file", () => { + expect(rewrite('import { login } from "@pages/login";')).toBe( + 'import { login } from "../pages/login/index.ts";', + ); + }); + + it("resolves a .js specifier to the TypeScript file on disk", () => { + expect(rewrite('import { ask } from "@utilities/gptHelpers.js";')).toBe( + 'import { ask } from "../utilities/gptHelpers.ts";', + ); + }); + + it("rewrites a dynamic import and a re-export", () => { + expect(rewrite('const m = await import("@utilities/gptHelpers.ts");')).toBe( + 'const m = await import("../utilities/gptHelpers.ts");', + ); + expect(rewrite('export { ask } from "@utilities/gptHelpers.ts";')).toBe( + 'export { ask } from "../utilities/gptHelpers.ts";', + ); + }); + + it("keeps the quote character the source used", () => { + expect(rewrite("import { ask } from '@utilities/gptHelpers.ts';")).toBe( + "import { ask } from '../utilities/gptHelpers.ts';", + ); + }); + + it("leaves packages, builtins and subpath imports alone", () => { + const code = [ + 'import { readFile } from "node:fs/promises";', + 'import { expect } from "playwright";', + 'import { chromium } from "#playwright";', + 'import { page } from "./page.ts";', + ].join("\n"); + expect(rewrite(code)).toBe(code); + }); + + it("leaves an alias whose target is not in the project alone", () => { + const code = 'import { gone } from "@utilities/missing.ts";'; + expect(rewrite(code)).toBe(code); + }); + + it("leaves the code alone when the project declares no aliases", () => { + const code = 'import { ask } from "@utilities/gptHelpers.ts";'; + expect( + rewriteAliasImports({ + code, + importingFilePath: "src/flows/checkout.flow.ts", + projectFilePaths, + tsconfigPaths: undefined, + typescript, + }), + ).toBe(code); + }); + + it("rewrites every alias in a file without moving any other line", () => { + const code = [ + 'import { ask } from "@utilities/gptHelpers.ts";', + 'import { login } from "@pages/login";', + 'import { shared } from "@flows/shared.ts";', + "", + "export default async function run() {", + " await login();", + " return ask(shared);", + "}", + "", + ].join("\n"); + const rewritten = rewrite(code); + + expect(rewritten.split("\n")).toEqual([ + 'import { ask } from "../utilities/gptHelpers.ts";', + 'import { login } from "../pages/login/index.ts";', + 'import { shared } from "./shared.ts";', + "", + "export default async function run() {", + " await login();", + " return ask(shared);", + "}", + "", + ]); + }); + + it("rewrites relative to the importing file, not the project root", () => { + expect( + rewrite( + 'import { ask } from "@utilities/gptHelpers.ts";', + "src/pages/login/index.ts", + ), + ).toBe('import { ask } from "../../utilities/gptHelpers.ts";'); + }); + it("leaves a type-only import alone, as the platform runner does", () => { + const code = 'import type { Helper } from "@utilities/gptHelpers.ts";'; + expect(rewrite(code)).toBe(code); + }); + + it("rewrites from a file sitting at the project root", () => { + expect( + rewrite( + 'import { ask } from "@utilities/gptHelpers.ts";', + "smoke.flow.ts", + ), + ).toBe('import { ask } from "./src/utilities/gptHelpers.ts";'); + }); + it("binds the TypeScript file when the project holds both spellings", () => { + expect(rewrite('import { both } from "@utilities/both";')).toBe( + 'import { both } from "../utilities/both.ts";', + ); + }); +}); diff --git a/src/core/aliasImports/rewriteAliasImports.ts b/src/core/aliasImports/rewriteAliasImports.ts new file mode 100644 index 000000000..dae69fff5 --- /dev/null +++ b/src/core/aliasImports/rewriteAliasImports.ts @@ -0,0 +1,87 @@ +import { posix } from "node:path"; +import type ts from "typescript"; + +import { collectAliasSpecifiers } from "./collectAliasSpecifiers.js"; +import { filePathVariants } from "./filePathVariants.js"; +import { resolvePathAlias, type TsconfigPaths } from "./tsconfigPaths.js"; + +type SpecifierEdit = { end: number; relativeSpecifier: string; start: number }; + +function findTargetFilePath(options: { + aliasTarget: string; + projectFilePaths: ReadonlySet; +}): string | undefined { + return filePathVariants(posix.normalize(options.aliasTarget)).find( + (candidate) => options.projectFilePaths.has(candidate), + ); +} + +function toRelativeSpecifier(options: { + importingFilePath: string; + targetFilePath: string; +}): string { + const relativePath = posix.relative( + posix.dirname(options.importingFilePath), + options.targetFilePath, + ); + return relativePath.startsWith(".") ? relativePath : `./${relativePath}`; +} + +function applyEdit(code: string, edit: SpecifierEdit): string { + const quote = code.slice(edit.start, edit.start + 1); + return ( + code.slice(0, edit.start) + + quote + + edit.relativeSpecifier + + quote + + code.slice(edit.end) + ); +} + +export function rewriteAliasImports(options: { + code: string; + importingFilePath: string; + projectFilePaths: ReadonlySet; + tsconfigPaths: TsconfigPaths | undefined; + typescript: typeof ts; +}): string { + const { code, importingFilePath, projectFilePaths, tsconfigPaths } = options; + const compiler = options.typescript; + if (tsconfigPaths === undefined) return code; + + const sourceFile = compiler.createSourceFile( + importingFilePath, + code, + compiler.ScriptTarget.Latest, + true, + ); + + const edits = collectAliasSpecifiers({ + sourceFile, + typescript: compiler, + }).flatMap((candidate) => { + const aliasTarget = resolvePathAlias(candidate.specifier, tsconfigPaths); + if (aliasTarget === undefined) return []; + + const targetFilePath = findTargetFilePath({ + aliasTarget, + projectFilePaths, + }); + if (targetFilePath === undefined) return []; + + return [ + { + end: candidate.end, + relativeSpecifier: toRelativeSpecifier({ + importingFilePath, + targetFilePath, + }), + start: candidate.start, + }, + ]; + }); + + return edits + .sort((left, right) => right.start - left.start) + .reduce(applyEdit, code); +} diff --git a/src/core/aliasImports/tsconfigPaths.test.ts b/src/core/aliasImports/tsconfigPaths.test.ts new file mode 100644 index 000000000..587581878 --- /dev/null +++ b/src/core/aliasImports/tsconfigPaths.test.ts @@ -0,0 +1,125 @@ +import { describe, expect, it } from "bun:test"; + +import { + parseTsconfigContent, + parseTsconfigPaths, + resolvePathAlias, +} from "./tsconfigPaths.js"; + +describe("parseTsconfigPaths", () => { + it("reads compilerOptions.paths", () => { + expect( + parseTsconfigPaths('{"compilerOptions":{"paths":{"~/*":["src/*"]}}}'), + ).toEqual({ "~/*": ["src/*"] }); + }); + + it("contributes nothing rather than throwing", () => { + expect(parseTsconfigPaths("{ not json")).toBeUndefined(); + expect(parseTsconfigPaths("[]")).toBeUndefined(); + expect(parseTsconfigPaths("{}")).toBeUndefined(); + expect(parseTsconfigPaths('{"compilerOptions":{}}')).toBeUndefined(); + expect( + parseTsconfigPaths('{"compilerOptions":{"paths":"~/*"}}'), + ).toBeUndefined(); + expect( + parseTsconfigPaths('{"compilerOptions":{"paths":{"~/*":[1]}}}'), + ).toBeUndefined(); + }); +}); + +describe("parseTsconfigContent", () => { + it("separates a tsconfig that does not parse from one declaring no paths", () => { + expect( + parseTsconfigContent('{"compilerOptions":{"paths":{}} // note'), + ).toEqual({ type: "unparseable" }); + expect(parseTsconfigContent('{"compilerOptions":{"strict":true}}')).toEqual( + { + paths: undefined, + type: "parsed", + }, + ); + expect(parseTsconfigContent("{}")).toEqual({ + paths: undefined, + type: "parsed", + }); + }); + + it("reads the paths table when there is one", () => { + expect( + parseTsconfigContent('{"compilerOptions":{"paths":{"~/*":["src/*"]}}}'), + ).toEqual({ paths: { "~/*": ["src/*"] }, type: "parsed" }); + }); +}); + +describe("resolvePathAlias", () => { + const paths = { "@pages/*": ["src/pages/*"], "~/*": ["src/*"] }; + + it("substitutes the suffix into the target", () => { + expect(resolvePathAlias("~/pages/login", paths)).toBe("src/pages/login"); + expect(resolvePathAlias("@pages/login", paths)).toBe("src/pages/login"); + }); + + it("matches a pattern with no wildcard by its whole name", () => { + expect(resolvePathAlias("~config", { "~config": ["src/config.ts"] })).toBe( + "src/config.ts", + ); + }); + + it("answers nothing for an import no pattern prefixes", () => { + expect(resolvePathAlias("playwright", paths)).toBeUndefined(); + expect(resolvePathAlias("./relative", paths)).toBeUndefined(); + expect(resolvePathAlias("~/anything", undefined)).toBeUndefined(); + }); + + it("honours only the first target", () => { + expect(resolvePathAlias("~/page", { "~/*": ["first/*", "second/*"] })).toBe( + "first/page", + ); + }); + + it("answers nothing for a pattern naming no target", () => { + expect(resolvePathAlias("~/page", { "~/*": [] })).toBeUndefined(); + }); + it("prefers an exact pattern over a wildcard that also matches", () => { + expect( + resolvePathAlias("@pages/login", { + "@pages/*": ["src/pages/*"], + "@pages/login": ["src/pages/legacyLogin.ts"], + }), + ).toBe("src/pages/legacyLogin.ts"); + }); + + it("prefers the longest prefix whatever order the patterns are written in", () => { + const paths = { + "@utilities/*": ["src/utilities/*"], + "@utilities/email/*": ["src/utilities/email/*"], + }; + expect(resolvePathAlias("@utilities/email/inbox", paths)).toBe( + "src/utilities/email/inbox", + ); + expect( + resolvePathAlias("@utilities/email/inbox", { + "@utilities/email/*": ["src/utilities/email/*"], + "@utilities/*": ["src/utilities/*"], + }), + ).toBe("src/utilities/email/inbox"); + }); + + it("requires the text after the wildcard to match too", () => { + const paths = { "@lib/*.js": ["src/lib/*.js"] }; + expect(resolvePathAlias("@lib/helper.js", paths)).toBe("src/lib/helper.js"); + expect(resolvePathAlias("@lib/helper.ts", paths)).toBeUndefined(); + }); + + it("ignores a pattern carrying more than one wildcard", () => { + expect( + resolvePathAlias("@lib/a/b", { "@lib/*/*": ["src/*/*"] }), + ).toBeUndefined(); + }); + + it("matches everything through a catch-all pattern", () => { + expect(resolvePathAlias("pages/login", { "*": ["src/*"] })).toBe( + "src/pages/login", + ); + }); +}); diff --git a/src/core/aliasImports/tsconfigPaths.ts b/src/core/aliasImports/tsconfigPaths.ts new file mode 100644 index 000000000..2744c6631 --- /dev/null +++ b/src/core/aliasImports/tsconfigPaths.ts @@ -0,0 +1,119 @@ +export type TsconfigPaths = Record; + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null; +} + +function isTsconfigPaths(value: unknown): value is TsconfigPaths { + return ( + isRecord(value) && + Object.values(value).every( + (targets) => + Array.isArray(targets) && + targets.every((target) => typeof target === "string"), + ) + ); +} + +export type ParsedTsconfigContent = + | { type: "parsed"; paths: TsconfigPaths | undefined } + | { type: "unparseable" }; + +export function parseTsconfigContent( + tsconfigContent: string, +): ParsedTsconfigContent { + let parsed: unknown; + try { + parsed = JSON.parse(tsconfigContent); + } catch { + return { type: "unparseable" }; + } + if (!isRecord(parsed)) return { paths: undefined, type: "parsed" }; + + const compilerOptions = parsed["compilerOptions"]; + if (!isRecord(compilerOptions)) return { paths: undefined, type: "parsed" }; + + const paths = compilerOptions["paths"]; + return { + paths: isTsconfigPaths(paths) ? paths : undefined, + type: "parsed", + }; +} + +/** An unreadable tsconfig contributes no aliases rather than failing the run. */ +export function parseTsconfigPaths( + tsconfigContent: string, +): TsconfigPaths | undefined { + const parsed = parseTsconfigContent(tsconfigContent); + return parsed.type === "parsed" ? parsed.paths : undefined; +} + +type WildcardPattern = { prefix: string; suffix: string }; + +function parseWildcardPattern(pattern: string): WildcardPattern | undefined { + const wildcard = pattern.indexOf("*"); + if (wildcard === -1 || wildcard !== pattern.lastIndexOf("*")) + return undefined; + + return { + prefix: pattern.slice(0, wildcard), + suffix: pattern.slice(wildcard + 1), + }; +} + +function matchesWildcard( + importPath: string, + { prefix, suffix }: WildcardPattern, +): boolean { + return ( + importPath.length >= prefix.length + suffix.length && + importPath.startsWith(prefix) && + importPath.endsWith(suffix) + ); +} + +type WildcardMatch = { pattern: WildcardPattern; targets: string[] }; + +function longestPrefixMatch( + importPath: string, + paths: TsconfigPaths, +): WildcardMatch | undefined { + return Object.entries(paths) + .flatMap(([pattern, targets]) => { + const parsed = parseWildcardPattern(pattern); + if (parsed === undefined || !matchesWildcard(importPath, parsed)) { + return []; + } + return [{ pattern: parsed, targets }]; + }) + .reduce( + (best, candidate) => + best === undefined || + candidate.pattern.prefix.length > best.pattern.prefix.length + ? candidate + : best, + undefined, + ); +} + +export function resolvePathAlias( + importPath: string, + paths: TsconfigPaths | undefined, +): string | undefined { + if (paths === undefined) return undefined; + + const [exactTarget] = paths[importPath] ?? []; + if (exactTarget !== undefined) return exactTarget; + + const match = longestPrefixMatch(importPath, paths); + if (match === undefined) return undefined; + + const [target] = match.targets; + if (target === undefined) return undefined; + + const substituted = importPath.slice( + match.pattern.prefix.length, + importPath.length - match.pattern.suffix.length, + ); + return target.replace("*", substituted); +} diff --git a/src/core/errors.test.ts b/src/core/errors.test.ts index cd6bc8616..c206026b1 100644 --- a/src/core/errors.test.ts +++ b/src/core/errors.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from "bun:test"; import { errorCode, - extractMissingPackage, + extractMissingSpecifier, isNoEntError, isTimeoutError, } from "./errors.js"; @@ -62,26 +62,55 @@ describe("isNoEntError", () => { }); }); -describe("extractMissingPackage", () => { +describe("extractMissingSpecifier", () => { it("extracts the package name from an ESM resolution error", () => { const text = "Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'date-fns' imported from /x/y.js"; - expect(extractMissingPackage(text)).toBe("date-fns"); + expect(extractMissingSpecifier(text)).toEqual({ + kind: "package", + specifier: "date-fns", + }); }); it("extracts a scoped package name from a CJS resolution error", () => { - expect(extractMissingPackage("Cannot find module '@faker-js/faker'")).toBe( - "@faker-js/faker", - ); + expect( + extractMissingSpecifier("Cannot find module '@faker-js/faker'"), + ).toEqual({ kind: "package", specifier: "@faker-js/faker" }); + }); + + it("reads a bare specifier naming a source file as a path alias", () => { + const text = + "Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@utilities/gpt-helpers.ts' imported from /run/exec/src/pages/admin.ts"; + expect(extractMissingSpecifier(text)).toEqual({ + kind: "path-alias", + specifier: "@utilities/gpt-helpers.ts", + }); + }); + + it("reads every source extension a flow project can import as a path alias", () => { + for (const extension of [ + ".ts", + ".tsx", + ".mts", + ".cts", + ".js", + ".jsx", + ".mjs", + ".cjs", + ]) { + expect( + extractMissingSpecifier(`Cannot find package '~/helper${extension}'`), + ).toEqual({ kind: "path-alias", specifier: `~/helper${extension}` }); + } }); it("returns undefined for non-resolution errors", () => { - expect(extractMissingPackage("locator timeout")).toBeUndefined(); + expect(extractMissingSpecifier("locator timeout")).toBeUndefined(); }); it("returns undefined for a relative file path specifier", () => { expect( - extractMissingPackage( + extractMissingSpecifier( "Cannot find module './helper.js' imported from /x/y.js", ), ).toBeUndefined(); @@ -89,7 +118,7 @@ describe("extractMissingPackage", () => { it("returns undefined for an absolute file path specifier", () => { expect( - extractMissingPackage( + extractMissingSpecifier( "Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/run/exec/helper.js' imported from /x/y.js", ), ).toBeUndefined(); @@ -97,7 +126,7 @@ describe("extractMissingPackage", () => { it("returns undefined for a Windows drive-letter path specifier", () => { expect( - extractMissingPackage("Cannot find module 'C:\\flows\\helper.js'"), + extractMissingSpecifier("Cannot find module 'C:\\flows\\helper.js'"), ).toBeUndefined(); }); }); diff --git a/src/core/errors.ts b/src/core/errors.ts index 20e3343dc..b3d18b9a2 100644 --- a/src/core/errors.ts +++ b/src/core/errors.ts @@ -24,17 +24,20 @@ export function isTimeoutError(err: unknown): boolean { const missingPackagePattern = /Cannot find (?:package|module) '([^']+)'/; const pathLikeSpecifierPattern = /^(?:\.|\/|[A-Za-z]:[\\/])/; +const sourceFileSpecifierPattern = /\.(?:[cm]?[jt]sx?)$/; -/** - * The package name from a Node "Cannot find package 'x'" / "Cannot find - * module 'x'" resolution error text, or undefined when the text is not a - * module-resolution failure or when the specifier is a file path (relative, - * absolute, or Windows drive-letter prefix) rather than a bare package name. - */ -export function extractMissingPackage(text: string): string | undefined { +export type MissingSpecifier = + | { kind: "package"; specifier: string } + | { kind: "path-alias"; specifier: string }; + +export function extractMissingSpecifier( + text: string, +): MissingSpecifier | undefined { const specifier = missingPackagePattern.exec(text)?.[1]; if (specifier === undefined || pathLikeSpecifierPattern.test(specifier)) { return undefined; } - return specifier; + return sourceFileSpecifierPattern.test(specifier) + ? { kind: "path-alias", specifier } + : { kind: "package", specifier }; } diff --git a/src/core/flowFailureHint.test.ts b/src/core/flowFailureHint.test.ts index 7c2e2ef75..a0f4b0cb9 100644 --- a/src/core/flowFailureHint.test.ts +++ b/src/core/flowFailureHint.test.ts @@ -23,4 +23,23 @@ describe("flowFailureHint", () => { it("returns undefined for a failure that is not a module resolution error", () => { expect(flowFailureHint("locator timeout", "/proj")).toBeUndefined(); }); + it("points an unresolved path alias at tsconfig rather than at npm", () => { + const hint = flowFailureHint( + "Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@utilities/gpt-helpers.ts' imported from /run/exec/src/pages/admin.ts", + "/proj", + ); + expect(hint).toBe( + "Hint: '@utilities/gpt-helpers.ts' looks like a tsconfig path alias. Ensure \"compilerOptions.paths\" in /proj/tsconfig.json maps it to a file that exists.", + ); + }); + + it("tells the user to run from their project when an alias fails with no project dir", () => { + const hint = flowFailureHint( + "Cannot find package '@utilities/gpt-helpers.ts'", + undefined, + ); + expect(hint).toBe( + "Hint: '@utilities/gpt-helpers.ts' looks like a tsconfig path alias. Run from within your flows project so its tsconfig.json can be found.", + ); + }); }); diff --git a/src/core/flowFailureHint.ts b/src/core/flowFailureHint.ts index 3d2a95759..b330e53ae 100644 --- a/src/core/flowFailureHint.ts +++ b/src/core/flowFailureHint.ts @@ -1,4 +1,4 @@ -import { extractMissingPackage } from "./errors.js"; +import { extractMissingSpecifier } from "./errors.js"; import { runnerMessages } from "./messages/index.js"; /** Every output mode renders the same hint from the same error text. */ @@ -6,7 +6,9 @@ export function flowFailureHint( errText: string, projectDir: string | undefined, ): string | undefined { - const missingPackage = extractMissingPackage(errText); - if (missingPackage === undefined) return undefined; - return runnerMessages.moduleNotFoundHint(missingPackage, projectDir); + const missing = extractMissingSpecifier(errText); + if (missing === undefined) return undefined; + return missing.kind === "path-alias" + ? runnerMessages.pathAliasNotFoundHint(missing.specifier, projectDir) + : runnerMessages.moduleNotFoundHint(missing.specifier, projectDir); } diff --git a/src/core/interactiveRunner/getImports.ts b/src/core/interactiveRunner/getImports.ts index 33dd50feb..47e781c9b 100644 --- a/src/core/interactiveRunner/getImports.ts +++ b/src/core/interactiveRunner/getImports.ts @@ -1,6 +1,6 @@ import type ts from "typescript"; -import type { TsconfigPaths } from "./tsconfigPaths.js"; +import type { TsconfigPaths } from "~/core/aliasImports/tsconfigPaths.js"; export function getImports(options: { content: string; diff --git a/src/core/interactiveRunner/resolveImportPath.ts b/src/core/interactiveRunner/resolveImportPath.ts index dd8b7b84b..78f345eea 100644 --- a/src/core/interactiveRunner/resolveImportPath.ts +++ b/src/core/interactiveRunner/resolveImportPath.ts @@ -2,7 +2,10 @@ // slashes, so resolving with backslashes on Windows would match nothing. import { dirname, join, normalize } from "node:path/posix"; -import { resolvePathAlias, type TsconfigPaths } from "./tsconfigPaths.js"; +import { + resolvePathAlias, + type TsconfigPaths, +} from "~/core/aliasImports/tsconfigPaths.js"; /** So `.tsx`, `.json`, `.mjs` and `.cjs` imports are unreachable. */ const supportedExtensions = [".ts", ".js"]; diff --git a/src/core/interactiveRunner/tsconfigPaths.test.ts b/src/core/interactiveRunner/tsconfigPaths.test.ts deleted file mode 100644 index f1f7231b6..000000000 --- a/src/core/interactiveRunner/tsconfigPaths.test.ts +++ /dev/null @@ -1,55 +0,0 @@ -import { describe, expect, it } from "bun:test"; - -import { parseTsconfigPaths, resolvePathAlias } from "./tsconfigPaths.js"; - -describe("parseTsconfigPaths", () => { - it("reads compilerOptions.paths", () => { - expect( - parseTsconfigPaths('{"compilerOptions":{"paths":{"~/*":["src/*"]}}}'), - ).toEqual({ "~/*": ["src/*"] }); - }); - - it("contributes nothing rather than throwing", () => { - expect(parseTsconfigPaths("{ not json")).toBeUndefined(); - expect(parseTsconfigPaths("[]")).toBeUndefined(); - expect(parseTsconfigPaths("{}")).toBeUndefined(); - expect(parseTsconfigPaths('{"compilerOptions":{}}')).toBeUndefined(); - expect( - parseTsconfigPaths('{"compilerOptions":{"paths":"~/*"}}'), - ).toBeUndefined(); - expect( - parseTsconfigPaths('{"compilerOptions":{"paths":{"~/*":[1]}}}'), - ).toBeUndefined(); - }); -}); - -describe("resolvePathAlias", () => { - const paths = { "@pages/*": ["src/pages/*"], "~/*": ["src/*"] }; - - it("substitutes the suffix into the target", () => { - expect(resolvePathAlias("~/pages/login", paths)).toBe("src/pages/login"); - expect(resolvePathAlias("@pages/login", paths)).toBe("src/pages/login"); - }); - - it("matches a pattern with no wildcard by its whole name", () => { - expect(resolvePathAlias("~config", { "~config": ["src/config.ts"] })).toBe( - "src/config.ts", - ); - }); - - it("answers nothing for an import no pattern prefixes", () => { - expect(resolvePathAlias("playwright", paths)).toBeUndefined(); - expect(resolvePathAlias("./relative", paths)).toBeUndefined(); - expect(resolvePathAlias("~/anything", undefined)).toBeUndefined(); - }); - - it("honours only the first target", () => { - expect(resolvePathAlias("~/page", { "~/*": ["first/*", "second/*"] })).toBe( - "first/page", - ); - }); - - it("answers nothing for a pattern naming no target", () => { - expect(resolvePathAlias("~/page", { "~/*": [] })).toBeUndefined(); - }); -}); diff --git a/src/core/interactiveRunner/tsconfigPaths.ts b/src/core/interactiveRunner/tsconfigPaths.ts deleted file mode 100644 index 4768bfd94..000000000 --- a/src/core/interactiveRunner/tsconfigPaths.ts +++ /dev/null @@ -1,56 +0,0 @@ -export type TsconfigPaths = Record; - -function isRecord(value: unknown): value is Record { - return typeof value === "object" && value !== null; -} - -function isTsconfigPaths(value: unknown): value is TsconfigPaths { - return ( - isRecord(value) && - Object.values(value).every( - (targets) => - Array.isArray(targets) && - targets.every((target) => typeof target === "string"), - ) - ); -} - -/** An unreadable tsconfig contributes no aliases rather than failing the run. */ -export function parseTsconfigPaths( - tsconfigContent: string, -): TsconfigPaths | undefined { - let parsed: unknown; - try { - parsed = JSON.parse(tsconfigContent); - } catch { - return undefined; - } - if (!isRecord(parsed)) return undefined; - - const compilerOptions = parsed["compilerOptions"]; - if (!isRecord(compilerOptions)) return undefined; - - const paths = compilerOptions["paths"]; - return isTsconfigPaths(paths) ? paths : undefined; -} - -/** - * Only the first target of the first matching pattern, as the socket path does. - * Honouring the rest would ship a different file set than a socket run. - */ -export function resolvePathAlias( - importPath: string, - paths: TsconfigPaths | undefined, -): string | undefined { - if (paths === undefined) return undefined; - - for (const [pattern, targets] of Object.entries(paths)) { - const prefix = pattern.replace("*", ""); - if (!importPath.startsWith(prefix)) continue; - const [target] = targets; - if (target !== undefined) { - return target.replace("*", importPath.slice(prefix.length)); - } - } - return undefined; -} diff --git a/src/core/messages/runner.ts b/src/core/messages/runner.ts index 242eecac5..b334894f5 100644 --- a/src/core/messages/runner.ts +++ b/src/core/messages/runner.ts @@ -76,4 +76,10 @@ export const runnerMessages = { projectDir === undefined ? `Hint: '${pkg}' could not be resolved. Run from within your flows project so its dependencies can be found.` : `Hint: '${pkg}' could not be resolved. Ensure it is declared in ${projectDir}/package.json "dependencies" and run npm install in that project.`, + tsconfigUnparsedNotice: (projectDir: string) => + `Not resolving path aliases: ${projectDir}/tsconfig.json is not valid JSON. Comments and trailing commas are not supported, here or on the platform. Remove them so "compilerOptions.paths" can be read.`, + pathAliasNotFoundHint: (specifier: string, projectDir: string | undefined) => + projectDir === undefined + ? `Hint: '${specifier}' looks like a tsconfig path alias. Run from within your flows project so its tsconfig.json can be found.` + : `Hint: '${specifier}' looks like a tsconfig path alias. Ensure "compilerOptions.paths" in ${projectDir}/tsconfig.json maps it to a file that exists.`, } as const; diff --git a/src/domains/runtimeEnv/prepareRunDir.ts b/src/domains/runtimeEnv/prepareRunDir.ts index 7db585bb6..eb7fa9eef 100644 --- a/src/domains/runtimeEnv/prepareRunDir.ts +++ b/src/domains/runtimeEnv/prepareRunDir.ts @@ -6,6 +6,7 @@ import { type Fs, makeDefaultFs } from "~/shell/fs.js"; import { writeExecSubpathImports } from "./execSubpathImports.js"; import { populateInnerHop } from "./innerHop.js"; import { type OuterHopResult, populateOuterHop } from "./outerHop.js"; +import { rewriteStagedAliases } from "./rewriteStagedAliases.js"; import { stageFlowFiles } from "./stageFlowFiles.js"; export type PrepareRunDirArgs = { @@ -17,6 +18,7 @@ export type PrepareRunDirArgs = { fs?: Fs; // Forwarded to populateOuterHop — fires just before a fallback npm install. onInstallStart?: (depCount: number) => void; + onTsconfigUnparsed?: (projectDir: string) => void; }; export type PrepareRunDirResult = { @@ -56,6 +58,14 @@ export async function prepareRunDir( // stage bare files that never use the "#playwright" alias. if (projectDir !== undefined) { await writeExecSubpathImports({ execDir, fs }); + const onTsconfigUnparsed = args.onTsconfigUnparsed; + await rewriteStagedAliases({ + execDir, + fs, + ...(onTsconfigUnparsed !== undefined + ? { onTsconfigUnparsed: () => onTsconfigUnparsed(projectDir) } + : {}), + }); } const outerHop = await populateOuterHop({ diff --git a/src/domains/runtimeEnv/rewriteStagedAliases.test.ts b/src/domains/runtimeEnv/rewriteStagedAliases.test.ts new file mode 100644 index 000000000..cad846c19 --- /dev/null +++ b/src/domains/runtimeEnv/rewriteStagedAliases.test.ts @@ -0,0 +1,158 @@ +import { describe, expect, it } from "bun:test"; +import { posix } from "node:path"; + +import { type Fs } from "~/shell/fs.js"; +import { makeMemoryFs } from "~/shell/fs.testUtils.js"; + +import { rewriteStagedAliases } from "./rewriteStagedAliases.js"; + +const execDir = "/run/exec"; + +const aliasTsconfig = JSON.stringify({ + compilerOptions: { paths: { "@utilities/*": ["src/utilities/*"] } }, +}); + +async function stage(tree: Record): Promise { + const fs = makeMemoryFs(); + for (const [path, content] of Object.entries(tree)) { + const absolute = posix.join(execDir, path); + await fs.mkdir(posix.dirname(absolute), { recursive: true }); + await fs.writeFile(absolute, content); + } + return fs; +} + +describe("rewriteStagedAliases", () => { + it("rewrites an alias import in every staged file that uses one", async () => { + const fs = await stage({ + "src/flows/checkout.flow.ts": 'import { ask } from "@utilities/gpt.ts";', + "src/pages/admin.ts": 'import { ask } from "@utilities/gpt.ts";', + "src/utilities/gpt.ts": "export const ask = 1;", + "tsconfig.json": aliasTsconfig, + }); + + expect(await rewriteStagedAliases({ execDir, fs })).toEqual([ + "src/flows/checkout.flow.ts", + "src/pages/admin.ts", + ]); + expect(await fs.readFile(`${execDir}/src/flows/checkout.flow.ts`)).toBe( + 'import { ask } from "../utilities/gpt.ts";', + ); + expect(await fs.readFile(`${execDir}/src/pages/admin.ts`)).toBe( + 'import { ask } from "../utilities/gpt.ts";', + ); + }); + + it("leaves the tree alone when no tsconfig was staged", async () => { + const fs = await stage({ + "src/flows/checkout.flow.ts": 'import { ask } from "@utilities/gpt.ts";', + "src/utilities/gpt.ts": "export const ask = 1;", + }); + + expect(await rewriteStagedAliases({ execDir, fs })).toEqual([]); + expect(await fs.readFile(`${execDir}/src/flows/checkout.flow.ts`)).toBe( + 'import { ask } from "@utilities/gpt.ts";', + ); + }); + + it("leaves the tree alone for a tsconfig declaring no paths", async () => { + const fs = await stage({ + "src/flows/checkout.flow.ts": 'import { ask } from "@utilities/gpt.ts";', + "tsconfig.json": '{"compilerOptions":{"strict":true}}', + }); + + expect(await rewriteStagedAliases({ execDir, fs })).toEqual([]); + }); + + it("reports a tsconfig that does not parse rather than silently resolving nothing", async () => { + const fs = await stage({ + "src/flows/checkout.flow.ts": 'import { ask } from "@utilities/gpt.ts";', + "tsconfig.json": '{"compilerOptions":{"paths":{}} // trailing comment', + }); + let reported = 0; + + expect( + await rewriteStagedAliases({ + execDir, + fs, + onTsconfigUnparsed: () => { + reported += 1; + }, + }), + ).toEqual([]); + expect(reported).toBe(1); + }); + + it("stays quiet when the project declares no paths", async () => { + const fs = await stage({ + "src/flows/checkout.flow.ts": 'import { ask } from "@utilities/gpt.ts";', + "tsconfig.json": '{"compilerOptions":{"strict":true}}', + }); + let reported = 0; + + await rewriteStagedAliases({ + execDir, + fs, + onTsconfigUnparsed: () => { + reported += 1; + }, + }); + expect(reported).toBe(0); + }); + + it("stays quiet when the project staged no tsconfig at all", async () => { + const fs = await stage({ + "src/flows/checkout.flow.ts": 'import { ask } from "@utilities/gpt.ts";', + }); + let reported = 0; + + await rewriteStagedAliases({ + execDir, + fs, + onTsconfigUnparsed: () => { + reported += 1; + }, + }); + expect(reported).toBe(0); + }); + + it("leaves a file mentioning no alias untouched", async () => { + const source = 'import { expect } from "playwright";'; + const fs = await stage({ + "src/flows/checkout.flow.ts": source, + "tsconfig.json": aliasTsconfig, + }); + + expect(await rewriteStagedAliases({ execDir, fs })).toEqual([]); + expect(await fs.readFile(`${execDir}/src/flows/checkout.flow.ts`)).toBe( + source, + ); + }); + + it("does not descend into node_modules", async () => { + const vendored = 'import { ask } from "@utilities/gpt.ts";'; + const fs = await stage({ + "node_modules/vendor/index.ts": vendored, + "src/utilities/gpt.ts": "export const ask = 1;", + "tsconfig.json": aliasTsconfig, + }); + + expect(await rewriteStagedAliases({ execDir, fs })).toEqual([]); + expect(await fs.readFile(`${execDir}/node_modules/vendor/index.ts`)).toBe( + vendored, + ); + }); + + it("leaves an alias resolving to no staged file alone", async () => { + const source = 'import { ask } from "@utilities/missing.ts";'; + const fs = await stage({ + "src/flows/checkout.flow.ts": source, + "tsconfig.json": aliasTsconfig, + }); + + expect(await rewriteStagedAliases({ execDir, fs })).toEqual([]); + expect(await fs.readFile(`${execDir}/src/flows/checkout.flow.ts`)).toBe( + source, + ); + }); +}); diff --git a/src/domains/runtimeEnv/rewriteStagedAliases.ts b/src/domains/runtimeEnv/rewriteStagedAliases.ts new file mode 100644 index 000000000..cff567475 --- /dev/null +++ b/src/domains/runtimeEnv/rewriteStagedAliases.ts @@ -0,0 +1,118 @@ +import { join } from "node:path"; + +import { rewriteAliasImports } from "~/core/aliasImports/rewriteAliasImports.js"; +import { + type ParsedTsconfigContent, + parseTsconfigContent, +} from "~/core/aliasImports/tsconfigPaths.js"; +import { batchMap, flowBatchSize } from "~/core/batchMap.js"; +import { isNoEntError } from "~/core/errors.js"; +import type { Fs } from "~/shell/fs.js"; + +const skippedDirs = new Set(["node_modules", ".git", ".qawolf"]); +const sourceExtensions = [".ts", ".js"]; + +export type RewriteStagedAliasesArgs = { + execDir: string; + fs: Fs; + onTsconfigUnparsed?: () => void; +}; + +export async function rewriteStagedAliases( + args: RewriteStagedAliasesArgs, +): Promise { + const tsconfig = await readStagedTsconfig(args); + if (tsconfig.type === "unparseable") { + args.onTsconfigUnparsed?.(); + return []; + } + if (tsconfig.type === "absent" || tsconfig.paths === undefined) return []; + const tsconfigPaths = tsconfig.paths; + + const stagedPaths = await listStagedSourceFiles({ + dir: args.execDir, + fs: args.fs, + prefix: "", + }); + const projectFilePaths = new Set(stagedPaths); + const aliasPrefixes = Object.keys(tsconfigPaths).map( + (pattern) => pattern.split("*")[0] ?? pattern, + ); + const mentionsAnAlias = (code: string) => + aliasPrefixes.some((prefix) => code.includes(prefix)); + const { default: typescript } = await import("typescript"); + + const rewriteOne = async ( + stagedPath: string, + ): Promise => { + const absolutePath = join(args.execDir, stagedPath); + const code = await args.fs.readFile(absolutePath); + if (!mentionsAnAlias(code)) return undefined; + + const rewritten = rewriteAliasImports({ + code, + importingFilePath: stagedPath, + projectFilePaths, + tsconfigPaths, + typescript, + }); + if (rewritten === code) return undefined; + + await args.fs.writeFile(absolutePath, rewritten); + return stagedPath; + }; + + const rewrittenPaths: string[] = []; + for await (const stagedPath of batchMap( + stagedPaths, + rewriteOne, + flowBatchSize, + )) { + if (stagedPath !== undefined) rewrittenPaths.push(stagedPath); + } + return rewrittenPaths; +} + +type StagedTsconfig = ParsedTsconfigContent | { type: "absent" }; + +async function readStagedTsconfig(options: { + execDir: string; + fs: Fs; +}): Promise { + let content: string; + try { + content = await options.fs.readFile(join(options.execDir, "tsconfig.json")); + } catch (err) { + if (isNoEntError(err)) return { type: "absent" }; + throw err; + } + return parseTsconfigContent(content); +} + +async function listStagedSourceFiles(options: { + dir: string; + fs: Fs; + prefix: string; +}): Promise { + const entries = await options.fs.readdirWithTypes(options.dir); + const nested = await Promise.all( + entries.map(async (entry) => { + if (skippedDirs.has(entry.name)) return []; + const stagedPath = + options.prefix === "" ? entry.name : `${options.prefix}/${entry.name}`; + + if (entry.isDirectory()) { + return listStagedSourceFiles({ + dir: join(options.dir, entry.name), + fs: options.fs, + prefix: stagedPath, + }); + } + const isSource = sourceExtensions.some((extension) => + entry.name.endsWith(extension), + ); + return entry.isFile() && isSource ? [stagedPath] : []; + }), + ); + return nested.flat(); +} diff --git a/src/shell/interactiveRunner/collectRunFiles.ts b/src/shell/interactiveRunner/collectRunFiles.ts index 40ba36f33..0b57baa0d 100644 Binary files a/src/shell/interactiveRunner/collectRunFiles.ts and b/src/shell/interactiveRunner/collectRunFiles.ts differ