🌐 US-Proxy
class="logged-out env-production page-responsive" style="word-wrap: break-word;" >
Skip to content

Migrate Seq.js to TypeScript and re-parent the Seq hierarchy - #2210

Merged
jdeniau merged 10 commits into
6.xfrom
migrate-seq-to-ts
Jul 12, 2026
Merged

Migrate Seq.js to TypeScript and re-parent the Seq hierarchy#2210
jdeniau merged 10 commits into
6.xfrom
migrate-seq-to-ts

Conversation

@jdeniau

@jdeniau jdeniau commented Jun 9, 2026

Copy link
Copy Markdown
Member

Summary

Migrates Seq.js to TypeScript and re-parents the lazy-sequence hierarchy so each *SeqImpl extends its matching *CollectionImpl directly, replacing the runtime mixin(*SeqImpl, *CollectionPrototype) copies.

Stacked on #2209 (make-size-undefined), which it depends on: the typed *SeqImpl inherit size: number | undefined.

No runtime behavior change.

Migration

  • Seq.ts (replaces Seq.js): KeyedSeqImpl extends KeyedCollectionImpl, IndexedSeqImpl extends IndexedCollectionImpl, SetSeqImpl extends SetCollectionImpl. SeqImpl stays as the kind-agnostic base (used by ConcatSeq). The IS_SEQ_SYMBOL brand, toSeq/to{Keyed,Indexed,Set}Seq, toString and cacheResult — previously inherited from SeqImpl — are now defined on each kind class so the re-parenting loses nothing.
  • CollectionImpl.js: the three mixin(*SeqImpl, *CollectionPrototype) calls are removed — the kind-specific methods are now inherited through the prototype chain.
  • Collection.ts: the lazy-cache dispatch moves into the base __iterate/__iterator (a present _cache is iterated, otherwise they delegate to __iterate*Uncached); to{Indexed,Keyed,Set}Seq become throwing placeholders the mixin overwrites, so the Seq subclasses can override them as (): this.

Because a *SeqImpl now is a *CollectionImpl, the Seq/collection-kind boundary casts disappear. The remaining casts are the Record d.ts boundary (dissolves when Record migrates) and noUncheckedIndexedAccess assertions on dense arrays.

Typing per the public d.ts contract

  • partition narrowed per kind (3 *CollectionImpl + 4 *SeqImpl), concat declared on the kind Seq classes, conversion methods return the Seq types (toSeq(): Seq<K, V>, entrySeq(), keySeq(), …).
  • Collection() / Collection.Indexed() / Collection.Set() callable without arguments, as the d.ts declares.
  • Range() and RangeImpl.slice() return RangeImpl (a subtype of Seq.Indexed<number>). slice keeps the base : this contract: when it rebuilds through the concrete Range() factory it uses an as this cast — erased at compile time, the same cast the base hides inside reify/sliceFactory.
  • makeSequence / collectionClass gain an unknown-kind base overload.

Type tests (ts-tests-src)

Un-skips the ts-tests-src source-pass duplicates now covered by the migration: seq.ts (3/4), empty.ts (2/2), range.ts (1/1 + a new #slice assertion), functional.ts (5/7), partition.ts (2/7) — tstyche actives go 212 → 225. The assertions reference the internal *Impl classes imported straight from the source until the public type names are emitted; each remaining skip documents its blocker (unmigrated List/Map/Record, mixin concat, d.ts-only types).

Docs

  • .agents/commands/migrate-to-ts.md: adds a section on covariant this return types — the decision rule for leaf overrides (slice/reverse/sort/…) and when the localized as this cast is the right tool.
  • CHANGELOG.md: notes the Range TypeScript migration under Unreleased (and moves the misplaced 5.1.9 entry below 6.0.0).

Test plan

  • npm run type-check (3 projects) ✅
  • npm run build ✅ (no module eval-order regression)
  • npm run test:unit — 795 passed ✅
  • npm run test:types (tstyche) — 888 assertions passed, 225 tests active / 197 skipped ✅
  • eslint + prettier on changed files ✅

Base automatically changed from make-size-undefined to 6.x June 10, 2026 18:31
@jdeniau
jdeniau force-pushed the migrate-seq-to-ts branch 2 times, most recently from 52227f8 to 0ce77f5 Compare June 10, 2026 19:06
@jdeniau
jdeniau force-pushed the migrate-seq-to-ts branch from d6fba55 to 8b543ea Compare June 11, 2026 09:01
jdeniau added a commit that referenced this pull request Jun 17, 2026
Comment thread src/Range.ts Outdated
@jdeniau

jdeniau commented Jun 17, 2026

Copy link
Copy Markdown
Member Author

@copilot review this PR please.

This comment was marked as low quality.

pull Bot pushed a commit to itsrbpandit/immutable-js that referenced this pull request Jun 17, 2026
@jdeniau
jdeniau force-pushed the migrate-seq-to-ts branch 2 times, most recently from da58721 to adf3746 Compare June 26, 2026 22:29
@jdeniau
jdeniau force-pushed the migrate-seq-to-ts branch from 6c6b3f9 to 550fc0b Compare June 29, 2026 22:00
jdeniau

This comment was marked as resolved.

jdeniau and others added 10 commits June 30, 2026 08:21
Seq.js becomes Seq.ts, and the `*SeqImpl` classes now extend their matching
`*CollectionImpl` directly instead of a shared `SeqImpl` plus runtime mixins:

- `KeyedSeqImpl extends KeyedCollectionImpl`, `IndexedSeqImpl extends
  IndexedCollectionImpl`, `SetSeqImpl extends SetCollectionImpl`. They inherit
  the kind-specific methods, so the three `mixin(*SeqImpl, *CollectionPrototype)`
  calls are removed from CollectionImpl.js.
- `SeqImpl` remains the kind-agnostic base (used by ConcatSeq).
- The lazy-cache dispatch moves from SeqImpl into the base CollectionImpl
  `__iterate`/`__iterator`: a present `_cache` is iterated, otherwise they
  delegate to `__iterateUncached`/`__iteratorUncached`. Concrete collections
  override `__iterate`/`__iterator` and are unaffected.
- `to{Indexed,Keyed,Set}Seq` become throwing placeholders (overwritten by the
  mixin) so the Seq subclasses can override them as `(): this`.

Because a `*SeqImpl` now is a `*CollectionImpl`, the Seq/collection-kind
boundary casts are gone. The casts that remain are the Record d.ts boundary
(Record is not migrated yet) and noUncheckedIndexedAccess assertions on dense
arrays.

No runtime behavior change.
Several signatures in the migrated source were narrower or looser than the
public contract in immutable.d.ts; align them:

- partition is narrowed per kind on the three *CollectionImpl and the four
  *SeqImpl classes ([Keyed<K, V>, Keyed<K, F>] for the type-guard overload,
  [this, this] otherwise), with super-delegating bodies
- concat is declared on the three kind Seq classes per the Seq.Keyed /
  Seq.Indexed / Seq.Set contracts. It cannot live on SeqImpl: a `declare`
  property is checked with strict parameter contravariance and would break
  the structural *SeqImpl → SeqImpl assignability toSeq relies on; the
  base-level concat arrives with the mixin migration as a real method
- the conversion methods return the Seq types per the contract
  (toSeq(): Seq<K, V>, toIndexedSeq(): Seq.Indexed<V>, entrySeq(),
  keySeq(), valueSeq(), fromEntrySeq()), with per-kind toSeq overrides
- Collection() / Collection.Indexed() / Collection.Set() are callable
  without arguments, as the d.ts declares
- Range() and RangeImpl.slice() return Seq.Indexed<number> (the public
  contract) instead of leaking the concrete RangeImpl class
- makeSequence / collectionClass gain an unknown-kind base overload: the
  factories called them with a plain CollectionImpl, which only passed by
  structurally matching the Set overload — the per-kind Seq members now
  make the kinds structurally distinct
- SetCollection narrows its non-associative branch with a documented cast:
  set-kind-ness has no positive brand the type system could narrow on
The ts-tests-src duplicates resolve `immutable` against the TS source. The
source does not emit the public type names (Seq.Indexed<T>, …) yet, so the
un-skipped assertions reference the internal *Impl classes imported straight
from the source, to be switched back once the public names exist:

- seq.ts: constructor, size (mutable in the source until the public readonly
  layer exists), Seq.Indexed concat; the base-Seq concat test stays skipped
  (concat lives in the not-yet-migrated mixin and cannot be declared on
  SeqImpl)
- empty.ts: all typed empty Seq and Collection factories
- range.ts: the Range constructor
- functional.ts: get, has, set, remove, update (plain object/array forms);
  getIn/hasIn stay skipped (they exercise the unmigrated List/Map)
- partition.ts: the Collection and Seq blocks, enabled by the per-kind
  partition typing; the concrete-collection blocks stay skipped
@jdeniau
jdeniau force-pushed the migrate-seq-to-ts branch from 550fc0b to 249202f Compare June 30, 2026 08:22
@jdeniau

jdeniau commented Jun 30, 2026

Copy link
Copy Markdown
Member Author

Retour PR Cloclo 🕺🤖 : Re-review après corrections

  • 🔴 SetSeqImpl.toString — corrigé dans 249202f11, thread résolu. Test de non-régression sur les 3 kinds backporté sur 6.x (Seq toString across the hierarchy). Du coup la mention « No runtime behavior change » de la description tient à nouveau.
  • 🟠 Description vs contenu — la description a été réécrite : section « Unit tests » fantôme retirée, section « Type tests (ts-tests-src) » qui décrit les vrais tests du diff, et bullet Range corrigé (Range()/.slice() renvoient RangeImpl).
  • 🟢 Garde entry && (Collection.ts) et placement de 5.1.9 (CHANGELOG) — notes informatives, threads résolus.

Vérifs locales sur le HEAD de la PR : type-check 0 erreur · jest Seq 77 passed · eslint/prettier OK. Plus rien de bloquant de mon côté. 👍

@jdeniau
jdeniau merged commit 2893dfb into 6.x Jul 12, 2026
5 checks passed
@jdeniau
jdeniau deleted the migrate-seq-to-ts branch July 12, 2026 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants