Backport RFC# 435

[cibernox]

Attempt to packport #923 into 0.38

[cibernox]

I had to change {{#let}} to {{#with}} on this test because let doesn't seem to exist in this version of glimmer. What do you suggest we do here? Leave with in this branch alone or come up with a single testing approach that works in both branches?

[chancancode]

with is totally fine 👍

[cibernox]

Apparently CI doesn't run in PRs to this branch but tests are green locally

[chancancode]

Let’s find out! 😬

[cibernox]

@chancancode can you release a new version? Alternatively, how can we install a lerna-style monorepo directly from github?

[cibernox]

@chancancode I found out that, very conveniently, there is a yarn:link script in the glimmer repo and a link:glimmer in the ember one to link everything, so I ran Ember's test with this branch merged at it's all green.

On one side it's good news, on the other i'm surprised/worried that the changes in the ordering didn't break anything.

[chancancode]

@cibernox yes! normally the yarn:link script is the way to go, but I also published the v0.38.2-alpha.1 to unblock your ember PR.

The reason I went with -alpha.1 is that it's not super legit to backport a backing change like this (as you mentioned, the ordering changed). The "proper" thing to do is to release 0.40 from master (which we should do anyway), but on the other hand 1) it's < 1.0.0 anyway so technically, anything goes, 2) probably no one outside of ember and glimmer.js uses this so we can be pragmatic about it. I have some hopes that we can upgrade ember to v0.39/40 during this ember release cycle, in which case, we can revert this on release-0-38 and pretend it never happens.

I think this being an alpha build wouldn't really affect you, but if it does let me know.

Also, I realized my mistake. I thought the queueing of modifiers is Ember's responsibility (it used to be) but it seems like it has moved into Glimmer, which is probably why the tests are only in Glimmer (I think we should add them to Ember anyway). That also means we should probably try to fix the ordering in Glimmer itself, which may make the whole breaking change problem moot.

I'll try to come up with an idea to fix the issue on Glimmer's side.

Anyway, in the meantime, you can start working on the Ember PR with the alpha build. We need to add a feature flag, and essentially re-implement the error you used to get in an AST transform on Ember's side, when the feature flag is off. That, and add a bunch of tests in Ember (basically what you added here, and the missing ordering tests).

Thank you again!