[Glimmer 2] "Backtracking re-render" is now an assertion

[chancancode]

Backtracking re-render refers to a scenario where, in the middle of the rendering process, you have modified something that has already been rendered.

For example:

{{foo}} {{foo-bar parent=this}}

// app/components/foo-bar.js

export default Ember.Component.extend({
  init() {
    this._super(...arguments);
    this.get('parent').set('foo', 'bar');
  }  
});

As you can see, by the time the foo-bar component is instantiated and rendered, we have already used the value foo from the parent context to populate the {{foo}} curly. However, in its constructor, it was trying to modify the same value, hence. "backtracking".

This is a pretty extreme example, but it illustrates the problem. Besides init, didInitAttrs, didReceiveAttrs, willInsertElement and willRender also happens synchronously during the rendering process. Additionally, backtracking is often an issue arising from the behavior of two-way bound properties.

This behavior has always been unreliable, but was partially supported with a deprecation (since Ember 1.13):

You modified ApplicationController.foo twice in a single render. This was unreliable in Ember 1.x and will be removed in Ember 3.0

Since 1.13, Ember supported this by immediately doing a second re-render when backtracking was detected (and then repeating until the system stabilizes). This strategy itself could be a source of performance problems. In extreme cases, this could cause an infinite loop.

In Glimmer 2, while the extra re-render is relatively cheap, the extra book-keeping to detect a backtracking set is not. One of the wins of the Glimmer 2 system is that it does not need to eagerly setup observers to track changes. Further, certain optimizations in Glimmer 2 allows the system to skip traversing subtrees when it knows nothing within it has changed.

Together, these factors mean that we can not readily detect these backtracking sets (or whether something was "already rendered" or not) without doing a large amount of extra book-keeping and intentionally defeating these optimizations.

We already wrote the code to support this, but due to the already unreliable nature of the feature, and the (very significant) book-keeping costs, we are hesitant to automatically enable them for everyone without knowing whether it is still needed.

As a compromise, we currently only perform the detection in development mode and turned the deprecation message into a development-mode assertion (hard error). In production mode, the detection code is stripped and backtracking will not work.

We have kept the facility to support this feature (without the assertion) in the codebase behind a second feature flag. The code is being tested continuously on CI, however, is disabled by default until we have enough usage information to determine next steps.

If you believe you have usage patterns that are affected by this, please provide as much detail about your scenario as possible below. It is very possible that there can be alternatives and/or targeted solutions we can use that do not require a wholesale change to the engine. Therefore, it would be helpful if you provide background information and context about your usage instead of just showing us small code snippets from your codebase.

[joukevandermaas]

FYI, we had some of these warnings in our app. Specifically, we updated some properties of a service in the init of a component, which would trigger something else on the page to render differently.

It is very simple to fix this warning by scheduling the property change on the next run loop. It took me ~an hour to track down and fix all of the warnings in our (fairly large) app. Although this is technically a breaking change, I agree with your assessment even if it caused me some extra work.

[chancancode]

@fivetanley improving the error message here sounds good. I know @krisselden and @stefanpenner has a workflow for tracking down these issues, maybe they can help give you some directions on this.

[krisselden]

@joukevandermaas run.next() is not a great fix for this error, though I understand if you are overwhelmed with these errors why you would go there. It is best to try to understand why the back flow of data is invalidating things that are already rendered.

Likely if you set props on a service that could be injected onto any component, it increases the chances of that set invalidating something that was already rendered. In general, the pattern should be that set() is only used on internal state during rendering hooks, not tied to input or services and or set() is used on an event, input state should be settled by the time stuff renders.

[stefanpenner]

@joukevandermaas run.next() is not a great fix for this error,

doing so cause performance issues, as glimmer2 in this case is informing "duplicate work is happening, you really don't want this if you want a performant app". Where previously, ember would absorb this, but result in heft performance penalty.

We have some more knowledge sharing work todo here... Ultimately we believe this is a healthy path forward for apps. But we need to make sure everyone has the tools + knowledge available to benefit :)

[Dhaulagiri]

As a person who follows Ember goings on relatively closely (twitter, here on github, mailing lists, etc) this issue snuck on on me so I would suspect that this might catch others by surprise if it lands as part of Ember 2.10, particularly because the deprecation warning associated with it specifically states the behavior will be supported until 3.0. I don't believe I've seen it socialized anywhere that this behavior will not work in Glimmer 2 (although I may have simply missed it).

[stefanpenner]

suspect that this might catch others by surprise if it lands as part of Ember 2.10, particularly because the deprecation warning associated with it specifically states the behavior will be supported until 3.0. I don't believe I've seen it socialized anywhere that this behavior will not work in Glimmer 2 (although I may have simply missed it).

yup, we need to improve some messaging / details here.

[Dhaulagiri]

I see this made it into the 2.10 release. Will this be mentioned in the 2.10 release blog post?

[bryanhickerson]

This was not mentioned in the 2.10 blog post and took me by surprise since the deprecation warning previously said it would be supported until 3.0, as was mentioned above.

[revanar]

I have a usage pattern that's affected by this. I'm sure the problem must be my usage pattern, and not this particular change, but I would love some input on what a good alternate usage pattern would be!

Basically, I have a page that shows a filterable set of data, and to achieve this, I'm using an Ember computed value to filter the data based on the value of several query-params on the page. However, to prevent invalid inputs (eg not letters or numbers) from being added to query-params from user input, I have the following pattern:

 filteredModel: Ember.computed('model', /*list of individual query params*/, function(){
    let model = this.get('model').filterBy('pdf.pdf_path.url'); //removes all records that don't have a pdf uploaded
    this.get('queryParams').forEach((filter)=> { // for each possible filter
      if ((this.get(filter).length > 0)) { //if the filter has content...
        //guardian pattern to prevent invalid inputs
        let valid = new RegExp('^[A-Za-z0-9 _]*[A-Za-z0-9][A-Za-z0-9 _]*$');
        while (this.get(filter).length > 0 && !valid.test(this.get(filter))){
          this.set(filter, this.get(filter).slice(0,-1));
        }
        //block of code where the model gets filtered
        //...
        //...
    });
    return model;
  }),

So basically, when I computed what the filtered model should look like, if any of the filter values have invalid characters in them, I strip off the last character until they become valid. Does anyone have a suggestion of a cleaner way to do validity-checking on these inputs?

[ykaragol]

From the initial post, some hooks were mentioned:

This is a pretty extreme example, but it illustrates the problem. Besides init, didInitAttrs, didReceiveAttrs, willInsertElement and willRender also happens synchronously during the rendering process. Additionally, backtracking is often an issue arising from the behavior of two-way bound properties.

Why are didInsertElement and didRender hooks working like a charm but the other hooks fail with twice render error?

[pixelhandler]

@BenjaminHorn @Blackening999 @Dhaulagiri @DingoEatingFuzz @Gaurav0 @GavinJoyce @Redsandro @TRMW @Turbo87 @aklkv @alidcastano @backspace @bdiz @bgentry @bjornharrtell @bryanhickerson @buschtoens @caseklim @cbou @chancancode @danaoira @daniel-de-wit @fivetanley @fotinakis @gabrielgrant @ghost @jakesjews @janmisek @joukevandermaas is this still an issue, perhaps we should close, what do you think?

[bryanhickerson]

I fixed any instances of the error in my app.

[backspace]

Anytime I’ve encountered this I’ve been able to rearrange things so that it wouldn’t happen anymore, so I think it’s okay to close.

[Gaurav0]

Yes, please close

[chancancode]

end of an era 😬

[amk221]

Sorry to bring up an old issue.

Say for example I am rendering

{{this.myBool}}

and the error is Error: Assertion Failed: You modified "myBool" twice

I can fix the error by changing it to:

{{if this.myBool true false}}

Similarly, if a class name binding is causing the problem

I can change:

classNameBindings: ['myBool']

to

classNameBindings: ['myBool:yes:no']

If I can silence the warning like this, why can't Ember handle it for me?

Here is the demo code I was using:

https://ember-twiddle.com/db7f6e382bd0b1de91447881eebb62a5?openFiles=templates.components.my-component.hbs%2C

[chancancode]

None of those things “fixes” the problem. It’s either that you switched to production mode, or there are bugs in the assertion/detection code. In any case, you need to fix the side that assigns/sets the value, not the side that consumes it.

[amk221]

Ok. Thank you. I understand the backtracking problem outlined in this original post because it has an explicit set

But in my demo twiddle, there is no set as far as I can see.

Edit That came out wrong, I mean to say there is no set coming from a subsequent part of the template like there is in the example

[chancancode]

Hm, I'm not getting the error in the twiddle, what is the sequence of clicks I should do?

[amk221]

Click open, and then click close. But the close button of the yielded child component, not the parent.

[chancancode]

I see, the problem is that during teardown, focusOut is called on the component which calls set on an already rendered property. I'm not 100% sure how the component lose focus and the timing semantics. I'm pretty sure that the variations you tried just confuses the tracking system and mask the underlying issue though. Let's track this in a new issue? I'm not sure if it's a valid issue, but let's do the investigation there.

[Venkatesan-Deivaisigamani-E3811]

@chancancode

We are trying to upgrade ember in our app from 2.8 to 2.12 and facing this error throughout our fairly large application (100+ routes and 450+ components). We tried to fix this issue, wherever it is captured, but still there are some uncovered areas where this "modified 'x' twice" might present.

My understanding is these backtracking will be detected and error is thrown only in debug builds. In production, the detection will not happen, so no errors will be thrown. Until v2.10, we were doing the backtracking rerender in prod and now that is not happening in the v2.12 prod. My question is - will there be any unexpected bugs in the way the application code works as we are not doing backtracking rerender in production?