{{ message }}
Conversation
…able-js into alexvictoor-no-constructor-returns
* Call super()" before this in constructors * Fix issue with MapIterator re-setting the next method
…wsers might re-compile immutable
Replace buble by babel
Define "exports" in package.json
Everything in the mixin that does not depend on module-evaluation order becomes a real typed method on Collection.ts: - base: toArray, toJSON, entrySeq (replacing its throwing placeholder), __toString, __toStringMapper, contains, inspect, toSource - keyed: mapEntries, mapKeys, plus the toArray/toJSON/__toStringMapper narrowings - indexed/set: toArray/toJSON narrowings, Set#has Symbol.iterator and Set#keys must stay reference-equal to the iteration method they delegate to (tested behaviour), so they move as prototype alias assignments below the classes, with declare members for the type. isOrdered's guard now intersects the IS_ORDERED_SYMBOL brand it actually tests: the public OrderedCollection shape (toArray + Symbol.iterator) is structurally matched by every collection now that the base class types them, which made negative isOrdered checks narrow plain collections to never (deepEqual). methods/toObject.js only backed the keyed toJSON alias and is now orphaned; delete it. The mixin keeps only the methods that cannot move yet: the To*Sequence/concat group (importing operations/sequences.ts from Collection.ts would put two eval-time extends edges in the same import cycle) and the late-bound concrete-collection conversions.
The base docstrings were carried over with the mixin migration, but the kind-specific overrides have their own wording in immutable.d.ts (Keyed: Object form + key stringification; Indexed/Set: Array form).
- concatFactory: the base collection type is statically iterable since Symbol.iterator moved out of the mixin onto CollectionImpl, so the stale comment is rewritten and the `as unknown as` bridge becomes a plain downcast (the remaining lie is only the `unknown` element type). - entrySeq: the patched `fromEntrySeq` round-trip is a plain downcast too (KeyedSeqImpl subtypes the SeqImpl returned by toSeq); the TODO documenting the indexed-source lie stays. - ToIndexedSequence: replace the `?? 0` fallbacks with non-null assertions plus a comment, stating the ensureSize invariant instead of masking it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WvEjriPc3Rbx3sqApa2coD
Only `this.includes` is off-limits (it would recurse through Set's own `includes` → `has` delegation); `super.includes` is exactly the wanted semantics and avoids duplicating the base method body. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WvEjriPc3Rbx3sqApa2coD
Unconditional assignment creates own properties whose value is `undefined` when the first iterable is unbranded. Today the predicates test truthiness so this is invisible, but the `in`-based checks planned for 6.0 (see the isOrdered @ts-expect-error note) would read such a property as branded, making every ConcatSeq look keyed, indexed and ordered at once. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WvEjriPc3Rbx3sqApa2coD
The `& { [IS_ORDERED_SYMBOL]: true }` intersection was repeated across
the three guard signatures; a named OrderedBrand alias carries the
rationale once and keeps the signatures readable.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WvEjriPc3Rbx3sqApa2coD
…es-to-ts Migrate operations/sequences.js to TypeScript and move the order-safe mixin methods onto the classes
The mixin entries in CollectionImpl.js only existed because Collection.ts cannot import operations/sequences.ts without a cycle. Installing the methods from sequences.ts itself keeps them next to the wrapping sequence classes they build, fully typed, and shrinks the mixin further. Collection.ts now declares the public types: fromEntrySeq joins the throwing placeholders and concat gets its base d.ts signature, which also makes the existing *SeqImpl.concat declare-narrowings valid against a real (bivariant) base method. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EJWDpKjoLcWomxKD3WEmvd
Snapshot of migrated vs remaining files, the mixin dismantling state, the TODO [TS-MIGRATION] inventory, the source-pass type-test skip list, and the findings of a behavioural JS->TS consistency review (recorded here, to be addressed in follow-up PRs). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EJWDpKjoLcWomxKD3WEmvd
Per review on the follow-up PR: the kind narrowings from the d.ts land with the migration itself instead of a deferred TODO. The three *CollectionImpl classes get declare properties (type-only, valid against the base bivariant method), and SeqImpl redeclares concat as a real method for the public Seq contract — a declare property there would break the structural *SeqImpl -> SeqImpl assignability — overwritten at runtime by operations/sequences.ts like the base one, via a new Seq-typed concatFactory overload. Un-skips the 'Set concat' source-pass type test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EJWDpKjoLcWomxKD3WEmvd
The end goal is to emit the public .d.ts from the sources, so migrated types must stay as close as possible to immutable.d.ts and may only deviate in the stricter direction (tighter types, or new type parameters replacing unknown/any). Refresh the status report accordingly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EJWDpKjoLcWomxKD3WEmvd
…igration-bctlmg Move sequence methods from mixin and add TS-migration status report
countByFactory/groupByFactory keep their runtime bodies; the Map they build is typed through the d.ts import until Map.js is migrated. The countBy/groupBy mixin entries move onto the prototype from aggregations.ts itself — same pattern as operations/sequences.ts — with throwing placeholders carrying the public d.ts types on CollectionImpl. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EJWDpKjoLcWomxKD3WEmvd
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EJWDpKjoLcWomxKD3WEmvd
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EJWDpKjoLcWomxKD3WEmvd
…gations-to-ts Migrate operations/aggregations.js to TypeScript
Fix some migration issue reported by AI
Each finding from .agents/migration-status.md was traced back to its originating PR, then fixed or confirmed as an intentional change: - R1 (isSubset string argument, from #2204): keep the new character-iteration semantics, consistent with isSuperset and with Collection('abc'); documented as a v6 breaking change in the CHANGELOG and pinned by multi-char unit tests. - R2 ('@@iterator' fallback removal, from #2127): confirmed intended modernisation; explicit CHANGELOG entry, and the two immutable.d.ts docstrings no longer mention @@iterator. immutable.js.flow is left untouched since Flow's @@iterator syntax denotes Symbol.iterator. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0122shfP2ngkkjuXZBWoeFGW
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0122shfP2ngkkjuXZBWoeFGW
…-yzlr2p Resolve the three remaining likely-bugs findings (R1-R4)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

No description provided.