Skip to content

Conversation

@carlansley
Copy link
Contributor

No description provided.

@carlansley carlansley self-assigned this Aug 14, 2025
Copilot AI review requested due to automatic review settings September 16, 2025 18:38
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR upgrades TypeScript configuration to support TypeScript 7, requires Node 24, and modernizes the development toolchain. The changes include updating package dependencies, migrating eslint configuration from MJS to TypeScript, and adding new build tools and compiler options.

  • Updates minimum Node.js requirement from 22.15 to 24
  • Adds TypeScript 7 support with native preview packages and new build tools
  • Migrates ESLint configuration from .mjs to .ts format with improved type safety

Reviewed Changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
package.json Version bump to 10.0.0, dependency updates, adds new build tools and TypeScript 7 support
eslint.config.ts New TypeScript-based ESLint configuration replacing the MJS version
eslint.config.mjs Removal of old MJS-based ESLint configuration
src/compile.ts Adds async keyword to map callback function
src/test/lib/es2026.spec.ts Adds TypeScript error suppression comment for Error.isError
SECURITY.md Updates supported version table to reflect 10.x series
README.md Updates Node.js requirement documentation
.github/workflows/ci.yml Removes Node 22.x from CI matrix, keeping only 24.x

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

const entries = await fs.readdir(directory, { withFileTypes: true });
const files = await Promise.all(
entries.map((entry) => {
entries.map(async (entry) => {
Copy link

Copilot AI Sep 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding async to the map callback creates an array of Promises instead of the expected values. The getFiles call should be awaited, but since this is inside Promise.all(), the current logic is broken. Either remove async or await the getFiles(result) call.

Suggested change
entries.map(async (entry) => {
entries.map((entry) => {

Copilot uses AI. Check for mistakes.
Copilot AI review requested due to automatic review settings September 20, 2025 00:11
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

src/compile.ts Outdated
entries.map((entry) => {
entries.map(async (entry) => {
const result = path.resolve(directory, entry.name);
return entry.isDirectory() ? getFiles(result) : result;
Copy link

Copilot AI Sep 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function is marked as async but doesn't await the recursive getFiles(result) call. This will return a Promise instead of the expected string array. Change to return entry.isDirectory() ? await getFiles(result) : result;

Suggested change
return entry.isDirectory() ? getFiles(result) : result;
return entry.isDirectory() ? await getFiles(result) : result;

Copilot uses AI. Check for mistakes.
Copilot AI review requested due to automatic review settings October 1, 2025 17:11
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

await (process.version < 'v24' ? describe.skip : describe)('es2026', () => {
// https://github.com/tc39/proposal-is-error
it('supports Error.isError', async () => {
// @ts-expect-error an error in TypeScript 5.8/7, but valid in 5.9
Copy link

Copilot AI Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment mentions 'TypeScript 5.8/7' which is confusing. It should either be '5.8' or clarify what is meant by '5.8/7'. If referring to TypeScript 7, it should be written as 'TypeScript 7.0' for clarity.

Suggested change
// @ts-expect-error an error in TypeScript 5.8/7, but valid in 5.9
// @ts-expect-error an error in TypeScript 5.8, but valid in 5.9

Copilot uses AI. Check for mistakes.
Copilot AI review requested due to automatic review settings October 15, 2025 18:48
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@github-actions
Copy link

Beta Published - Install Command: npm install @checkdigit/[email protected]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants