Update Node.js, TypeScript, drop ts-node dependency#82
Conversation
NickGerleman
left a comment
There was a problem hiding this comment.
Looks like this is breaking the lint job.
44d1ad9 to
47cef72
Compare
There was a problem hiding this comment.
Pull request overview
Updates the toolchain to newer Node.js/TypeScript versions and removes the ts-node runtime dependency for scripts.
Changes:
- Bump GitHub Actions Node.js version from 18 to 22.
- Upgrade TypeScript to
^5.8.3and removets-node. - Adjust scripts and TS/ESLint configs to support the new setup.
Reviewed changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Removes ts-node and updates the TypeScript lock entries. |
| package.json | Drops ts-node, bumps TypeScript, and changes script invocations. |
| tsconfig.json | Excludes scripts from the root TypeScript project. |
| scripts/tsconfig.json | Adds TS options for extensionful imports and verbatim module syntax. |
| scripts/updateHistory.ts | Switches ESM import specifiers to .ts for scripts. |
| scripts/generateAssets.ts | Switches ESM import specifiers to .ts for scripts. |
| scripts/fetchWaybackMachineHistory.ts | Switches ESM import specifiers to .ts for scripts. |
| scripts/helper/createAxiosInstance.ts | Adjusts imports for verbatimModuleSyntax and adds type-cast workarounds. |
| .eslintrc.cjs | Points ESLint to both TS projects (root + scripts). |
| .github/workflows/ci.yml | Updates workflow Node.js version to 22. |
| .github/workflows/deploy.yml | Updates workflow Node.js version to 22. |
| .github/workflows/update-history.yml | Updates workflow Node.js version to 22. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "verbatimModuleSyntax": true | ||
| }, | ||
| "exclude": [] |
There was a problem hiding this comment.
allowImportingTsExtensions commonly requires noEmit to be set (otherwise the config can error in editors/tsserver even if CI uses tsc --noEmit). Instead of relying on the CLI flag, set compilerOptions.noEmit: true in this config; also consider avoiding exclude: [] by explicitly excluding generated folders (if any) to prevent accidental project-wide typechecking creep.
| "verbatimModuleSyntax": true | |
| }, | |
| "exclude": [] | |
| "verbatimModuleSyntax": true, | |
| "noEmit": true | |
| }, | |
| "exclude": ["dist", "build"] |
| uses: actions/setup-node@v3 | ||
| with: | ||
| node-version: "18" | ||
| node-version: "22" |
There was a problem hiding this comment.
actions/setup-node@v3 is an older major; for newer Node versions (and to stay aligned with current GitHub Actions runtime expectations), it’s safer to upgrade to actions/setup-node@v4. This change appears in multiple workflows (ci.yml, deploy.yml, update-history.yml) and should be updated consistently.
| // @FIXME: Legacy version of axios does not play well with verbatim types | ||
| const axiosClient = (axios as unknown as AxiosStatic).create(); |
There was a problem hiding this comment.
The unknown as ... casts remove type safety and can hide real integration issues. Prefer addressing this by updating the dependency typings (or the dependencies themselves) and/or applying narrower typing locally (e.g., type the returned instance/function signature) rather than casting the imported module values to unknown.
| // @FIXME: Legacy version of axios-rate-limit does not play well with verbatim types | ||
| return (rateLimit as unknown as typeof import("axios-rate-limit").default)( | ||
| axiosClient, | ||
| axiosRateLimitOptions | ||
| ); |
There was a problem hiding this comment.
The unknown as ... casts remove type safety and can hide real integration issues. Prefer addressing this by updating the dependency typings (or the dependencies themselves) and/or applying narrower typing locally (e.g., type the returned instance/function signature) rather than casting the imported module values to unknown.
No description provided.