Migrate Seq.js to TypeScript and re-parent the Seq hierarchy - #2210
Merged
Conversation
jdeniau
force-pushed
the
migrate-seq-to-ts
branch
2 times, most recently
from
June 10, 2026 19:06
52227f8 to
0ce77f5
Compare
jdeniau
force-pushed
the
migrate-seq-to-ts
branch
from
June 11, 2026 09:01
d6fba55 to
8b543ea
Compare
jdeniau
commented
Jun 17, 2026
Member
Author
|
@copilot review this PR please. |
This comment was marked as low quality.
This comment was marked as low quality.
jdeniau
force-pushed
the
migrate-seq-to-ts
branch
2 times, most recently
from
June 26, 2026 22:29
da58721 to
adf3746
Compare
jdeniau
force-pushed
the
migrate-seq-to-ts
branch
from
June 29, 2026 22:00
6c6b3f9 to
550fc0b
Compare
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
force-pushed
the
migrate-seq-to-ts
branch
from
June 30, 2026 08:22
550fc0b to
249202f
Compare
Member
Author
|
Retour PR Cloclo 🕺🤖 : Re-review après corrections ✅
Vérifs locales sur le HEAD de la PR : |
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.
Summary
Migrates
Seq.jsto TypeScript and re-parents the lazy-sequence hierarchy so each*SeqImplextends its matching*CollectionImpldirectly, replacing the runtimemixin(*SeqImpl, *CollectionPrototype)copies.Stacked on #2209 (
make-size-undefined), which it depends on: the typed*SeqImplinheritsize: number | undefined.No runtime behavior change.
Migration
Seq.ts(replacesSeq.js):KeyedSeqImpl extends KeyedCollectionImpl,IndexedSeqImpl extends IndexedCollectionImpl,SetSeqImpl extends SetCollectionImpl.SeqImplstays as the kind-agnostic base (used byConcatSeq). TheIS_SEQ_SYMBOLbrand,toSeq/to{Keyed,Indexed,Set}Seq,toStringandcacheResult— previously inherited fromSeqImpl— are now defined on each kind class so the re-parenting loses nothing.CollectionImpl.js: the threemixin(*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_cacheis iterated, otherwise they delegate to__iterate*Uncached);to{Indexed,Keyed,Set}Seqbecome throwing placeholders the mixin overwrites, so the Seq subclasses can override them as(): this.Because a
*SeqImplnow is a*CollectionImpl, the Seq/collection-kind boundary casts disappear. The remaining casts are theRecordd.ts boundary (dissolves whenRecordmigrates) andnoUncheckedIndexedAccessassertions on dense arrays.Typing per the public d.ts contract
partitionnarrowed per kind (3*CollectionImpl+ 4*SeqImpl),concatdeclared 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()andRangeImpl.slice()returnRangeImpl(a subtype ofSeq.Indexed<number>).slicekeeps the base: thiscontract: when it rebuilds through the concreteRange()factory it uses anas thiscast — erased at compile time, the same cast the base hides insidereify/sliceFactory.makeSequence/collectionClassgain an unknown-kind base overload.Type tests (
ts-tests-src)Un-skips the
ts-tests-srcsource-pass duplicates now covered by the migration:seq.ts(3/4),empty.ts(2/2),range.ts(1/1 + a new#sliceassertion),functional.ts(5/7),partition.ts(2/7) — tstyche actives go 212 → 225. The assertions reference the internal*Implclasses imported straight from the source until the public type names are emitted; each remaining skip documents its blocker (unmigratedList/Map/Record, mixinconcat, d.ts-only types).Docs
.agents/commands/migrate-to-ts.md: adds a section on covariantthisreturn types — the decision rule for leaf overrides (slice/reverse/sort/…) and when the localizedas thiscast is the right tool.CHANGELOG.md: notes theRangeTypeScript migration underUnreleased(and moves the misplaced5.1.9entry below6.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 ✅