diff --git a/packages/@glimmer-workspace/integration-tests/lib/render-test.ts b/packages/@glimmer-workspace/integration-tests/lib/render-test.ts index 3bb2198f2a0..fcb040348a2 100644 --- a/packages/@glimmer-workspace/integration-tests/lib/render-test.ts +++ b/packages/@glimmer-workspace/integration-tests/lib/render-test.ts @@ -448,14 +448,16 @@ export class RenderTest implements IRenderTest { private assertEachCompareResults( items: (number | string | [string | number, string | number])[] ) { - [...(this.element as unknown as HTMLElement).querySelectorAll('.test-item')].forEach( - (el, index) => { - let key = Array.isArray(items[index]) ? items[index][0] : index; - let value = Array.isArray(items[index]) ? items[index][1] : items[index]; + let rendered = (this.element as unknown as HTMLElement).querySelectorAll('.test-item'); - QUnit.assert.equal(el.textContent, `${key}.${value}`, `Comparing the rendered key.value`); - } - ); + QUnit.assert.strictEqual(rendered.length, items.length, `Comparing the rendered item count`); + + rendered.forEach((el, index) => { + let key = Array.isArray(items[index]) ? items[index][0] : index; + let value = Array.isArray(items[index]) ? items[index][1] : items[index]; + + QUnit.assert.equal(el.textContent, `${key}.${value}`, `Comparing the rendered key.value`); + }); } protected assertReactivity( diff --git a/packages/@glimmer-workspace/integration-tests/lib/suites/each.ts b/packages/@glimmer-workspace/integration-tests/lib/suites/each.ts index a8bc509b94c..576e8f81d94 100644 --- a/packages/@glimmer-workspace/integration-tests/lib/suites/each.ts +++ b/packages/@glimmer-workspace/integration-tests/lib/suites/each.ts @@ -46,6 +46,46 @@ export class EachSuite extends RenderTest { this.assertStableNodes(); } + @test + 'a plain array that shrinks during iteration stops the iteration'() { + let list: Item[] = []; + + // The getter is on the prototype so that `JSON.stringify` in `render` does not call it. + class Item { + constructor(private index: number) {} + + get name() { + if (this.index === 0) list.length = 1; + return `item-${this.index}`; + } + } + + for (let i = 0; i < 4; i++) list.push(new Item(i)); + + this.render('{{#each this.list key="@index" as |item|}}[{{item.name}}]{{/each}}', { list }); + this.assertHTML('[item-0]'); + } + + @test + 'a plain array that grows during iteration renders the new items'() { + let list: Item[] = []; + + // The getter is on the prototype so that `JSON.stringify` in `render` does not call it. + class Item { + constructor(private index: number) {} + + get name() { + if (this.index === 0) list.push(new Item(1)); + return `item-${this.index}`; + } + } + + list.push(new Item(0)); + + this.render('{{#each this.list key="@index" as |item|}}[{{item.name}}]{{/each}}', { list }); + this.assertHTML('[item-0][item-1]'); + } + @test 'autotracked custom iterable'() { if (typeof Symbol !== 'function') { diff --git a/packages/@glimmer/reference/lib/iterable.ts b/packages/@glimmer/reference/lib/iterable.ts index 71134eb5c2b..20366c736e1 100644 --- a/packages/@glimmer/reference/lib/iterable.ts +++ b/packages/@glimmer/reference/lib/iterable.ts @@ -225,42 +225,29 @@ class IteratorWrapper implements OpaqueIterator { } class ArrayIterator implements OpaqueIterator { - private current: { kind: 'empty' } | { kind: 'first'; value: unknown } | { kind: 'progress' }; - private pos = 0; + private pos = -1; + private empty: boolean; constructor( private iterator: unknown[] | readonly unknown[], private keyFor: KeyFor ) { - if (iterator.length === 0) { - this.current = { kind: 'empty' }; - } else { - this.current = { kind: 'first', value: iterator[this.pos] }; - } + // Only this read runs in the tracking frame of the iterator reference. + this.empty = iterator.length === 0; } isEmpty(): boolean { - return this.current.kind === 'empty'; + return this.empty; } next(): Nullable> { - let value: unknown; - - let current = this.current; - if (current.kind === 'first') { - this.current = { kind: 'progress' }; - value = current.value; - } else if (this.pos >= this.iterator.length - 1) { - return null; - } else { - value = this.iterator[++this.pos]; - } + let memo = ++this.pos; - let { keyFor } = this; + // The length is live because code in the block can change a plain array. + if (memo >= this.iterator.length) return null; - let key = keyFor(value, this.pos); - let memo = this.pos; + let value = this.iterator[memo]; - return { key, value, memo }; + return { key: this.keyFor(value, memo), value, memo }; } }