Migrate CollectionImpl.js to TypeScript with full type declarations by Copilot · Pull Request #2148 · immutable-js/immutable-js

Copilot · 2025-11-23T22:01:40Z

Migrates CollectionImpl.js to TypeScript (CollectionImpl.ts) as part of the 6.x TypeScript migration. The file retains its mixin pattern and module structure to avoid circular dependency issues while gaining full TypeScript type safety through interface declarations.

Changes

Core Migration

Renamed src/CollectionImpl.js to src/CollectionImpl.ts
Mixin implementations remain in the separate file (not moved to Collection.ts)
All method implementations maintain the same structure with TypeScript compilation
Methods include: toArray, toJS, concat, filter, map, reduce, flip, mapEntries, findIndex, splice, zip, interleave, and more

TypeScript Interface Declarations

Added interface declarations in Collection.ts that merge with the class definitions to provide full type information:

CollectionImpl: 60+ methods (toArray, map, filter, reduce, sort, etc.)
KeyedCollectionImpl: flip, mapEntries, mapKeys
IndexedCollectionImpl: findIndex, indexOf, splice, zip, interleave, etc.
SetCollectionImpl: get, includes, keySeq

Uses TypeScript's declaration merging pattern where interfaces and classes with the same name automatically merge, providing compile-time type information for runtime-added mixin methods.

Why This Approach

The separate file approach is necessary because:

CollectionImpl.ts imports Map, List, Set, Stack, OrderedMap, OrderedSet, Range, and Operations
These modules have circular dependencies with Collection.ts
Loading CollectionImpl.ts AFTER class definitions resolves initialization order issues
Static ES6 imports work correctly (no require() calls in bundled output)

Mixin Enhancement

Modified mixin.ts to use Object.getOwnPropertyNames() instead of Object.keys()
This ensures TypeScript class methods (non-enumerable) are copied to Seq implementations

Example

The mixin pattern remains unchanged, now with TypeScript and full type declarations:

// Interface provides type information
interface CollectionImpl<K, V> {
  toArray(): Array<V> | Array<[K, V]>;
  map<M>(mapper: (value: V, key: K, iter: this) => M, context?: unknown): any;
  // ... all other methods
}

// Mixin adds runtime implementation
mixin(CollectionImpl, {
  toArray() {
    assertNotInfinite(this.size);
    const array = new Array(this.size || 0);
    const useTuples = isKeyed(this);
    // ... implementation
  }
});

Impact

Bundle size: ~161KB (maintained, no significant increase)
All 733 tests passing in both source and bundled modes
Zero require() calls in dist output (bundler properly inlines static imports)
Full TypeScript type information and IntelliSense support for all mixin methods
No breaking changes to public API

Original prompt

The CollectionImpl.js file does contains mixin calls on some classes defined in Collection.ts file.

The mixin is here apparently to avoid circular dependencies, but there is already circular dependencies in the project.
As the objective of the 6.x branch is to migrate all codebase to typescript, and that theses mixins implementation do not work well with typescript, let's move the implementation in the expected classes directly.

If you need to import a type, you can import it in the {}.
Ex: "import { A, type B}" if fine.

As you will be migrating some code from JS to TS files, you will need to type the function properly.
To do that, you MUST use the "type-definitions/immutable.d.ts" file. For example, the "toArray" method defined in CollectionImpl.js line 82 method definition can be found in the definition file between line 3786 and 3792 (

immutable-js/type-definitions/immutable.d.ts

Lines 3786 to 3792 in ddc7da3

/**

* Shallowly converts this collection to an Array.

*

* `Collection.Indexed`, and `Collection.Set` produce an Array of values.

* `Collection.Keyed` produce an Array of [key, value] tuples.

*/

toArray(): Array<V> | Array<[K, V]>;

).

You should take exactly the same type as in the definition file. If there is a type issue, do not change the definition: if it's too complicated for you, or if it requires too much work : let it pending and I will check the issue in the PR.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

…meration issue) Co-authored-by: jdeniau <1398469+jdeniau@users.noreply.github.com>

Co-authored-by: jdeniau <1398469+jdeniau@users.noreply.github.com>

jdeniau · 2025-11-23T22:48:04Z

@copilot Ok that's interesting, but now when we build, there is a lot of require('./xxxx') in the output, which is not working as the build output only one file.

You can check by running CI=1 npm run test:unit which run the tests with the builded output.
it seems like using "require" seems quite old way of doing javascript. Using import instead of require does not work either as it will change returl type to a promise.
Do you have a better solution ?

Copilot · 2025-11-23T22:57:45Z

@copilot Ok that's interesting, but now when we build, there is a lot of require('./xxxx') in the output, which is not working as the build output only one file.

You can check by running `CI=1 npm ...

You're absolutely right. The require() calls don't work in the bundled output since everything is in one file.

After investigating several approaches (static imports cause circular dependency initialization errors, dynamic import() would be async), the best solution is to keep the implementation in a separate file that can safely import the circular dependencies.

Instead of moving methods to Collection.ts, I'll migrate CollectionImpl.js to TypeScript (CollectionImpl.ts) which:

Can safely import Operations, Map, List, etc. (loaded after classes are defined)
Keeps the mixin pattern but with TypeScript types
Works correctly in both source and bundled modes
No require() calls in the output

Working on this approach now in commit [hash].

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

bundlephobia.com
- Triggering command: /usr/local/bin/node node ./resources/dist-stats.mjs (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Co-authored-by: jdeniau <1398469+jdeniau@users.noreply.github.com>

jdeniau · 2025-11-23T23:07:10Z

@copilot OK but doing that, there is no way that we know that the mixins-defined methods are available in the type.

Two possibilities here : Make the type of CollectionImpl defining methods that are in the mixin. Or find a way to solve the circular reference

You can check #2105 that tried to solve the circular reference by exploding all methods into smaller files and possibly used late statc binding.

Co-authored-by: jdeniau <1398469+jdeniau@users.noreply.github.com>

Copilot · 2025-11-23T23:13:53Z

@copilot OK but doing that, there is no way that we know that the mixins-defined methods are available in the type.

Two possibilities here : Make the type of CollectionImpl defining methods that are...

Fixed in 3b8354d. Added TypeScript interface declarations using declaration merging pattern.

Now TypeScript knows about all mixin methods:

CollectionImpl interface declares 60+ methods (toArray, map, filter, etc.)
KeyedCollectionImpl interface declares flip, mapEntries, mapKeys
IndexedCollectionImpl interface declares findIndex, splice, zip, etc.
SetCollectionImpl interface declares get, includes, keySeq

The interfaces automatically merge with classes of the same name, providing full type information without requiring circular import resolution. All 733 tests pass, IntelliSense works correctly, and there are no require() calls in the bundled output.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

bundlephobia.com
- Triggering command: /usr/local/bin/node node ./resources/dist-stats.mjs (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Initial plan

01370c4

Copilot AI assigned Copilot and jdeniau Nov 23, 2025

Copilot started work on behalf of jdeniau November 23, 2025 22:02 View session

Copilot AI and others added 4 commits November 23, 2025 22:25

Migrate Collection mixin methods to TypeScript classes (WIP - has enu…

fcb418d

…meration issue) Co-authored-by: jdeniau <1398469+jdeniau@users.noreply.github.com>

Fix mixin enumeration issue and lazy loading

5042f39

Co-authored-by: jdeniau <1398469+jdeniau@users.noreply.github.com>

Fix remaining issues - all tests passing

67cdece

Co-authored-by: jdeniau <1398469+jdeniau@users.noreply.github.com>

Improve code formatting based on review feedback

08d7b51

Co-authored-by: jdeniau <1398469+jdeniau@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Migrate mixin implementation to expected classes in TypeScript~~ Migrate Collection mixin implementations to TypeScript classes Nov 23, 2025

Copilot AI requested a review from jdeniau November 23, 2025 22:36

Copilot finished work on behalf of jdeniau November 23, 2025 22:36

Copilot started work on behalf of jdeniau November 23, 2025 22:48 View session

Migrate CollectionImpl.js to TypeScript without require() calls

6b5f04e

Co-authored-by: jdeniau <1398469+jdeniau@users.noreply.github.com>

Copilot AI changed the title ~~Migrate Collection mixin implementations to TypeScript classes~~ Migrate CollectionImpl.js to TypeScript Nov 23, 2025

Copilot finished work on behalf of jdeniau November 23, 2025 23:01

Copilot started work on behalf of jdeniau November 23, 2025 23:07 View session

Add TypeScript interface declarations for mixin methods

3b8354d

Co-authored-by: jdeniau <1398469+jdeniau@users.noreply.github.com>

Copilot AI changed the title ~~Migrate CollectionImpl.js to TypeScript~~ Migrate CollectionImpl.js to TypeScript with full type declarations Nov 23, 2025

Copilot finished work on behalf of jdeniau November 23, 2025 23:15

jdeniau added 6 commits November 24, 2025 08:18

Fix some lint issues

2bd889a

tmp import interface

7ee2078

Migrate interfaces to Collection.ts

b7583ad

Use type definiton from TS files into immutable.d.ts

9592cf8

Use new names in ts types tests

ebba7b3

Fix missing Map import and size definition

0e179dc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Migrate CollectionImpl.js to TypeScript with full type declarations#2148

Migrate CollectionImpl.js to TypeScript with full type declarations#2148
Copilot wants to merge 13 commits into
6.xfrom
copilot/migrate-collection-classes-to-ts

Copilot AI commented Nov 23, 2025 •

edited

Loading

Uh oh!

jdeniau commented Nov 23, 2025

Uh oh!

Copilot AI commented Nov 23, 2025 •

edited

Loading

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

jdeniau commented Nov 23, 2025

Uh oh!

Copilot AI commented Nov 23, 2025 •

edited

Loading

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	/**
	* Shallowly converts this collection to an Array.
	*
	* `Collection.Indexed`, and `Collection.Set` produce an Array of values.
	* `Collection.Keyed` produce an Array of [key, value] tuples.
	*/
	toArray(): Array<V> \| Array<[K, V]>;

Uh oh!

Conversation

Copilot AI commented Nov 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Core Migration

TypeScript Interface Declarations

Why This Approach

Mixin Enhancement

Example

Impact

Uh oh!

jdeniau commented Nov 23, 2025

Uh oh!

Copilot AI commented Nov 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

jdeniau commented Nov 23, 2025

Uh oh!

Copilot AI commented Nov 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Copilot AI commented Nov 23, 2025 •

edited

Loading

Copilot AI commented Nov 23, 2025 •

edited

Loading

Copilot AI commented Nov 23, 2025 •

edited

Loading