Skip to content

Deprecate willDestroy() and isDestroy(ing|ed)#212

Merged
chriskrycho merged 1 commit intomasterfrom
deprecate-destroy
Mar 21, 2022
Merged

Deprecate willDestroy() and isDestroy(ing|ed)#212
chriskrycho merged 1 commit intomasterfrom
deprecate-destroy

Conversation

@chriskrycho
Copy link
Copy Markdown
Contributor

With the availability of the @ember/destroyables API, we no longer need these to be handled via lifecycle hooks. Users previously set up their own willDestroy hook, and it got called for them:

import Modifier from 'ember-modifier';

class MyModifier extends Modifier {
  willDestroy() {
    // ...
  }
}

Now, like with other classes in modern Ember/Glimmer, this can stop being a "lifecycle hook" and simply use the registerDestructor API:

import Modifier from 'ember-modifier';
import { registerDestructor } from '@ember/destroyable';

class MyModifier extends Modifier {
  constructor(owner, args) {
    super(owner, args);
    registerDestructor(this, () => {
      // ...
    });
  }
}

Similarly, instead of checking the .isDestroyed or .isDestroying properties on the instance, users can import the correspondingly-named functions from @ember/destroyables, e.g. isDestroyed(instance).

With the availability of the `@ember/destroyables` API, we no longer
need these to be handled via lifecycle hooks. Users previously set up
their own `willDestroy` hook, and it got called for them:

    import Modifier from 'ember-modifier';

    class MyModifier extends Modifier {
      willDestroy() {
        // ...
      }
    }

Now, like with other classes in modern Ember/Glimmer, this can stop
being a "lifecycle hook" and simply use the `registerDestructor` API:

    import Modifier from 'ember-modifier';
    import { registerDestructor } from '@ember/destroyable';

    class MyModifier extends Modifier {
      constructor(owner, args) {
        super(owner, args);
        registerDestructor(this, () => {
          // ...
        });
      }
    }

Similarly, instead of checking the `.isDestroyed` or `.isDestroying`
properties on the instance, users can import the correspondingly-named
functions from `@ember/destroyables`, e.g. `isDestroyed(instance)`.
@chriskrycho chriskrycho added the enhancement New feature or request label Mar 21, 2022
Copy link
Copy Markdown
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines +65 to +66
deprecateForDestroyables('isDestroying', this);
deprecateForDestroyables('isDestroyed', this);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's very very unlikely that someone overrides the isDestroying/isDestroyed part.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I assume the same, but if someone can override those things... 😅

@chriskrycho chriskrycho merged commit 2c4308d into master Mar 21, 2022
@chriskrycho chriskrycho deleted the deprecate-destroy branch March 21, 2022 20:19
@chriskrycho chriskrycho added deprecation and removed enhancement New feature or request labels Mar 21, 2022
@chriskrycho chriskrycho mentioned this pull request Mar 21, 2022
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants