From 8ea7c7c7e421ad8c739a0da949703cd3c3f7dbdd Mon Sep 17 00:00:00 2001 From: Dramex Date: Mon, 20 Apr 2026 22:15:29 +0300 Subject: [PATCH] fix(collection): preserve ReadonlyCollection through tap/each (#11501) * fix(collection): preserve ReadonlyCollection through tap/each `each` and `tap` return polymorphic `this`, which TypeScript resolves against the `Omit` portion of `ReadonlyCollection` rather than the full intersection. That let callers reach `set` and `delete` on the result of a chain started from a `ReadonlyCollection`: const ro: ReadonlyCollection = new Collection(...); ro.tap(() => {}).set('x', 0); // compiled, mutated the underlying Map The fix omits `each` and `tap` from the base `Omit` and re-declares them on the `ReadonlyCollection` side of the intersection so the return type narrows back to `ReadonlyCollection`. Closes #10514 * test(collection): gate readonly-chain checks behind if(false) Previously the `@ts-expect-error` lines still executed the `set` and `delete` mutations at runtime, and the final `size === 1` passed only because they happened to cancel out. Wrapping the assertions in `if (false)` keeps the compile-time guarantee while the backing collection is truly untouched, and adds a `get('a') === 1` check as a belt. * test(collection): move readonly type checks to *.test-d.ts Addresses review feedback. The type-level assertions around tap() and each() preserving ReadonlyCollection belong in a *.test-d.ts file so they run through vitest's typecheck pass instead of runtime. Replaces the if(false)-gated @ts-expect-error block in collection.test.ts with expectTypeOf assertions in a new collection.test-d.ts. Covers both the no-thisArg and with-thisArg overloads of tap and each. --- .../collection/__tests__/collection.test-d.ts | 16 ++++++++++++++++ packages/collection/src/collection.ts | 17 +++++++++++++++-- 2 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 packages/collection/__tests__/collection.test-d.ts diff --git a/packages/collection/__tests__/collection.test-d.ts b/packages/collection/__tests__/collection.test-d.ts new file mode 100644 index 000000000..3a9bedbb2 --- /dev/null +++ b/packages/collection/__tests__/collection.test-d.ts @@ -0,0 +1,16 @@ +import { expectTypeOf, test } from 'vitest'; +import { Collection, type ReadonlyCollection } from '../src/index.js'; + +test('ReadonlyCollection#tap preserves the readonly type', () => { + const readonly: ReadonlyCollection = new Collection([['a', 1]]); + + expectTypeOf(readonly.tap(() => {})).toEqualTypeOf>(); + expectTypeOf(readonly.tap(() => {}, null)).toEqualTypeOf>(); +}); + +test('ReadonlyCollection#each preserves the readonly type', () => { + const readonly: ReadonlyCollection = new Collection([['a', 1]]); + + expectTypeOf(readonly.each(() => {})).toEqualTypeOf>(); + expectTypeOf(readonly.each(() => {}, null)).toEqualTypeOf>(); +}); diff --git a/packages/collection/src/collection.ts b/packages/collection/src/collection.ts index 9e2930ef4..f6800a181 100644 --- a/packages/collection/src/collection.ts +++ b/packages/collection/src/collection.ts @@ -5,9 +5,22 @@ */ export type ReadonlyCollection = Omit< Collection, - keyof Map | 'ensure' | 'reverse' | 'sort' | 'sweep' + keyof Map | 'each' | 'ensure' | 'reverse' | 'sort' | 'sweep' | 'tap' > & - ReadonlyMap; + ReadonlyMap & { + each( + fn: (value: Value, key: Key, collection: ReadonlyCollection) => void, + ): ReadonlyCollection; + each( + fn: (this: This, value: Value, key: Key, collection: ReadonlyCollection) => void, + thisArg: This, + ): ReadonlyCollection; + tap(fn: (collection: ReadonlyCollection) => void): ReadonlyCollection; + tap( + fn: (this: This, collection: ReadonlyCollection) => void, + thisArg: This, + ): ReadonlyCollection; + }; export interface Collection { /**