Skip to content

Fix keys of an interposed Seq iterated in reverse - #2310

Merged
bdurrer merged 1 commit into
immutable-js:mainfrom
b84330808:fix-interpose-reverse-keys
Oct 31, 2026
Merged

bdurrer merged 1 commit into
immutable-js:mainfrom
b84330808:fix-interpose-reverse-keys

Conversation

@b84330808

Copy link
Copy Markdown
Contributor

interpose numbers its keys in iteration order, so when it is iterated in reverse the last value gets key 0:

Seq([1, 2, 3]).interpose(0).lastIndexOf(1); // 4, expected 0
Seq([1, 2, 3]).interpose(0).findLastIndex((x) => x === 3); // 0, expected 4

List#interpose is fine because the List is built in a forward pass, but a lazy Seq (including Range) hits it through lastIndexOf, findLastIndex, findLastEntry, reduceRight and reversed iterators.

It's the same problem as #2309 (filter), in interposeFactory. In reverse, keys now count down from the size of the interposed sequence. When that size is unknown (e.g. after filter), ensureSize works it out first, like #2309 does.

The new test in __tests__/interpose.ts covers a Seq of known and unknown size, through both __iterate and __iterator. It fails on main and passes with this change. The same code is on 6.x.

I used Claude Code to write this change.

`interpose` numbered its keys from 0 in iteration order, so iterating it
in reverse gave the last value key 0. `Seq([1, 2, 3]).interpose(0)
.lastIndexOf(1)` returned 4 instead of 0. Count down from the size of
the interposed sequence when iterating in reverse.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@bdurrer

bdurrer commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

Thanks for testing all these reverse cases and submitting fixes!

I bet without claude, most people wouldn't write or even try to read code like (reverse ? size - ++iterations : iterations++) 😄

@bdurrer
bdurrer merged commit d60ed6e into immutable-js:main Oct 31, 2026
5 checks passed
@b84330808

Copy link
Copy Markdown
Contributor Author

Ha, true 😄 Thanks for the quick reviews and merges!

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