Full re-render on any tracked property change

[code0100fun]

I've noticed that if I change any tracked property all the tracked properties seem to be updated. This only happens with @glimmer/application >= 0.7.0.

This demo expresses the problem by rendering an audio element.

When getMetaData or onCanPlay gets called then setting this.duration = this.audio.duration; or this.canPlay = true; causes the fileUrl on the audio element to also be changed, which triggers getMetaData and onCanPlay again.

Reverting to @glimmer/application 0.6.0 fixes the issue. Also just making the audio elements src static mitigates the issue (not really a solution though).

[ablanco]

I've also encountered this issue (I'm on 0.7.2).

When I modify an input field and that updates a @tracked property in a component, all functions decorated as @tracked fire up, but they don't depend at all on the modified property. They fire up even in different components!

[nmcclay]

I can confirm I'm also seeing this issue. Reverting @glimmer/application' to0.6.0` as @code0100fun mentioned seems to fix it and tracked properties update as you would expect.

[wycats]

Yikes, this seems bad!

@tomdale do you have any ideas what might have caused it?

[rwjblue]

We should probably try to get a beta out of the code on master so we can test and see if the issue is fixed already (but unreleased)...

[tomdale]

Yikes, very bad. Let me look into it…

[pittst3r]

Ugh, I wonder if this is related to my poor VM upgrade attempt. Probably. Sorry. :(

There is a chance this has been fixed though and just unreleased like @rwjblue said (https://github.com/glimmerjs/glimmer-component/pull/67).

[tomdale]

@code0100fun First, thank you so much for the reproduction, it made looking into this easier than it should have been. 😛

You have uncovered a very bad bug that frankly I'm surprised we didn't notice earlier. It looks like it actually has nothing to do with tracked properties or their invalidation at all.

Stepping through the reproduction, I verified that setting duration or canPlay tracked properties invalidated only those properties. Next, I set a breakpoint inside the tracked property getter so I could take a peek at the values when they were being consumed inside the main updating VM loop.

We vend CachedReferences for tracked properties, and I verified that the cache here was working correctly. Stepping through the reference for the fileUrl, it returned the cached result because its tag had not been invalidated.

To my horror, I realized that we eagerly re-set every dynamic attribute even when the value has not changed. I believe we didn't notice it until now because attribute sets tend to be idempotent—except, of course, in cases like setting an <audio> element's src.

This seems like an oversight, because all UpdatingOpcodes have a public tag property that could (and should) be validated before evaluating the opcode. I think the fix should be embarrassingly simple.

Thank you to everyone who raised this issue and provided very actionable reproduction cases. We will get a fix out for this ASAP!

[t-sauer]

I am still seeing this problem (or at least I assume that it is the same problem) with 0.8.0-alpha.9.

I upgraded Glimmeroids which uses a canvas. The width and height attributes of the canvas get set in the template (https://github.com/t-sauer/Glimmeroids/blob/59eb5bb2cb0ab2d5dd00007a02b1ace407ca0d07/src/ui/components/Glimmeroids/template.hbs#L31).
The values for that come from a getter function(canvasSize) which tracks the state property of the component (the screen size is stored in there, https://github.com/t-sauer/Glimmeroids/blob/59eb5bb2cb0ab2d5dd00007a02b1ace407ca0d07/src/ui/components/Glimmeroids/component.ts#L96). Now when pressing an arrow key, the state property gets set to a new value, which I assume invalidates canvasSize and sets the width and height on the canvas element again (which leads to visible flashing when playing the game).

I guess in this case it can easily be solved in the app by moving the screen size out of the state property but it might be problematic in other cases.

[tomdale]

@t-sauer I think this is a slightly different issue, but thanks for catching it!

In your case, I'd argue that it's probably not ideal to model it this way. Every time state changes, it will have no choice but to invalidate and recompute the value of canvasSize in case it has changed. The fix in glimmerjs/glimmer-vm#694 doesn't help here because the underlying reference has indeed been invalidated.

However, we may want to have a "last chance" check to avoid DOM work that would kick in in your case. The downside is that it's a little bit more work and memory use for every attribute on the page, so we should measure the impact to make sure. It's probably worth it if it's a case real world apps are likely to commonly hit.

[tomdale]

Looked into this a little more and it seems like we are doing the "last chance diff" for dynamic text content: https://github.com/glimmerjs/glimmer-vm/blob/65ed9cd/packages/@glimmer/runtime/lib/vm/content/node.ts#L14

I don't have a good explanation for why we would do it for text but not attributes. I can't come up with any reasons why attributes would be more or less memory intensive than text. We should probably just do it.