feat(#1): scaffold httptape-js monorepo + vite-plugin-httptape v0.0.1#2
Conversation
Root package.json with pnpm workspaces, tsconfig.base.json, .gitignore, .editorconfig, and root README. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Package.json with peerDependencies on vite >=6, optionalDependencies on platform binary packages, tsconfig, eslint flat config, and vitest config. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- options.ts: HttptapeOptions type + resolveOptions() with validation - binary.ts: resolveBinary() with require.resolve fallback and friendly error - port.ts: pickFreePort() via net.createServer on port 0 - process.ts: spawn wrapper with SIGTERM->3s->SIGKILL lifecycle - log.ts: line-buffered stdout/stderr piping to Vite logger - index.ts: default export Plugin factory with config() and configureServer() Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- options.test.ts: defaults, validation, mode/upstream constraints, port bounds - port.test.ts: returns valid port, distinct ports on consecutive calls - integration.test.ts: binary-on-PATH gate with test.skipIf 15 tests pass, 1 skipped (integration requires httptape binary). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@httptape/binary-{darwin-arm64,darwin-x64,linux-x64,linux-arm64,win32-x64}
each with os/cpu fields, bin/.gitkeep placeholder, and one-line README.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
GitHub Actions CI: typecheck, lint, test on Node 20+22 x ubuntu/macos/windows matrix. Uses pnpm/action-setup and actions/setup-node v4 with pnpm cache. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
pnpm 11 requires Node >= 22.13, breaking all Node 20 CI jobs. Downgrade to pnpm 10.33.4 which supports Node >= 18. Also switch binary optionalDependencies from fixed "0.0.1" to "workspace:*" so pnpm properly records them in pnpm-lock.yaml. Without this, --frozen-lockfile fails because the lockfile is missing the optionalDependencies specifiers. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
| child.kill('SIGKILL'); | ||
| } catch { | ||
| // Already dead. | ||
| } |
There was a problem hiding this comment.
Bug -- timer keeps Node alive on shutdown: The setTimeout returned here is never .unref()'d. If the Vite server closes and this timeout is the last handle on the event loop, Node will hang for up to 3 seconds waiting for it to fire -- even if the child process already exited and cleared the timeout via clearTimeout.
This is the same pattern you correctly applied in port.ts (where the server gets .unref()). Apply it here:
const timeout = setTimeout(() => {
try {
child.kill('SIGKILL');
} catch {
// Already dead.
}
}, GRACEFUL_SHUTDOWN_MS);
timeout.unref();Without this, Ctrl-C on vite dev will visibly stall for up to 3 seconds before the terminal returns, which directly violates acceptance criterion #4 ('cleanly stops both Vite and the httptape child').
|
|
||
| return { | ||
| server: { | ||
| proxy: { |
There was a problem hiding this comment.
Bug -- route prefix not regex-escaped: routePrefix is user-supplied and may contain regex-special characters (e.g., /api/v1.0 where . matches any character, or /api/[internal] where brackets are character classes). This silently rewrites the wrong paths.
Escape the prefix before constructing the regex:
function escapeRegExp(s: string): string {
return s.replace(/[.*+?^${}()|[\]\\]/g, '\\\$&');
}
// in config():
rewrite: (path: string) =>
path.replace(new RegExp(`^${escapeRegExp(routePrefix)}`), ''),Alternatively, since this is always a prefix match on a fixed string, you could use path.slice(routePrefix.length) when path.startsWith(routePrefix), which avoids the regex entirely and is cleaner:
rewrite: (path: string) =>
path.startsWith(routePrefix) ? path.slice(routePrefix.length) : path,The startsWith approach is simpler and sidesteps the escaping problem entirely. Either fix works.
tibtof
left a comment
There was a problem hiding this comment.
Review: CHANGES REQUESTED (2 bugs, 0 architecture issues)
Two bugs found, both straightforward fixes:
-
process.ts:97--setTimeoutnot.unref()'d inkillGracefully: The 3-second SIGKILL timeout keeps the Node event loop alive after Vite shuts down, causing a visible 3-second hang onCtrl-C. Addtimeout.unref()after thesetTimeoutcall (same pattern already used correctly inport.ts). -
index.ts:48-- route prefix not regex-escaped in proxy rewrite:new RegExp('^${routePrefix}')treats user-supplied route strings as regex patterns. A route like/api/v1.0would match/api/v1X0because.is unescaped. Simplest fix: replace the regex withpath.startsWith(routePrefix) ? path.slice(routePrefix.length) : path.
Everything else is solid:
- File layout matches ADR-1 exactly (6 source modules, 5 binary stubs, CI workflow, READMEs)
- Public API surface matches the ADR's locked design
- TypeScript is strict, ESM-only,
constthroughout, noany, proper type imports - Lifecycle is correct:
spawn(notexec),detached: false, SIGTERM->3s->SIGKILL, top-levelprocess.on('exit')belt-and-suspenders pickFreePort()matches ADR-1 snippet with.unref()- Logging uses
readlinefor line-buffering with[httptape]prefix - Binary resolution uses
createRequire+require.resolvewith a friendly error - Tests cover options validation (defaults, mode/upstream constraints, port bounds) and port helper (valid port, distinct ports); integration test skips cleanly with
test.skipIf - Binary stub packages have correct
os/cpufields and Apache-2.0 license - CI runs typecheck + lint + test on Node 20+22 x ubuntu/macos/windows (all green)
- No out-of-scope drift (no release workflow, no npm publish, no examples migration)
viteaspeerDependencies: ">=6", binary packages asoptionalDependencies: "workspace:*","type": "module"set
Fix the two inline comments and this is ready to approve.
- process.ts: call timeout.unref() so the Node event loop doesn't hang for up to 3s after Vite shuts down (matches pattern already used in port.ts). - index.ts: replace RegExp-based rewrite with startsWith/slice so that regex metacharacters in route prefixes (e.g. "/api.v1") are handled literally instead of being interpreted as regex patterns. - Add test/index.test.ts covering the rewrite function, including paths with regex metacharacters. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Both reviewer-requested fixes are pushed:
All checks pass: |
tibtof
left a comment
There was a problem hiding this comment.
Review: APPROVED (would approve if not self-owned PR)
Verified the two requested fixes in commit 4a6d86f:
-
process.ts--timeout.unref(): Added on line 99, immediately after thesetTimeoutblock closes. Correct placement -- the Node event loop will no longer hang for 3 seconds on Ctrl-C. Matches the existing.unref()pattern inport.ts. -
index.ts-- regex replaced withstartsWith/slice: Thepath.replace(new RegExp(...))is gone, replaced withpath.startsWith(routePrefix) ? path.slice(routePrefix.length) : path. No regex metacharacter issues possible. -
New
test/index.test.ts: 4 tests covering simple prefix stripping, non-matching passthrough, and two regex-metacharacter edge cases (.and+in route prefixes). Good coverage of the regression scenario.
No unrelated changes in the commit -- only the three files directly related to the two fixes.
Closes #1
Summary
Scaffolds the
httptape-jsmonorepo per ADR-1:package.jsonwith pnpm workspaces,tsconfig.base.json(strict ESM, bundler moduleResolution),.gitignore,.editorconfig, rootREADME.mdpackages/vite-plugin-httptape/: Vite plugin that manages an httptape child process duringvite devsrc/options.ts--HttptapeOptionstype +resolveOptions()with validationsrc/binary.ts--resolveBinary()viarequire.resolve('@httptape/binary-<platform>-<arch>/bin/httptape')src/port.ts--pickFreePort()usingnet.createServeron port 0src/process.ts-- spawn wrapper with SIGTERM -> 3s -> SIGKILL lifecycle,process.on('exit')belt-and-suspenderssrc/log.ts-- line-buffered stdout/stderr -> Vite logger with[httptape]prefixsrc/index.ts-- default export Plugin factory (apply: 'serve',config(),configureServer())test/options.test.ts-- 12 tests: defaults, validation, mode/upstream constraints, port boundstest/port.test.ts-- 2 tests: returns valid port, distinct ports on consecutive callstest/integration.test.ts--test.skipIf(!httptapeOnPath)so CI passes before binaries existREADME.mdwith copy-pasteable quickstart@httptape/binary-{darwin-arm64,darwin-x64,linux-x64,linux-arm64,win32-x64}withos/cpufields andbin/.gitkeep.github/workflows/ci.yml: typecheck + lint + test on Node 20+22 x ubuntu/macos/windows matrixOut of scope (separate follow-ups)
release.ymlcross-repo dispatch workflowhttptape/httptape'sexamples/ts-frontend-firstTest plan
pnpm -r typecheckpassespnpm -r lintpassespnpm -r testpasses (15 passed, 1 skipped -- integration test skips when httptape binary not on PATH)Co-Authored-By: Claude Opus 4.7 noreply@anthropic.com