Ember.js: "A property ... was modified inside the didInsertElement hook" deprecation warnings (v1.12.0 -> v1.13.0)

[gerry3]

After upgrading from 1.12.0 to 1.13.0 (and Ember Data 1.0.0-beta.19.1) I am seeing a lot of these warnings:

DEPRECATION: A property of <Ember.OutletView:ember1021> was modified inside the didInsertElement hook. You should never change properties on components, services or models during didInsertElement because it causes significant performance degradation.

However, I have very few didInsertElement hooks, they only read properties and set up jQuery stuff, and I see this message even when none of them are being called.

What else might cause this? Also, why isn't this deprecation in the guide?

[gerry3]

It looks like this is caused by didRender in my component based on alexspeller/ember-cli-active-link-wrapper#4 since ActiveLinkComponent sets childViews.

cc @rwjblue

[raytiley]

I have essentially the same code. I've queued the setting of the childViews in the afterRender hook. Technically this is probably causing two renders... but its the easiest approach to get rid of the deprecations.

import Ember from 'ember';

function filterAttrMorphs(node) {
  return node.attrName === undefined;
}

export default Ember.Component.extend({
  tagName: 'li',
  classNameBindings: ['active'],
  active: function() {
    return this.get('childViews').anyBy('active');
  }.property('[email protected]'),

  didRender: function() { 
    Ember.run.scheduleOnce('afterRender', this, 'calcChildViews');
  },

  calcChildViews: function() {
    var childNodes = this._renderNode.childNodes.filter(filterAttrMorphs)[0].childNodes;
    if (childNodes && Ember.isArray(childNodes)) {
      var childViews = childNodes.map(function(node) {
        return node.emberView;
      });
      this.set('childViews', Ember.A(childViews));
    }
  }
});

This problem seems like its screaming for some new UI. The looking internally to find the active chidren seems like a mega hack. What would really make this better is if we could easily get state from the router in a component. I know @tomdale had a PR or an RFC to make the router a service. With that in place a a is-active helper could easily be written and could be used as such.

<li class="{{if (is-active 'route-name') 'active'}}">
  {{link-to 'YEAH' 'route-name'}}
</li>

@wycats does this count as an addon relying on a private api that needs to be made public?

[mixonic]

I think https://github.com/alexspeller/ember-cli-active-link-wrapper can be updated to schedule the setting of active into the actions queue, and to use didRender instead of a property. active-link doesn't get passed the route you want to check for activity, so the routing service won't help it.

@raytiley I'm not crazy about the wording or messaging of this deprecation, for sure. Basically I think if you need to set a value during render, you should schedule the set into actions. Yes, that means there is a second render (exactly what the deprecation message is telling you). However, if you do this same thing in multiple places, at least it only triggers _one_ rerender.

So I think the scheduling solution here is a reasonable one. There are times when you will need a rerender:

• Something like {{active-link that wants to read its children for state. In the case of active-link, this is a design decision, and it is one that will cause a perf hit. Requiring that the path be passed twice (once to the link and once to the active checker) is obviously clumsy but would not need a rerender. Maybe active-link could yield the passed path as a block param so it doesn't need to be typed twice.
• Forcing layout calculations to use in further positioning.

I do strongly suggest that you couple to the DOM and _not_ to _renderNode, or even childViews at all. For example, in both this and @alexspeller's addon, you could simply check the DOM for an a.active with if (this.$('a.active').length) and avoid coupling to internals (complex, fragile internals that are probably slower to work with than the DOM query anyway. Well maybe).

Lastly, yeah I'd love to see a router API. I've not gotten to look at the RFC though.

Also, why isn't this deprecation in the guide?

There is no possible good excuse for skipping on a guide entry. It simply has not been written. It obviously should be written.

[raytiley]

@mixonic I've gone down the road using jQuery to look for the active class. It doesn't work. It will be correct on initial render... but if the route changes there is nothing to trigger your wrapping component to re-execute the dom query.

[mixonic]

@raytiley Ah right. We need the hook into routing to trigger the DOM check. Let us push that RFC forward.

[raytiley]

Not sure of an RFC yet... but here is @tomdale PR https://github.com/emberjs/ember.js/pull/10596

[sly7-7]

@mixonic If I understand well, in the case of Forcing layout calculations to use in further positioning. there is no other way than scheduling the calculations in the afterRender queue if I want to get rid of the deprecation warning ?

[rwjblue]

@sly7-7 - Yes, I believe that is correct.

[sly7-7]

Ok, thanks @rwjblue. I thought I was wrong to have to manipulate the run loop for such cases (I have many places in my app, having to compute the width of a parent component to use it in children (using D3.js).

[rwjblue]

Hehe, I'm not saying it is a _good_ solution, just that it is the only way today. :smile_cat:

[sly7-7]

Ok, great :) I'm on the edge :smiley_cat:

[olivierlesnicki]

The original issue is partially linked to https://github.com/emberjs/ember.js/issues/11505
Where te deprecation mentions "didInsertElement" instead of "didRender"

[ballPointPenguin]

I just happened across this after searching around for a while.
@raytiley your example with .sceduleOnce helped me out a lot. Thanks :hamster:

[acorncom]

Is there anything else we need to do on this issue or should we close it?

[danconnell]

@acorncom Doesn't it still need to be fixed? This error still pops up inside a didRender as far as I know.

[acorncom]

@danconnell Do you know if it's happening on 2.x versions of Ember or is this a 1.12 / 1.13 bug? If it's a 1.12 / 1.13 bug, not sure it'll end up getting fixed ...

[rwjblue]

The deprecation message is basically incorrect (it says OutletView most of the time, but that isn't what triggered the deprecation), I submitted https://github.com/emberjs/ember.js/pull/13353 to address the poor messages...

[danconnell]

It was happening for me on 2.3 at least. I haven't checked on 2.4. Once the fix gets into master I can check again.

[talhaobject90]

i have this warning on 2.4 , no idea where it comes from , i tried computed property and observers, but the warning still persist.

[krisselden]

The deprecation always says didInsertElement but what is deprecated is a change state during render/update that has already been reflected to DOM in that pass. You can work around it by scheduling another pass but you have to be careful that your multiple pass renders will actually settle and are guarded from looping

[krisselden]

The idea is this is a serious performance issue and is often an accident, there are a few cases it is needed but scheduling another turn and being more deliberate and aware of it seems safer

[burritoIand]

hello, Im getting this error but I havent been able to figure out what is causing it... I tried updating all packages and removing all calls to didInsertElement but it persists... Is there any way to figure out what's triggering it? (I tried ember-cli-deprecation-workflow and it didnt reveal anything)

[Serabe]

Marking this issue as stale due to inactivity. If someone has a reproduction with a current version of Ember, I'll be happy to reopen it.

[wycats]

@Serabe it's also extremely unlikely that the root cause of this issue persists, since the rendering engine was rewritten entirely since this issue, and we have addressed "backflow re-rendering" more directly since then.