Skip to content

Commit e8cb8ff

Browse files
authored
Merge pull request #1550 from pmcelhaney/copilot/add-prune-option-for-clearing-paths
Add --prune option to remove defunct route files when OpenAPI spec changes
2 parents bf994d4 + 2fc3033 commit e8cb8ff

File tree

7 files changed

+285
-2
lines changed

7 files changed

+285
-2
lines changed

.changeset/add-prune-option.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"counterfact": minor
3+
---
4+
5+
Added `--prune` option to remove route files that no longer exist in the OpenAPI spec.
6+
7+
When an OpenAPI spec renames a path parameter (e.g. `/pet/{id}/update/{Name}``/pet/{id}/update/{nickname}`), running without `--prune` leaves the old file in place alongside the newly generated one, causing wildcard ambiguity in route matching. The new flag cleans up defunct route files before generation runs.
8+
9+
```sh
10+
npx counterfact openapi.yaml ./out --generate --prune
11+
```
12+
13+
Context files (`_.context.ts`) and empty directories are handled correctly — context files are never pruned, and any directories left empty after pruning are removed automatically.

bin/counterfact.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,8 @@ async function main(source, destination) {
155155
options.watch ||
156156
options.watchTypes ||
157157
options.buildCache,
158+
159+
prune: Boolean(options.prune),
158160
},
159161

160162
openApiPath: source,
@@ -326,5 +328,9 @@ program
326328
"--always-fake-optionals",
327329
"random responses will include optional fields",
328330
)
331+
.option(
332+
"--prune",
333+
"remove route files that no longer exist in the OpenAPI spec",
334+
)
329335
.action(main)
330336
.parse(process.argv);

src/server/config.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ export interface Config {
44
basePath: string;
55
buildCache: boolean;
66
generate: {
7+
prune?: boolean;
78
routes: boolean;
89
types: boolean;
910
};

src/typescript-generator/code-generator.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,18 @@ export class CodeGenerator extends EventTarget {
99

1010
private readonly destination: string;
1111

12-
private readonly generateOptions: { routes: boolean; types: boolean };
12+
private readonly generateOptions: {
13+
prune?: boolean;
14+
routes: boolean;
15+
types: boolean;
16+
};
1317

1418
private watcher: FSWatcher | undefined;
1519

1620
public constructor(
1721
openApiPath: string,
1822
destination: string,
19-
generateOptions: { routes: boolean; types: boolean },
23+
generateOptions: { prune?: boolean; routes: boolean; types: boolean },
2024
) {
2125
super();
2226
this.openapiPath = openApiPath;

src/typescript-generator/generate.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import createDebug from "debug";
77

88
import { ensureDirectoryExists } from "../util/ensure-directory-exists.js";
99
import { OperationCoder } from "./operation-coder.js";
10+
import { pruneRoutes } from "./prune.js";
1011
import { Repository } from "./repository.js";
1112
import { Specification } from "./specification.js";
1213

@@ -72,6 +73,12 @@ export async function generate(
7273

7374
debug("got %i paths", paths.size);
7475

76+
if (generateOptions.prune && generateOptions.routes) {
77+
debug("pruning defunct route files");
78+
await pruneRoutes(destination, paths.keys());
79+
debug("done pruning");
80+
}
81+
7582
const securityRequirement = specification.getRequirement(
7683
"#/components/securitySchemes",
7784
);

src/typescript-generator/prune.js

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
import fs from "node:fs/promises";
2+
import nodePath from "node:path";
3+
4+
import createDebug from "debug";
5+
6+
const debug = createDebug("counterfact:typescript-generator:prune");
7+
8+
/**
9+
* Collects all .ts route files in a directory recursively.
10+
* Context files (_.context.ts) are excluded.
11+
* @param {string} routesDir - Path to routes directory
12+
* @param {string} currentPath - Current subdirectory being processed (relative to routesDir)
13+
* @returns {Promise<string[]>} - Array of relative paths (using forward slashes)
14+
*/
15+
async function collectRouteFiles(routesDir, currentPath = "") {
16+
const files = [];
17+
18+
try {
19+
const fullDir = currentPath
20+
? nodePath.join(routesDir, currentPath)
21+
: routesDir;
22+
const entries = await fs.readdir(fullDir, { withFileTypes: true });
23+
24+
for (const entry of entries) {
25+
const relativePath = currentPath
26+
? `${currentPath}/${entry.name}`
27+
: entry.name;
28+
29+
if (entry.isDirectory()) {
30+
files.push(...(await collectRouteFiles(routesDir, relativePath)));
31+
} else if (entry.name.endsWith(".ts") && entry.name !== "_.context.ts") {
32+
files.push(relativePath);
33+
}
34+
}
35+
} catch (error) {
36+
if (error.code !== "ENOENT") {
37+
throw error;
38+
}
39+
}
40+
41+
return files;
42+
}
43+
44+
/**
45+
* Recursively removes empty directories under rootDir, but not rootDir itself.
46+
* @param {string} dir - Directory to check
47+
* @param {string} rootDir - Root directory that should never be removed
48+
*/
49+
async function removeEmptyDirectories(dir, rootDir) {
50+
let entries;
51+
52+
try {
53+
entries = await fs.readdir(dir, { withFileTypes: true });
54+
} catch {
55+
return;
56+
}
57+
58+
for (const entry of entries) {
59+
if (entry.isDirectory()) {
60+
await removeEmptyDirectories(nodePath.join(dir, entry.name), rootDir);
61+
}
62+
}
63+
64+
if (nodePath.resolve(dir) === nodePath.resolve(rootDir)) {
65+
return;
66+
}
67+
68+
const remaining = await fs.readdir(dir);
69+
70+
if (remaining.length === 0) {
71+
await fs.rmdir(dir);
72+
debug("removed empty directory: %s", dir);
73+
}
74+
}
75+
76+
/**
77+
* Converts an OpenAPI path to the expected route file path (relative to routesDir).
78+
* e.g. "/pet/{id}" -> "pet/{id}.ts", "/" -> "index.ts"
79+
* @param {string} openApiPath
80+
* @returns {string}
81+
*/
82+
function openApiPathToRouteFile(openApiPath) {
83+
const filePath = openApiPath === "/" ? "index" : openApiPath.slice(1);
84+
85+
return `${filePath}.ts`;
86+
}
87+
88+
/**
89+
* Prunes route files that no longer correspond to any path in the OpenAPI spec.
90+
* Context files (_.context.ts) are never pruned.
91+
* @param {string} destination - Base destination directory (contains the routes/ sub-directory)
92+
* @param {Iterable<string>} openApiPaths - Iterable of OpenAPI path strings (e.g. "/pet/{id}")
93+
* @returns {Promise<number>} - Number of files removed
94+
*/
95+
export async function pruneRoutes(destination, openApiPaths) {
96+
const routesDir = nodePath.join(destination, "routes");
97+
98+
const expectedFiles = new Set(
99+
Array.from(openApiPaths).map(openApiPathToRouteFile),
100+
);
101+
102+
debug("expected route files: %o", Array.from(expectedFiles));
103+
104+
const actualFiles = await collectRouteFiles(routesDir);
105+
106+
debug("actual route files: %o", actualFiles);
107+
108+
let prunedCount = 0;
109+
110+
for (const file of actualFiles) {
111+
const normalizedFile = file.replaceAll("\\", "/");
112+
113+
if (!expectedFiles.has(normalizedFile)) {
114+
const fullPath = nodePath.join(routesDir, file);
115+
116+
debug("pruning %s", fullPath);
117+
await fs.rm(fullPath);
118+
prunedCount++;
119+
}
120+
}
121+
122+
await removeEmptyDirectories(routesDir, routesDir);
123+
124+
debug("pruned %d files", prunedCount);
125+
126+
return prunedCount;
127+
}
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
import { describe, expect, it } from "@jest/globals";
2+
import { promises as fs } from "node:fs";
3+
import path from "node:path";
4+
5+
import { pruneRoutes } from "../../src/typescript-generator/prune.js";
6+
import { withTemporaryFiles } from "../lib/with-temporary-files.ts";
7+
8+
describe("pruneRoutes", () => {
9+
it("removes a route file that is not in the OpenAPI spec", async () => {
10+
await withTemporaryFiles(
11+
{
12+
"routes/pet/{id}.ts": "export const GET = () => ({ status: 200 });",
13+
"routes/pet/{name}.ts": "export const GET = () => ({ status: 200 });",
14+
},
15+
async (basePath, { path: getPath }) => {
16+
const expectedPaths = ["/pet/{id}"];
17+
const count = await pruneRoutes(basePath, expectedPaths);
18+
19+
expect(count).toBe(1);
20+
await expect(
21+
fs.access(getPath("routes/pet/{id}.ts")),
22+
).resolves.toBeUndefined();
23+
await expect(
24+
fs.access(getPath("routes/pet/{name}.ts")),
25+
).rejects.toThrow();
26+
},
27+
);
28+
});
29+
30+
it("keeps context files even when not in the spec", async () => {
31+
await withTemporaryFiles(
32+
{
33+
"routes/_.context.ts": "export class Context {}",
34+
"routes/pet/_.context.ts": "export class Context {}",
35+
"routes/pet/{id}.ts": "export const GET = () => ({ status: 200 });",
36+
},
37+
async (basePath, { path: getPath }) => {
38+
const expectedPaths = ["/pet/{id}"];
39+
const count = await pruneRoutes(basePath, expectedPaths);
40+
41+
expect(count).toBe(0);
42+
await expect(
43+
fs.access(getPath("routes/_.context.ts")),
44+
).resolves.toBeUndefined();
45+
await expect(
46+
fs.access(getPath("routes/pet/_.context.ts")),
47+
).resolves.toBeUndefined();
48+
},
49+
);
50+
});
51+
52+
it("removes empty directories after pruning", async () => {
53+
await withTemporaryFiles(
54+
{
55+
"routes/old/{id}.ts": "export const GET = () => ({ status: 200 });",
56+
},
57+
async (basePath, { path: getPath }) => {
58+
const expectedPaths = [];
59+
await pruneRoutes(basePath, expectedPaths);
60+
61+
await expect(fs.access(getPath("routes/old"))).rejects.toThrow();
62+
},
63+
);
64+
});
65+
66+
it("does not remove directories that still contain context files", async () => {
67+
await withTemporaryFiles(
68+
{
69+
"routes/old/{id}.ts": "export const GET = () => ({ status: 200 });",
70+
"routes/old/_.context.ts": "export class Context {}",
71+
},
72+
async (basePath, { path: getPath }) => {
73+
const expectedPaths = [];
74+
await pruneRoutes(basePath, expectedPaths);
75+
76+
await expect(fs.access(getPath("routes/old"))).resolves.toBeUndefined();
77+
await expect(
78+
fs.access(getPath("routes/old/_.context.ts")),
79+
).resolves.toBeUndefined();
80+
},
81+
);
82+
});
83+
84+
it("handles the root path '/'", async () => {
85+
await withTemporaryFiles(
86+
{
87+
"routes/index.ts": "export const GET = () => ({ status: 200 });",
88+
"routes/old.ts": "export const GET = () => ({ status: 200 });",
89+
},
90+
async (basePath, { path: getPath }) => {
91+
const expectedPaths = ["/"];
92+
const count = await pruneRoutes(basePath, expectedPaths);
93+
94+
expect(count).toBe(1);
95+
await expect(
96+
fs.access(getPath("routes/index.ts")),
97+
).resolves.toBeUndefined();
98+
await expect(fs.access(getPath("routes/old.ts"))).rejects.toThrow();
99+
},
100+
);
101+
});
102+
103+
it("returns 0 when all files match the spec", async () => {
104+
await withTemporaryFiles(
105+
{
106+
"routes/pet/{id}.ts": "export const GET = () => ({ status: 200 });",
107+
"routes/pet.ts": "export const GET = () => ({ status: 200 });",
108+
},
109+
async (basePath) => {
110+
const expectedPaths = ["/pet/{id}", "/pet"];
111+
const count = await pruneRoutes(basePath, expectedPaths);
112+
113+
expect(count).toBe(0);
114+
},
115+
);
116+
});
117+
118+
it("returns 0 when the routes directory does not exist", async () => {
119+
await withTemporaryFiles({}, async (basePath) => {
120+
const count = await pruneRoutes(basePath, ["/pet/{id}"]);
121+
122+
expect(count).toBe(0);
123+
});
124+
});
125+
});

0 commit comments

Comments
 (0)