Convert TypeScript to JavaScript with JSDoc type annotations#1360
Draft
dscho wants to merge 13 commits into
Draft
Convert TypeScript to JavaScript with JSDoc type annotations#1360dscho wants to merge 13 commits into
dscho wants to merge 13 commits into
Conversation
In preparation for incrementally converting the source from TypeScript to JavaScript (with JSDoc type annotations for type safety), adjust the build and test configuration to accept both file types. The TypeScript compiler gains `allowJs` and `checkJs` so that it will compile and type-check `.js` files placed next to the existing `.ts` ones. The `include` globs in both `tsconfig.json` and `tsconfig.eslint.json` are widened to cover `src/**/*.js` and `main.js` at the root. Vitest's include pattern is extended to match `.test.js` in addition to `.test.ts`. The `format`, `format-check` and `lint` scripts in `package.json` are updated to cover both `*.ts` and `*.js` (excluding generated directories). No source files are converted yet; this commit only opens the door.
Several `@typescript-eslint` rules do not apply to `.js` source files or would produce false positives there: `explicit-function-return-type` (JSDoc `@returns` serves this purpose instead), `no-require-imports` and `no-var-requires` (irrelevant in ESM but could flag JSDoc patterns). Add a file-type override block that disables them for `**/*.js` so that converted files lint cleanly from the start.
Replace `src/spawn.ts` with `src/spawn.js`. The `export type
SpawnReturnArgs` becomes a JSDoc `@typedef`, and the function
parameter types move into JSDoc `@param`/`@returns` tags. The
`SpawnOptions` type, previously available as a TypeScript import,
is now referenced via `@param {import('child_process').SpawnOptions}`.
Because the existing importers (`src/git.ts`, `src/ci_artifacts.ts`)
already use `from './spawn.js'` (the ESM convention introduced by
the Node 24 migration), they resolve to the new `.js` file without
any changes on their side.
Replace `src/downloader.ts` with `src/downloader.js`. The `as
{code: string}` cast becomes a JSDoc `@type` annotation on the
expression. The single function's parameter and return types move into
JSDoc tags.
Replace `src/git.ts` with `src/git.js`. The type-only import of `SpawnReturnArgs` from `./spawn.js` becomes a JSDoc `@typedef` with an `import()` type expression. All function signatures move their type annotations into `@param`/`@returns` tags. The `let head_sha: string` and `let child: SpawnReturnArgs` local type annotations become inline `@type` comments. The `download` closure's return-type annotation (`: Promise<void>`) is dropped because `tsc --checkJs` infers it from the async function.
Replace `src/ci_artifacts.ts` with `src/ci_artifacts.js`. The function signatures and the `let error: Error | undefined` local type annotation move into JSDoc tags.
Replace `main.ts` with `main.js`. The `ln` arrow function's parameter types become a JSDoc `@param` block. The `let useCache: boolean` annotation becomes an inline `@type`. The return-type annotations on `run()`, `cleanup()` and `getDriveLetterPrefix()` move into `@returns` tags.
The test file contained no TypeScript-specific syntax (the Node 24 migration already rewrote it to use Vitest's `vi.mock`/`vi.spyOn` without type annotations), so a plain rename suffices.
The `runAction` function's optional parameter type and `Promise` return type move into JSDoc tags. The `Promise<number>` generic argument is dropped since `tsc --checkJs` infers it from usage.
dscho
force-pushed
the
no-more-typescript
branch
from
980e0bf to
e5d87a6
Compare
Now that all source files are JavaScript with JSDoc annotations, `tsc` no longer needs to emit compiled output to `lib/`. The `noEmit` option is enabled and `outDir`/`rootDir` are dropped. The `ncc build` command now bundles `main.js` directly instead of going through the `lib/` intermediate directory. The `include` globs are narrowed to just the `.js` source files (no `.ts` patterns remain), and the `typecheck` script is removed since `build` (i.e. `tsc`) now serves the same purpose. The CI workflows are updated accordingly: the `npm run typecheck` step is dropped since `npm run build` already runs `tsc`.
Now that all source is JavaScript, the `@typescript-eslint` parser, the `typescript-eslint` config helper and every `@typescript-eslint/*` rule are no longer needed. Replace `tseslint.config(...)` with a plain flat config array, drop the TS parser in favor of the default espree parser, and remove the `import/resolver` typescript setting. The `import/extensions` and `import/no-unresolved` rules are disabled because ESM mandates `.js` extensions in import specifiers, which the import plugin's default resolver does not understand without the TypeScript resolver (which we are removing). Also fixes the format/lint script globs in `package.json` to drop the single-quote wrappers that broke on Windows, matching the style the repository used before our changes. The `.prettierignore` file already excludes `dist/`, `lib/` and `node_modules/`. Running `npm run format` after the eslint change reformats a few files (minor whitespace adjustments from prettier on the converted JavaScript).
With the ESLint config no longer using the TypeScript parser or rules, remove the now-unused packages: `@typescript-eslint/eslint-plugin`, `@typescript-eslint/parser`, `typescript-eslint`, and `eslint-import-resolver-typescript`. Also delete `tsconfig.eslint.json` which was only needed because the ESLint parser required a separate tsconfig with broader includes. The `typescript` package itself is kept because `tsc --noEmit --checkJs` provides type-checking for the JSDoc-annotated JavaScript source. The `@types/node` and `@types/unzipper` packages are kept because `tsc` needs them to type-check calls into those libraries. This reduces the top-level dependency count in the lockfile from 352 to 344 packages.
dscho
force-pushed
the
no-more-typescript
branch
from
e5d87a6 to
8fbbd0c
Compare
Member
Author
|
Thoughts, @rimrul? |
Member
|
I prefer typescript over javascript with jsdoc comments, but the idea of reducing our dependencies is enticing. |
Member
Author
Right, the reduction of dependencies was my main aim, in particular to reduce the Dependabot firehose effect. Dependency-wise, this is nothing to write home about: a mere 4 dependencies. The main benefit would be to step out of the ESLint cone, which seems to amount for a lot of the Dependabot stuff we have to deal with on a regular basis. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TypeScript was a great help getting this project off the ground, by
providing strong typing and thereby increasing confidence in the code.
In the meantime, JavaScript with JSDoc-style type annotations has
become essentially on par with that:
tsc --checkJsvalidates typesfrom JSDoc just as rigorously as it does from
.tssource. Thecommit series demonstrates this by converting files one at a time,
with type-checking, linting and tests passing at every step.
The practical motivation is reducing dependency churn. The TypeScript
ESLint toolchain (
@typescript-eslint/eslint-plugin,@typescript-eslint/parser,typescript-eslint, andeslint-import-resolver-typescript) sees frequent releases thatgenerate Dependabot PRs, each requiring a CI cycle and review.
Removing those four packages (and their transitive dependencies) should
noticeably quiet the Dependabot noise.
Type safety is not sacrificed:
tscstays as anoEmittypechecker, and
@types/nodeand@types/unzipperremain forlibrary type definitions.