fix(coding-agent/eval): fixed TypeScript type-only import rewriting
- Updated local module loading to force TS syntax stripping for .ts/.tsx/.mts modules and use the matching Bun transpiler loader. - Extended TypeScript stripping to detect `import type`/`export type` syntax and rewired wrapping to strip TS syntax after final-expression extraction with TypeScript-aware parsing. - Added tests verifying type-only imports are handled correctly in evaluator modules and rewritten code no longer contains type-only import declarations.
This commit is contained in:
can1357
@@ -492,6 +492,42 @@ display({"label": "A"})`,
|
||||
expect(reloaded.output.trim()).toBe("2");
|
||||
});
|
||||
|
||||
it("loads TypeScript type-only imports in cells and local modules", async () => {
|
||||
using tempDir = TempDir.createSync("@omp-eval-js-type-imports-");
|
||||
const sessionFile = path.join(tempDir.path(), "session.jsonl");
|
||||
const sessionId = `js-type-imports:${crypto.randomUUID()}`;
|
||||
const session = createToolSession(tempDir.path(), sessionFile);
|
||||
const typesPath = path.join(tempDir.path(), "types.ts");
|
||||
const valuesPath = path.join(tempDir.path(), "values.ts");
|
||||
const entryPath = path.join(tempDir.path(), "entry.ts");
|
||||
const typesSpec = JSON.stringify(typesPath);
|
||||
const entrySpec = JSON.stringify(entryPath);
|
||||
await Bun.write(typesPath, "export interface TypeOnly { value: number }\n");
|
||||
await Bun.write(valuesPath, "export interface InlineOnly { value: number }\nexport const imported = 41;\n");
|
||||
await Bun.write(
|
||||
entryPath,
|
||||
[
|
||||
'import type { TypeOnly } from "./types.ts";',
|
||||
'import { type InlineOnly, imported } from "./values.ts";',
|
||||
"export const typeOnly = 1;",
|
||||
"export const inlineType = imported;",
|
||||
"",
|
||||
].join("\n"),
|
||||
);
|
||||
|
||||
const result = await executeJs(
|
||||
`import type { TypeOnly } from ${typesSpec};\nconst mod = await import(${entrySpec});\nreturn mod.typeOnly + mod.inlineType;`,
|
||||
{
|
||||
sessionId,
|
||||
session,
|
||||
sessionFile,
|
||||
},
|
||||
);
|
||||
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(result.output.trim()).toBe("42");
|
||||
});
|
||||
|
||||
it("refreshes the Python tool proxy when bridge env appears after kernel warm-up", async () => {
|
||||
using tempDir = TempDir.createSync("@omp-eval-py-tool-proxy-");
|
||||
const sessionFile = path.join(tempDir.path(), "session.jsonl");
|
||||
|
||||
@@ -88,7 +88,10 @@ export class LocalModuleLoader {
|
||||
|
||||
async #buildLocalModule(modulePath: string): Promise<LocalModuleEntry> {
|
||||
const rawSource = fs.readFileSync(modulePath, "utf8");
|
||||
const stripped = stripTypeScriptSyntax(rawSource);
|
||||
const stripped = stripTypeScriptSyntax(rawSource, {
|
||||
force: isTypeScriptModulePath(modulePath),
|
||||
loader: stripLoaderForPath(modulePath),
|
||||
});
|
||||
const moduleDir = path.dirname(modulePath);
|
||||
const localDeps = new Set<string>();
|
||||
for (const specifier of collectModuleSourceSpecifiers(stripped)) {
|
||||
@@ -251,6 +254,15 @@ function isLocalPathSpecifier(source: string): boolean {
|
||||
);
|
||||
}
|
||||
|
||||
function isTypeScriptModulePath(modulePath: string): boolean {
|
||||
const ext = path.extname(modulePath);
|
||||
return ext === ".ts" || ext === ".tsx" || ext === ".mts";
|
||||
}
|
||||
|
||||
function stripLoaderForPath(modulePath: string): "ts" | "tsx" {
|
||||
return path.extname(modulePath) === ".tsx" ? "tsx" : "ts";
|
||||
}
|
||||
|
||||
function isManagedLocalModulePath(target: string): boolean {
|
||||
return (
|
||||
path.isAbsolute(target) &&
|
||||
|
||||
@@ -75,6 +75,7 @@ function parseProgram(code: string): { program: { body: ReadonlyArray<BabelProgr
|
||||
allowSuperOutsideMethod: true,
|
||||
allowUndeclaredExports: true,
|
||||
errorRecovery: true,
|
||||
plugins: ["typescript"],
|
||||
}) as unknown as { program: { body: ReadonlyArray<BabelProgramNode> } };
|
||||
} catch {
|
||||
return null;
|
||||
@@ -447,38 +448,48 @@ function requiresAsyncWrapper(code: string): boolean {
|
||||
}
|
||||
|
||||
/**
|
||||
* Strip TypeScript syntax (type annotations, `interface`, `as`, `satisfies`, generics in
|
||||
* call expressions, etc.) before the import/lexical rewriters parse the code. We use Bun's
|
||||
* native transpiler in `ts` loader mode — fast, no JSX transforms, preserves `import`/
|
||||
* `export` declarations so the downstream Babel rewrites keep working.
|
||||
* Strip TypeScript syntax (type annotations, type-only imports/exports, `interface`, `as`,
|
||||
* `satisfies`, generics in call expressions, etc.) before the import/lexical rewriters parse
|
||||
* the code. Bun's native transpiler preserves `import`/`export` declarations, so downstream
|
||||
* Babel rewrites still control module resolution.
|
||||
*
|
||||
* Skipped when the code parses as plain JavaScript already (Babel can accept it), so the
|
||||
* common case avoids an extra transpile pass. We detect "looks like TS" with a cheap regex
|
||||
* before invoking the transpiler.
|
||||
* Eval cells use a cheap "looks like TS" heuristic to avoid transpiling ordinary JS. Known
|
||||
* TypeScript modules pass `force` because a file can contain TS-only module syntax such as
|
||||
* `import type` without any value-level type annotations.
|
||||
*/
|
||||
function stripTypeScript(code: string): string {
|
||||
if (!LOOKS_LIKE_TS.test(code)) return code;
|
||||
type TypeScriptStripLoader = "ts" | "tsx";
|
||||
|
||||
const TS_TRANSPILER = new Bun.Transpiler({ loader: "ts" });
|
||||
const TSX_TRANSPILER = new Bun.Transpiler({ loader: "tsx" });
|
||||
|
||||
function stripTypeScript(code: string, options: { force?: boolean; loader?: TypeScriptStripLoader } = {}): string {
|
||||
if (!options.force && !LOOKS_LIKE_TS.test(code)) return code;
|
||||
try {
|
||||
return new Bun.Transpiler({ loader: "ts" }).transformSync(code);
|
||||
const transpiler = options.loader === "tsx" ? TSX_TRANSPILER : TS_TRANSPILER;
|
||||
return transpiler.transformSync(code);
|
||||
} catch {
|
||||
// Transpiler failed (e.g. unrecoverable syntax). Hand the original source back so the
|
||||
// downstream rewriter / VM surfaces the real error to the user.
|
||||
return code;
|
||||
}
|
||||
}
|
||||
export function stripTypeScriptSyntax(code: string): string {
|
||||
return stripTypeScript(code);
|
||||
export function stripTypeScriptSyntax(
|
||||
code: string,
|
||||
options: { force?: boolean; loader?: TypeScriptStripLoader } = {},
|
||||
): string {
|
||||
return stripTypeScript(code, options);
|
||||
}
|
||||
|
||||
// Heuristic: any of the obvious TS-only tokens. Plain JS using `as` only inside strings
|
||||
// won't match because we require a leading word boundary plus a colon/keyword neighbor.
|
||||
// Heuristic: obvious TS-only tokens, including type-only module syntax. Plain JS using `as`
|
||||
// only inside strings won't match because we require a leading word boundary plus a
|
||||
// colon/keyword neighbor.
|
||||
const LOOKS_LIKE_TS =
|
||||
/(?:\binterface\s+\w|\btype\s+\w+\s*=|\b(?:as|satisfies)\s+(?:[A-Z]|\bconst\b)|:\s*(?:string|number|boolean|any|unknown|void|never|object|[A-Z]\w*)\b|<\s*[A-Z]\w*\s*[,>])/;
|
||||
/(?:\bimport\s+type\b|\bexport\s+type\b|\b(?:import|export)\s*\{[^}\n]*\btype\s+\w|\binterface\s+\w|\btype\s+\w+\s*=|\b(?:as|satisfies)\s+(?:[A-Z]|\bconst\b)|:\s*(?:string|number|boolean|any|unknown|void|never|object|[A-Z]\w*)\b|<\s*[A-Z]\w*\s*[,>])/;
|
||||
|
||||
export function wrapCode(code: string): { source: string; asyncWrapped: boolean; finalExpressionReturned: boolean } {
|
||||
const stripped = stripTypeScript(code);
|
||||
const finalExpression = returnFinalExpression(stripped);
|
||||
const importsRewritten = rewriteImports(finalExpression.source);
|
||||
const finalExpression = returnFinalExpression(code);
|
||||
const stripped = stripTypeScript(finalExpression.source);
|
||||
const importsRewritten = rewriteImports(stripped);
|
||||
const needsAsyncWrapper = requiresAsyncWrapper(importsRewritten);
|
||||
const rewritten = {
|
||||
source: demoteTopLevelLexicals(importsRewritten, { publishGlobals: needsAsyncWrapper }),
|
||||
|
||||
@@ -130,4 +130,11 @@ describe("rewriteImports", () => {
|
||||
expect(wrapped.finalExpressionReturned).toBe(true);
|
||||
expect(wrapped.source).toContain("__omp_set_final_expr__((await Promise.resolve(1)))");
|
||||
});
|
||||
|
||||
it("strips type-only imports before rewriting imports and top-level return", () => {
|
||||
const wrapped = wrapCode(`${IMPORT} type { Thing } from "./types";\nreturn 42;`);
|
||||
expect(wrapped.finalExpressionReturned).toBe(true);
|
||||
expect(wrapped.source).toContain("__omp_set_final_expr__(42)");
|
||||
expect(wrapped.source).not.toContain(`${IMPORT} type`);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -9,8 +9,8 @@ import { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme";
|
||||
import { SILENT_ABORT_MARKER } from "@oh-my-pi/pi-coding-agent/session/messages";
|
||||
import { Text } from "@oh-my-pi/pi-tui";
|
||||
import { TempDir } from "@oh-my-pi/pi-utils";
|
||||
import type { HookSelectorSlider } from "../src/modes/components/hook-selector";
|
||||
import { ModelRegistry } from "../src/config/model-registry";
|
||||
import type { HookSelectorSlider } from "../src/modes/components/hook-selector";
|
||||
import { InteractiveMode } from "../src/modes/interactive-mode";
|
||||
import { AgentSession } from "../src/session/agent-session";
|
||||
import { AuthStorage } from "../src/session/auth-storage";
|
||||
|
||||
+2
-1
@@ -3,7 +3,8 @@
|
||||
"compilerOptions": {
|
||||
"composite": true,
|
||||
"noEmit": true,
|
||||
"emitDeclarationOnly": false
|
||||
"emitDeclarationOnly": false,
|
||||
"allowImportingTsExtensions": true
|
||||
},
|
||||
"include": ["scripts"],
|
||||
"exclude": ["node_modules"]
|
||||
|
||||
Reference in New Issue
Block a user