[Glimmer2] Add each-in support

[Joelkang]

I've implemented this with a simple AST transform as opposed to creating a whole new syntax for each-in (or exporting the EachSyntax in glimmer and then instantiating an each-in specific EachSyntax object). Let me know this is the right way to do it.

Also for some reason internal mutations of the underlying object don't cause a rerender even though the object reference is a regular PropertyReference and not an UnboundReference. Do I need to create an UnboundReference for this particular case?

[chancancode]

Need to handle the case when only one block param is supplied

[Joelkang]

i actually did that at first by creating b.path('_') -- but you know what, I think it's cause we don't have a test case for this. will add

[chancancode]

_ is dangerous, because it might shadow an putter variable (or self lookup) with that same name. If you do the wrapping trick I mentioned below, you could simply flip the value and memo in the iterator and not worry about rearranging the block params here

[krisselden]

@Joelkang I needed to bump glimmer so I pull in the changes for each to be compatible, this needs a rebase now

[homu]

☔ The latest upstream changes (presumably #13493) made this pull request unmergeable. Please resolve the merge conflicts.

[Joelkang]

The object test cases pass in objects for both truthy and falsy values: { isTruthy: this.falsyValue }, so it will always render T1T2. If we want to include them here perhaps I should update #13441 to better reflect the test cases we care about rather than testing for objects with isTruthy.

[Joelkang]

I didnt know that {{each-in}} didn't support get in htmlbars; do we want to support it in glimmer2?

Also I don't know why when i ran ember test locally it showed that it was passing =(

[chancancode]

I think you can just do:

class ProxyReference {
  constructor(inner) {
    this.tag = inner.tag;
    this.inner = inner;
  }

  value() {
    this.inner.value();
  }

  get(key) {
    return this.inner.get(key);
  }
}

class EachInReference extends ProxyReference {
  constructor(inner) {
    super(inner);
    this[EACH_IN_REFERENCE] = true;
  }
}

Might want to put the ProxyReference in utils.

One problem with this is that you loose the const-ness on the original reference and any extra things like [UPDATE]. For propagating const-ness, we can do this:

class ProxyReference {
  static create(inner) {
    if (isConst(inner)) {
      return new ConstProxyReference(inner);
    } else {
      return new this(inner);
    }
  }

  // ...
}

class ConstProxyReference extends ConstReference {
  // ... repeat the same code
}

But now you have the EachInReference subclass problem :(

So at the end of the day, what we really want to do is:

export default {
    isInternalHelper: true,
    toReference(args) {
      let wrapped = Object.create(args.positional.at(0));
      wrapped[EACH_IN_REFERENCE] = true;
      return wrapped;
    }
};

I know we usually want to avoid Object.create, but as you can see everything else have other composition problems, so maybe this is the best option (it only happens once per {{#each-in}} invocation during initial render anyway, so hopefully it is fine?

cc @krisselden

[Joelkang]

what about moving the isConst into the EachInReference#toReference? I believe this is what the get helper is doing.

If we do the wrapping using Object.create doesnt Object.keys() only return

{
  [EACH_IN_REFERENCE]: true
}

since the key's of the underlying object reference aren't technically properties of the wrapped object, but of its prototype?

[chancancode]

I think we don't actually care about Object.keys for our purpose (on the reference) – if the tests passes and @krisselden gives the OK to use Object.create, I think we can just do it

[Joelkang]

Oh right this is the reference not the value. Ok, I shall update

[homu]

☔ The latest upstream changes (presumably e5e6d25) made this pull request unmergeable. Please resolve the merge conflicts.

[krisselden]

there are travis failures

[Joelkang]

@krisselden rebased and fixed the travis failure. Let me know if there's anything preventing this from being merged, especially vis-a-vis godfrey's comment above

[rwjblue]

Thank you!