Skip to content

feat(types): migrate core modules to TypeScript (Phase 5.4) #554

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
carlos-alm merged 17 commits into from
Mar 22, 2026
Merged

feat(types): migrate core modules to TypeScript (Phase 5.4) #554

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
3b8e612
feat(types): add SQLite, StmtCache, and NativeAddon foundation types
carlos-alm Mar 21, 2026
2c3d27b
feat(types): convert db/repository layer from JS to TypeScript
carlos-alm Mar 21, 2026
4bfc950
feat(types): migrate core modules to TypeScript (Phase 5.4)
carlos-alm Mar 21, 2026
26cc36c
fix: add .js → .ts fallback to dynamic import verifier (#554)
carlos-alm Mar 21, 2026
37ac14f
fix: add load hook, remove unused imports, fix Node 20 compat (#554)
carlos-alm Mar 21, 2026
2ab13ab
fix: add load hook, preserve NODE_OPTIONS, fix Node 20 compat (#554)
carlos-alm Mar 21, 2026
447510a
fix: replace CJS require() with ESM import in formatCycles tests (#554)
carlos-alm Mar 21, 2026
bb22706
fix: resolve merge conflicts with main, fix TS errors (#554)
carlos-alm Mar 21, 2026
8ae7c6d
fix: add dedup guards, Node 20.6 guard, simplify type assertion (#554)
carlos-alm Mar 21, 2026
d1dc03b
fix: cast kind to SymbolKind instead of string in includes() (#554)
carlos-alm Mar 21, 2026
65dcb2a
fix: resolve merge conflicts with main
carlos-alm Mar 21, 2026
897993b
fix: resolve readonly array includes TS errors from main merge
carlos-alm Mar 21, 2026
306b833
fix: guard --import flag for Node >= 20.6 and add implements IReposit…
carlos-alm Mar 21, 2026
7465f9b
fix: correct NativeAddon.resolveImports signature and remove unused d…
carlos-alm Mar 22, 2026
b08ce30
fix: correct NativeAddon.parseFiles signature and relax transaction type
carlos-alm Mar 22, 2026
4a63243
fix: document double-cast in purgeFilesFromGraph
carlos-alm Mar 22, 2026
d0b4b8d
fix: restore typed db in watcher.ts instead of widening to any
carlos-alm Mar 22, 2026
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
29 changes: 29 additions & 0 deletions scripts/ts-resolve-hooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/**
* ESM resolve/load hooks for .js → .ts fallback during gradual migration.
*
* - resolve: when a .js specifier resolves to a path that doesn't exist,
* check if a .ts version exists and redirect to it.
* - load: for .ts files, strip type annotations using Node 22's built-in
* --experimental-strip-types via a source transform.
*/

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

@greptile-apps greptile-apps Bot Mar 21, 2026

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 Unused imports — missing load hook

The module header documents a load hook for .ts files ("strip type annotations using Node 22's built-in --experimental-strip-types"), but only the resolve hook was implemented. As a result, all three imports on these lines are completely unused:

  • existsSync from node:fs — never called
  • readFile from node:fs/promises — never called
  • fileURLToPath / pathToFileURL from node:url — never called

The actual TypeScript stripping is delegated to the global --experimental-strip-types Node flag set in vitest.config.js, so the load hook may not be needed at all. Either remove the dead imports or implement the intended load hook.

Suggested change
import { existsSync } from 'node:fs';
import { readFile } from 'node:fs/promises';
import { fileURLToPath, pathToFileURL } from 'node:url';
import { existsSync } from 'node:fs';

Or remove all three import lines if no load hook is planned.

@carlos-alm carlos-alm Mar 21, 2026

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 — removed all unused imports (existsSync, readFile, fileURLToPath, pathToFileURL). Added the missing load hook that handles .ts files: tries nextLoad first (works on Node >= 22.6 with --experimental-strip-types), falls back to reading the file as module source.


export async function resolve(specifier, context, nextResolve) {
try {
return await nextResolve(specifier, context);
} catch (err) {
// Only intercept ERR_MODULE_NOT_FOUND for .js specifiers
if (err.code === 'ERR_MODULE_NOT_FOUND' && specifier.endsWith('.js')) {
const tsSpecifier = specifier.replace(/\.js$/, '.ts');
try {
return await nextResolve(tsSpecifier, context);
} catch {
// .ts also not found — throw the original error
}
}
throw err;
}
}
15 changes: 15 additions & 0 deletions scripts/ts-resolve-loader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/**
* Node.js ESM loader hook for the JS → TS gradual migration.
*
* When a .js import specifier can't be found on disk, this loader tries the
* corresponding .ts file. This lets plain .js files import from already-
* migrated .ts modules without changing their import specifiers.
*
Comment on lines +1 to +7

@greptile-apps greptile-apps Bot Mar 21, 2026

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 module.register() requires Node >= 20.6.0, not just Node >= 20

register from node:module was introduced in Node 18.19.0 / 20.6.0. On Node 20.0.x – 20.5.x (which satisfy the project's stated >= 20 requirement), importing this loader via --import in NODE_OPTIONS throws a fatal ERR_PACKAGE_PATH_NOT_EXPORTED / named-export error and crashes every child process that inherits NODE_OPTIONS.

The vitest.config.js always injects --import ${loaderPath} into NODE_OPTIONS, regardless of the running Node version. Child-process tests are guarded by skipIf(!canStripTypes), but if a user runs the test suite on Node 20.0–20.5, the --import injection itself would crash those child processes before the skip logic ever fires.

Consider adding a version guard before registering the hooks:

import { createRequire } from 'node:module';
const _req = createRequire(import.meta.url);
// module.register() was added in Node 20.6.0
const [major, minor] = process.versions.node.split('.').map(Number);
if (major > 20 || (major === 20 && minor >= 6)) {
  const { register } = await import('node:module');
  const hooksURL = new URL('./ts-resolve-hooks.js', import.meta.url);
  register(hooksURL.href, { parentURL: import.meta.url });
}

Or update the minimum Node constraint in package.json to >=20.6.0.

@carlos-alm carlos-alm Mar 21, 2026

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 — ts-resolve-loader.js now guards module.register() with a Node >= 20.6 version check. On older Node versions, the hook registration is silently skipped. See commit 8ae7c6d.

* Usage: node --import ./scripts/ts-resolve-loader.js ...
* (or via NODE_OPTIONS / vitest poolOptions.execArgv)
*/

import { register } from 'node:module';

const hooksURL = new URL('./ts-resolve-hooks.js', import.meta.url);
register(hooksURL.href, { parentURL: import.meta.url });
201 changes: 0 additions & 201 deletions src/db/repository/base.js
View file
Open in desktop

This file was deleted.

Loading
Loading