Conversation
BREAKING CHANGE: drop CommonJS support, only ESM module is released
There was a problem hiding this comment.
Pull Request Overview
This pull request represents a major migration from V1 to V2 of the library, introducing comprehensive changes to improve the development experience, modernize tooling, and add new functionality.
- Migration from Jest to Node.js native test runner with built-in coverage support
- Complete replacement of build tooling from Microbundle to TypeScript compiler with explicit file extensions
- Addition of new
iterablemodule with universalreducefunction and newoutdenttemplate literal function
Reviewed Changes
Copilot reviewed 36 out of 39 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Updated build tooling, test setup, and dependencies for V2 migration |
| biome.json | Added Biome for code formatting and linting |
| src/iterable.ts | New module with universal reduce function for sync/async iterables |
| src/string.ts | Added outdent function for removing leading indentation from template literals |
| src/guards.ts | Enhanced with new type guards and updated to use modern JavaScript methods |
| spec/*.spec.ts | Migrated all tests from Jest to Node.js native test runner |
| src/*.ts | Updated import statements to use explicit .ts file extensions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| * @param strings | ||
| * @param values | ||
| * @returns |
There was a problem hiding this comment.
The JSDoc parameters lack descriptions. Add proper descriptions for strings (TemplateStringsArray), values (interpolated values), and @returns (processed string with indentation removed).
| * @param strings | |
| * @param values | |
| * @returns | |
| * @param strings The array of string literals from the template (TemplateStringsArray). | |
| * @param values The interpolated values within the template. | |
| * @returns The processed string with indentation removed. |
| const numKey = parseInt(key.toString(), 10) | ||
|
|
||
| return isNaN(numKey) | ||
| return Number.isNaN(numKey) |
There was a problem hiding this comment.
[nitpick] Good improvement replacing the global isNaN with Number.isNaN which provides more precise NaN checking without type coercion.
| export function ensureArray<T>(value: T | T[]): T[] | ||
| export function ensureArray<T>(value: T | ReadonlyArray<T>): ReadonlyArray<T> | ||
| export function ensureArray<T>(value: T | readonly T[]): ReadonlyArray<T> { | ||
| return value instanceof Array ? value : [value] | ||
| return Array.isArray(value) ? value : [value as T] |
There was a problem hiding this comment.
The type assertion as T could potentially mask type errors. Consider using a more explicit type guard or ensuring the generic constraint is properly defined to avoid unsafe casting.
| T extends unknown, | ||
| Key extends string | number | symbol, | ||
| >( | ||
| export function hasKeys<T, Key extends string | number | symbol>( |
There was a problem hiding this comment.
[nitpick] Good simplification of the generic constraint from T extends unknown to just T, which is more concise and equally effective.
| @@ -9,343 +9,343 @@ | |||
| * | |||
| * @group Http | |||
There was a problem hiding this comment.
Converting from enum to const objects is a breaking change that affects how these values are imported and used. Consider documenting this breaking change and providing migration guidance for users upgrading from V1.
| * @group Http | |
| * @group Http | |
| * | |
| * @remarks | |
| * **Breaking Change in V2:** `HttpStatusCodes` was previously exported as a TypeScript `enum` in V1. It is now exported as a `const` object. | |
| * - **Migration:** Update your imports and usage from `import { HttpStatusCodes } from ...` and `HttpStatusCodes.ACCEPTED` to the same property access, but note that reverse mapping (e.g., `HttpStatusCodes[202]`) is no longer supported. | |
| * - If you relied on enum-specific features, refactor your code to use the object properties directly. |
| export const HttpStatusReasons = { | ||
| /** |
There was a problem hiding this comment.
Converting from enum to const objects is a breaking change that affects how these values are imported and used. Consider documenting this breaking change and providing migration guidance for users upgrading from V1.
| export const HttpStatusReasons = { | |
| /** |
| * @param compare find calls predicate once for each element of the array, in ascending | ||
| * order, until it finds one where predicate returns true. If such an element is found, |
There was a problem hiding this comment.
The parameter name in the documentation (compare) doesn't match the actual parameter name in the function signature. This should be consistent with the implementation.
| * @param compare find calls predicate once for each element of the array, in ascending | |
| * order, until it finds one where predicate returns true. If such an element is found, | |
| * @param compare `compare` is called once for each element of the array, in ascending | |
| * order, until it returns true. If such an element is found, |
No description provided.