[Glimmer2] Add test for glimmer re-renders #13772
Conversation
| @@ -1013,6 +1013,56 @@ moduleFor('Components test: curly components', class extends RenderingTest { | |||
| this.assertText('In layout - someProp: value set in instance'); | |||
| } | |||
|
|
|||
| // Note: Hooks are not re-ran for idempotent re-renders | |||
chadhietala
force-pushed
the
rerendering-attrs
branch
from
d7787b5
to
e59f5af
Compare
|
I personally believe the current behavior where they do rerun is costly and makes little sense, but I think it is problematic we keep changing the behavior of rerender and not very semver like. I would be trivial to make htmlbars match this, I already have to do it manually in |
|
We should likely consider making this behavior (the specific semantics around ordering and if hooks will be ran when args do not change) "locked down" and guaranteed as part of SemVer, and I agree that changing the semantics repeatedly seems trolly, but I do not think it is a SemVer violation to switch to the glimmer behavior. The current behavior is (IMO) bad, and the glimmer behavior (documented by this test) seems much better. FWIW, I believe this is also tested by the lifecycle tests (which has branched assertions based on engine). |
|
I think that, for now, saying that we promise to run the hooks if any of the declared inputs change, and MIGHT run hooks otherwise is correct. We should expressly say in documentation that you should NOT rely upon hooks that run in the absence of a change to the inputs, and that we expect further optimizations to reduce the number of such cases over time. |
|
Is there any consensus as to if this should be merged or not? |
Part of #13644