Infinite revalidation bug error thrown in willDestroyElement calls if we set any parent properties inside it in Ember versions higher than 5.5.0

[tamil-arasu849]

We recently upgraded our ember application from version 3.28 to v5.9.0. After upgrade some cleanup operations using willDestroyElement calls in ember component throws infinite revalidation bug error, but when we use willDestroy method or registerDestructor function, this issue doesn't occur.

We found that this issue is present in ember versions starting from v5.6.0. But upto v5.5.0 this issue isn't there.
I've tried to reproduce the bug in the repo https://github.com/tamil-arasu849/ember-lifecycle-hook-bug-repo.

The app will throw the below error when toggling the child component. But downgrading the version to v5.5.0 or lower fixes this issue

This bug breaks many of our existing components

[NullVoxPopuli]

Thanks for the reproduction!

after looking through your code, it seems like you don't want destruction to auto-track.
A work-around (we'll need to defer to @chancancode to know if this is a bug or not),
is to change:
https://github.com/tamil-arasu849/ember-lifecycle-hook-bug-repo/blob/main/app/components/example.js#L10

@action
toggleChild() {
  setProperties(this, { showChild: !this.showChild });
}

to

@action
async toggleChild() {
  await 0
  setProperties(this, { showChild: !this.showChild });
}

[tamil-arasu849]

But this change breaks many of our existing components, so had to downgrade to v5.5.0. Also we can't do this handling in all places right!

[NullVoxPopuli]

Discussed in discord, an alternative is to switch to willDestroy, from willlDestroyElement, which will also help in migration to Glimmer components (since glimmer components don't have willDestroyElement)

[tamil-arasu849]

Won't it cause any issue if we use willDestroy in classic components? @NullVoxPopuli @chancancode

[NullVoxPopuli]

why would it? -- it exists in classic components
https://api.emberjs.com/ember/5.9/classes/Component/methods/willDestroy?anchor=willDestroy

[kategengler]

I believe switching to willDestroy would only be an issue where you need access to the element in willDestroyElement.

[tamil-arasu849]

Do we expicitly set the element of a classic component to null | undefined after willDestroyElement or will it be garbage collected when component instance is cleared?

[NullVoxPopuli]

it'll be cleaned up, all handled by the framework <3

[tamil-arasu849]

Yeah. I'm asking can we able to access the HTML element in the willDestroy hook or will be set as null | undefined by the framework after the willDestroyElement hook call

[NullVoxPopuli]

I don't recall, that'd be a good thing to try -- what is this.element

[kategengler]

I don't believe the element is available in willDestroy -- otherwise we would not have needed willDestroyElement as a hook.

[NullVoxPopuli]

Thank you for the reproduction, I've put it all in a gjs file over here: https://github.com/NullVoxPopuli/repro-destruction-tracking

import Route from 'ember-route-template';
import { Labelled } from './components/labelled';
import { tracked } from '@glimmer/tracking';
import { on } from '@ember/modifier';

import EmberComponent from '@ember/component';
import Component from '@glimmer/component';

class Child extends EmberComponent {
  willDestroyElement() {
    super.willDestroyElement();
    this.incrementWillDestroy();
  }

  <template>
    <Labelled @name="Child">
      <p>Child Rendered</p>
    </Labelled>
  </template>
}

class Example extends EmberComponent {
  @tracked showChild = true;

  toggleChild = () => {
    this.showChild = !this.showChild;
  };

  <template>
    <Labelled @name="Example">
      <p>willDestroyCalls: {{this.willDestroyCalls}}</p>
      <button {{on "click" this.toggleChild}}>Toggle child</button>

      {{#if this.showChild}}
        <Child @incrementWillDestroy={{this.incrementWillDestroy}} />
      {{/if}}
    </Labelled>
  </template>
}

// Repro from
// https://github.com/tamil-arasu849/ember-lifecycle-hook-bug-repo/blob/main/app/templates/application.hbs
class Demo extends Component {
  @tracked willDestroyCalls = 0;

  increment = () => this.willDestroyCalls++;

  <template>
    <Labelled @name="Demo">
      <Example
        @incrementWillDestroy={{this.increment}}
        @willDestroyCalls={{this.willDestroyCalls}}
      />
    </Labelled>
  </template>
}
export default Route(Demo);

[NullVoxPopuli]

What's interesting is that if I remove the middle component, both 5.5 and 5.6+ error

import Route from 'ember-route-template';
import { Labelled } from './components/labelled';
import { tracked } from '@glimmer/tracking';
import { on } from '@ember/modifier';

import EmberComponent from '@ember/component';
import Component from '@glimmer/component';

class Child extends EmberComponent {
  willDestroyElement() {
    super.willDestroyElement();
    this.incrementWillDestroy();
  }

  <template>
    <Labelled @name="Child">
      <p>Child Rendered</p>
    </Labelled>
  </template>
}

// Repro from
// https://github.com/tamil-arasu849/ember-lifecycle-hook-bug-repo/blob/main/app/templates/application.hbs
class Demo extends Component {
  @tracked willDestroyCalls = 0;

  increment = () => this.willDestroyCalls++;

  @tracked showChild = true;

  toggleChild = () => {
    this.showChild = !this.showChild;
  };

  <template>
    <Labelled @name="Demo">
      <p>willDestroyCalls: {{this.willDestroyCalls}}</p>
      <button {{on "click" this.toggleChild}}>Toggle child</button>

      {{#if this.showChild}}
        <Child @incrementWillDestroy={{this.increment}} />
      {{/if}}
    </Labelled>
  </template>
}
export default Route(Demo);

So something about that middle Example component is protecting 5.5 (or broke in 5.6+)

[NullVoxPopuli]

Good news -- switching to @glimmer/component, and using registerDestructor via:

  constructor(...args) {
    super(...args);

    registerDestructor(this, () => {
      this.args.incrementWillDestroy();
    });
  }

is a good way to migrate away from this problem.

[tamil-arasu849]

Yeah. We used registerDestructor in places where we faced the computation issue and its working fine for us

[kategengler]

@NullVoxPopuli Does registerDestructor work with @ember/component?

[NullVoxPopuli]

It does, ya. And is async, so it would avoid the problem altogether

(Sync destruction is the problem here, so i don't think we have a bug)

[kategengler]

It is a bug in that code that worked in 5.4 doesn't work in >= 5.5

[NullVoxPopuli]

I think the bug is that it ever worked at all