@rwjblue I have applied the requested changes as well added a few more tests.

Please take another round of review.

LGTM, I also pinged @mmun for review...

Should we debug freeze the array? The empty case is frozen like hashes iirc

Should we debug freeze the array? The empty case is frozen like hashes iirc

Yes, probably.

Unfortunately debug freezing is not going to be super easy. We can merge this as-is and do a follow up PR for that. I also just realized that the current version of the glimmer code seems to always return a new array/object even for the empty case.

We can do something like:

class DebugFreezeReference<T> implements PathReference<T> {
  constructor(private inner: PathReference<T>) {
    this.tag = inner.tag;
  }

  value(): T {
    return Object.freeze(this.inner.value());
  }

  get(path: string): PathReference<Opaque> {
    return this.inner.get(path); // or should this be `new DebugFreezeReference(this.inner.get(path))`?
  }
}

function array(_vm: VM, args: Arguments): PathReference<Opaque[]> {
  return new DebugFreezeReference(args.positional.capture());
}

function hash(_vm: VM, args: Arguments): PathReference<Dict<Opaque>> {
  return new DebugFreezeReference(args.named.capture());
}

Looking good. Probably should have a few more test testing "capturing" the array into JS values and make sure it is correct. Something like this:

let captured;

this.registerHelper('capture', function([array]) {
  captured = array;
  return "captured";
});

this.render(`{{capture (array ...)}}`);

assert.deepEqual(captured, [...]);

this.runTask(this.set(...));

assert.deepEqual(captured, [...]);

...

Basically what you already did in the last test, but more coverage around the actual values, and that it updates correctly, etc. You can probably just merge that into your last test since it's already doing similar things.

I have addressed the last few comments from @chancancode. This should be ready for another review.

@josemarluedke have you tried the indentation style in #17054 (comment)? If the linter prefer you write it like this, it's nbd

@chancancode Yes I did, I changed most of the code to that indentation style. I did not change all because some already had a line break due to component args. But if you do think I should change that as well, I'm happy to do so.

Thank you!