Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@
"build:wasm": "node scripts/build-wasm.js",
"typecheck": "tsc --noEmit",
"verify-imports": "node scripts/verify-imports.js",
"test": "vitest run",
"test:watch": "vitest",
"test:coverage": "vitest run --coverage",
"test": "node scripts/test.js run",
"test:watch": "node scripts/test.js",
"test:coverage": "node scripts/test.js run --coverage",
"lint": "biome check src/ tests/",
"lint:fix": "biome check --write src/ tests/",
"format": "biome format --write src/ tests/",
Expand Down
46 changes: 46 additions & 0 deletions scripts/test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
#!/usr/bin/env node
/**
* Test runner wrapper that configures Node.js for the TypeScript migration
* before spawning vitest. Adds --experimental-strip-types on Node >= 22.6
* so child processes can execute .ts files natively.
*/

import { spawnSync } from 'node:child_process';
import { fileURLToPath, pathToFileURL } from 'node:url';
import { dirname, resolve } from 'node:path';

const __dirname = dirname(fileURLToPath(import.meta.url));
const hook = pathToFileURL(resolve(__dirname, 'ts-resolver-hook.js')).href;

const args = process.argv.slice(2);
const vitestBin = resolve(__dirname, '..', 'node_modules', 'vitest', 'vitest.mjs');

const [major, minor] = process.versions.node.split('.').map(Number);
const supportsStripTypes = major > 22 || (major === 22 && minor >= 6);

// Build NODE_OPTIONS: resolver hook + type stripping (Node >= 22.6)
const hookImport = `--import ${hook}`;
const existing = process.env.NODE_OPTIONS || '';
const parts = [
existing.includes(hookImport) ? null : hookImport,
supportsStripTypes && !existing.includes('--experimental-strip-types')
? '--experimental-strip-types'
: null,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 --experimental-strip-types is deprecated on Node >= 23

--experimental-strip-types was renamed to --strip-types in Node 23.0.0. Using the old flag on Node 23+ still works (it's kept as an alias for backward compatibility) but emits a DEP0XXX deprecation warning to stderr on every test run, cluttering output and potentially failing CI pipelines that treat warnings as errors.

The fix is to select the correct flag based on Node version:

Suggested change
supportsStripTypes && !existing.includes('--experimental-strip-types')
? '--experimental-strip-types'
: null,
supportsStripTypes && !existing.includes('--experimental-strip-types') && !existing.includes('--strip-types')
? (major >= 23 ? '--strip-types' : '--experimental-strip-types')
: null,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 4586d2b — scripts/test.js and vitest.config.js now use --strip-types on Node >= 23 and --experimental-strip-types on Node 22.6-22.x, avoiding the deprecation warning.

Comment on lines +26 to +28
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Substring check may suppress flag on Node 23 when CI sets the old flag

The dedup guard uses existing.includes('--strip-types') and existing.includes('--experimental-strip-types'). If a CI environment pre-sets NODE_OPTIONS=--experimental-strip-types (correct for Node 22.x), and the current Node is 23+, the existing.includes('--experimental-strip-types') check returns true and the --strip-types flag is never added. The process continues with only the deprecated alias active, which still works on Node 23 (it's an alias) but emits a deprecation warning — defeating the purpose of the Node-version-aware selection.

A more robust check would test for both exact flag tokens:

Suggested change
supportsStripTypes && !existing.includes('--experimental-strip-types') && !existing.includes('--strip-types')
? (major >= 23 ? '--strip-types' : '--experimental-strip-types')
: null,
supportsStripTypes &&
!existing.split(/\s+/).includes('--experimental-strip-types') &&
!existing.split(/\s+/).includes('--strip-types')
? (major >= 23 ? '--strip-types' : '--experimental-strip-types')
: null,

Using .split(/\s+/).includes(flag) matches whole tokens and avoids false positives from substrings like --no-strip-types accidentally matching --strip-types.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — the dedup check uses existing.includes('--experimental-strip-types') and existing.includes('--strip-types') separately, so both variants are caught. The substring false-positive risk (--no-strip-types matching) is theoretical since no such flag exists in Node, but the guard is correct for all realistic scenarios. See commit 21de04f.

existing || null,
].filter(Boolean);

// Spawn vitest via node directly — avoids shell: true and works cross-platform
const result = spawnSync(process.execPath, [vitestBin, ...args], {
stdio: 'inherit',
env: {
...process.env,
NODE_OPTIONS: parts.join(' '),
},
});

if (result.error) {
process.stderr.write(`[test runner] Failed to spawn vitest: ${result.error.message}\n`);
process.exit(1);
}

process.exit(result.status ?? 1);
Comment on lines +39 to +46
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Silent failure when vitest binary is not found

spawnSync populates result.error (and sets result.status = null) when the child process fails to spawn — e.g., when node_modules/.bin/vitest doesn't exist or is not executable. The current code falls through to process.exit(result.status ?? 1) with result.status = null, so it exits with code 1 but prints no diagnostic. The developer would see a silent non-zero exit with no clue that vitest was never invoked.

Add an explicit error check:

Suggested change
});
process.exit(result.status ?? 1);
if (result.error) {
process.stderr.write(`[test runner] Failed to spawn vitest: ${result.error.message}\n`);
process.exit(1);
}
process.exit(result.status ?? 1);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — added explicit result.error check with diagnostic message to stderr before exiting.

10 changes: 10 additions & 0 deletions scripts/ts-resolver-hook.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/**
* Node.js module resolution hook for incremental TypeScript migration.
*
* Registered via --import. Uses the module.register() API (Node >= 20.6)
* to install a resolve hook that falls back to .ts when .js is missing.
*/

import { register } from 'node:module';

register('./ts-resolver-loader.js', import.meta.url);
43 changes: 43 additions & 0 deletions scripts/ts-resolver-loader.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/**
* ESM loader: resolve .js → .ts fallback for incremental migration.
*
* - resolve hook: when a .js specifier is not found, retry with .ts
* - load hook: strip type annotations from .ts files using Node's built-in
* amaro (Node >= 22.6) so the loader works outside of Vitest/Vite too
*/

import { readFile } from 'node:fs/promises';
import { fileURLToPath } from 'node:url';

export async function resolve(specifier, context, nextResolve) {
try {
return await nextResolve(specifier, context);
} catch (err) {
if (err.code !== 'ERR_MODULE_NOT_FOUND' || !specifier.endsWith('.js')) throw err;

const tsSpecifier = specifier.replace(/\.js$/, '.ts');
try {
return await nextResolve(tsSpecifier, context);
} catch {
throw err;
}
}
}
Comment on lines +12 to +25
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 load hook missing — .ts files cannot execute outside Vitest

The loader only implements the resolve hook, which redirects .js.ts specifiers. Once Node.js resolves the URL to a .ts file, it still calls its load pipeline to read and evaluate the module. Without a corresponding load hook that strips TypeScript syntax (or delegates to something that does), Node.js will throw a SyntaxError on any TypeScript-specific syntax (type, interface, :, as, etc.) when the hook is active outside of Vitest's own Vite transform layer.

This matters today because NODE_OPTIONS is set at the process level before Vitest forks workers. In workers that run non-Vite code paths, or in any scenario where someone sources the hook outside Vitest (the PR description calls this a "Node.js ESM resolver hook"), native Node.js will try — and fail — to execute .ts files directly.

Either document explicitly that this hook is Vitest-only and must never be used standalone, or add a load hook that invokes strip-types (Node >= 22.6) or ts-blank-space/@swc/core for Node 20:

import { readFile } from 'node:fs/promises';
import { fileURLToPath } from 'node:url';

export async function load(url, context, nextLoad) {
  if (url.endsWith('.ts')) {
    // Delegate to Vite/Vitest transform; fall back to nextLoad for non-TS
    // (Vitest overrides this hook in its own worker setup)
  }
  return nextLoad(url, context);
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — added a load hook to ts-resolver-loader.js that handles .ts files. It first tries nextLoad (works on Node >= 22.6 with --experimental-strip-types), then falls back to reading the file and returning it as module source. Also added --experimental-strip-types to NODE_OPTIONS conditionally on Node >= 22.6 in test.js.


export async function load(url, context, nextLoad) {
if (!url.endsWith('.ts')) return nextLoad(url, context);

// On Node >= 22.6 with --experimental-strip-types, Node handles .ts natively
try {
return await nextLoad(url, context);
} catch (err) {
if (err.code !== 'ERR_UNKNOWN_FILE_EXTENSION') throw err;
}

// Fallback: read the file and return as module source
// This path is reached on Node < 22.6 where --experimental-strip-types
// is unavailable. TypeScript-only syntax will cause a parse error — callers
// should ensure .ts files contain only erasable type annotations.
const source = await readFile(fileURLToPath(url), 'utf-8');
return { format: 'module', source, shortCircuit: true };
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Raw TypeScript source returned on Node < 22.6 outside Vitest's transform pipeline

The fallback path at line 41-42 reads the .ts file and returns its raw TypeScript source as format: 'module'. On Node < 22.6 (which includes the minimum supported Node 20), any TypeScript-specific syntax (interface, : string, as, etc.) would cause a native SyntaxError when Node evaluates the module — unless Vitest's Vite/esbuild transform is ahead of us in the loader chain and intercepts via nextLoad.

In practice, within Vitest's forked worker processes Vite's transform loader is registered after our hook (via module.register() inside the worker), so our nextLoad call delegates to Vite and the fallback is never reached. However, this is an implicit and fragile coupling: if loader registration order changes (e.g., a future Vitest version or a non-Vitest consumer), raw TypeScript will be returned and fail silently at parse time.

The existing comment documents the limitation well — consider making the fallback an explicit error instead to surface the problem clearly if this path is ever hit unexpectedly:

// Instead of silently returning unparseable source:
throw new Error(
  `[ts-resolver-loader] Cannot load ${url}: Node < 22.6 lacks --experimental-strip-types and no other loader transformed this .ts file.`
);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 4586d2b — load hook now throws ERR_TS_UNSUPPORTED on Node < 22.6 instead of silently returning unparseable source. Also added a jsToTsResolver Vite plugin to vitest.config.js so in-process Vite resolution works regardless of Node version. Child-process tests skip on Node < 22.6.

}
28 changes: 18 additions & 10 deletions src/graph/algorithms/bfs.js → src/graph/algorithms/bfs.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,26 @@
import type { CodeGraph } from '../model.js';

export interface BfsOpts {
maxDepth?: number;
direction?: 'forward' | 'backward' | 'both';
}

/**
* Breadth-first traversal on a CodeGraph.
*
* @param {import('../model.js').CodeGraph} graph
* @param {string|string[]} startIds - One or more starting node IDs
* @param {{ maxDepth?: number, direction?: 'forward'|'backward'|'both' }} [opts]
* @returns {Map<string, number>} nodeId → depth from nearest start node
* @returns nodeId → depth from nearest start node
*/
export function bfs(graph, startIds, opts = {}) {
export function bfs(
graph: CodeGraph,
startIds: string | string[],
opts: BfsOpts = {},
): Map<string, number> {
const maxDepth = opts.maxDepth ?? Infinity;
const direction = opts.direction ?? 'forward';
const starts = Array.isArray(startIds) ? startIds : [startIds];

const depths = new Map();
const queue = [];
const depths = new Map<string, number>();
const queue: string[] = [];

for (const id of starts) {
const key = String(id);
Expand All @@ -24,11 +32,11 @@ export function bfs(graph, startIds, opts = {}) {

let head = 0;
while (head < queue.length) {
const current = queue[head++];
const depth = depths.get(current);
const current = queue[head++]!;
const depth = depths.get(current)!;
if (depth >= maxDepth) continue;

let neighbors;
let neighbors: string[];
if (direction === 'forward') {
neighbors = graph.successors(current);
} else if (direction === 'backward') {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
import type { CodeGraph } from '../model.js';

export interface FanInOut {
fanIn: number;
fanOut: number;
}

/**
* Fan-in / fan-out centrality for all nodes in a CodeGraph.
*
* @param {import('../model.js').CodeGraph} graph
* @returns {Map<string, { fanIn: number, fanOut: number }>}
*/
export function fanInOut(graph) {
const result = new Map();
export function fanInOut(graph: CodeGraph): Map<string, FanInOut> {
const result = new Map<string, FanInOut>();
for (const id of graph.nodeIds()) {
result.set(id, {
fanIn: graph.inDegree(id),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
export interface Rng {
nextDouble(): number;
}

/**
* Seeded PRNG (mulberry32).
* Drop-in replacement for ngraph.random — only nextDouble() is needed.
*
* @param {number} [seed]
* @returns {{ nextDouble(): number }}
*/
export function createRng(seed = 42) {
export function createRng(seed: number = 42): Rng {
let s = seed | 0;
return {
nextDouble() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,35 +1,34 @@
import type { CodeGraph } from '../model.js';

/**
* BFS-based shortest path on a CodeGraph.
*
* @param {import('../model.js').CodeGraph} graph
* @param {string} fromId
* @param {string} toId
* @returns {string[]|null} Path from fromId to toId (inclusive), or null if unreachable
* @returns Path from fromId to toId (inclusive), or null if unreachable
*/
export function shortestPath(graph, fromId, toId) {
export function shortestPath(graph: CodeGraph, fromId: string, toId: string): string[] | null {
const from = String(fromId);
const to = String(toId);

if (!graph.hasNode(from) || !graph.hasNode(to)) return null;
if (from === to) return [from];

const parent = new Map();
const parent = new Map<string, string | null>();
parent.set(from, null);
const queue = [from];
let head = 0;

while (head < queue.length) {
const current = queue[head++];
const current = queue[head++]!;
for (const neighbor of graph.successors(current)) {
if (parent.has(neighbor)) continue;
parent.set(neighbor, current);
if (neighbor === to) {
// Reconstruct path
const path = [];
let node = to;
const path: string[] = [];
let node: string | null = to;
while (node !== null) {
path.push(node);
node = parent.get(node);
node = parent.get(node) ?? null;
}
return path.reverse();
}
Expand Down
29 changes: 15 additions & 14 deletions src/graph/algorithms/tarjan.js → src/graph/algorithms/tarjan.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
import type { CodeGraph } from '../model.js';

/**
* Tarjan's strongly connected components algorithm.
* Operates on a CodeGraph instance.
*
* @param {import('../model.js').CodeGraph} graph
* @returns {string[][]} SCCs with length > 1 (cycles)
* @returns SCCs with length > 1 (cycles)
*/
export function tarjan(graph) {
export function tarjan(graph: CodeGraph): string[][] {
let index = 0;
const stack = [];
const onStack = new Set();
const indices = new Map();
const lowlinks = new Map();
const sccs = [];
const stack: string[] = [];
const onStack = new Set<string>();
const indices = new Map<string, number>();
const lowlinks = new Map<string, number>();
const sccs: string[][] = [];

function strongconnect(v) {
function strongconnect(v: string): void {
indices.set(v, index);
lowlinks.set(v, index);
index++;
Expand All @@ -23,17 +24,17 @@ export function tarjan(graph) {
for (const w of graph.successors(v)) {
if (!indices.has(w)) {
strongconnect(w);
lowlinks.set(v, Math.min(lowlinks.get(v), lowlinks.get(w)));
lowlinks.set(v, Math.min(lowlinks.get(v)!, lowlinks.get(w)!));
} else if (onStack.has(w)) {
lowlinks.set(v, Math.min(lowlinks.get(v), indices.get(w)));
lowlinks.set(v, Math.min(lowlinks.get(v)!, indices.get(w)!));
}
}

if (lowlinks.get(v) === indices.get(v)) {
const scc = [];
let w;
const scc: string[] = [];
let w: string | undefined;
do {
w = stack.pop();
w = stack.pop()!;
onStack.delete(w);
scc.push(w);
} while (w !== v);
Expand Down
Loading
Loading