[Glimmer2] Add mut helper and update readonly helper

[Joelkang]

Implemented based on comments in #13502

• readonly helper tests forthcoming
• Support for invoking closure actions with the mut helper
• {{mut}} of {{readonly}} support
• {{action attrs.somethingUndefined}} returns PropertyReference as opposed to UNDEFINED_REFERENCE, causing a call-time error rather than a create time error

[chadhietala]

Is this because the mut wrapping isn't happening?

[Joelkang]

you're right; updated

[homu]

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

[Joelkang]

@krisselden @chancancode so this is still failing in glimmer.

To make it backwards compatible for curly components, what ember-htmlbars seems to be doing is disconnecting the readonly if it is updated from a child, such that get(child, 'parentAttr') is no longer the same as child.attr.parentAttr. But when a change comes from the upstream property, the value correctly updates.

I've tried a variety of ways of disconnecting the value from the reference and reconnecting it, but nothing I've tried really works .It also seems like there could be multiple places this logic can go into (should it be in process-args or mut?) Any direction here would be helpful.

[chadhietala]

Doesn't the value need to be reflected on both sides?

[Joelkang]

So I added tests to verify the value of component.attrs.myAttr.value and component.get('myAttr')and they all seem to be passing after the update() without any code changes. Is that what you meant?

[zackthehuman]

Would be nice to put this forking into one of the helper files so that the test can just import { INVOKE } from '../../utils/helpers'; and it would get either the HTMLBars or Glimmer symbol.

[homu]

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

[rwjblue]

This is only used for the assertion, but as written it will not get stripped. Can you refactor to avoid splitting in prod builds where the assertion is not needed? Something like:

let value = get(this, key);
runInDebug(function() {
  let name = this._debugContainerKey.split(':')[1];
  assert(strip`
    Cannot set the \`${key}\` property (on component ${name}) to
    \`${value}\`. The \`${key}\` property came from an immutable
    binding in the template, such as {{${name} ${key}="string"}}
    or {{${name} ${key}=(if theTruth "truth" "false")}}.
  `, reference[UPDATE]);
});

reference[UPDATE](value);

[rwjblue]

This is looking really good. I am no expert in the ember-glimmer techniques (I'll leave that to the rest of the team) but I am curious what you see as the roadmap for this PR. What is still outstanding, what is blocking, etc...

One thing I was noticing/thinking about was the auto-mut AST transform, should that be reenabled in this PR?

[Joelkang]

Rebased and addressed your concerns, @rwjblue . As far as a I can tell there's nothing really blocking this PR, once these issues are resolved

• This line
• Meta tag for MUT symbol? <-- @krisselden
• Re-enable AST transform to make everything mut? <-- @chancancode

[rwjblue]

Can you change this to a TODO:? I discussed with @chancancode and this seems okish for now, and we can circle back to it later...

[rwjblue]

I was chatting with @chancancode about this abit today, he is going to try to give it more review sometime next week...

[homu]

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

[homu]

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