From c34ac7a5b2099481c0836d1396e3752e1c3a3a39 Mon Sep 17 00:00:00 2001
From: Chris Garrett <cgarrett@linkedin.com>
Date: Fri, 10 Jan 2020 13:12:56 -0800
Subject: [PATCH 01/11] add destroyables

---
 text/0000-destroyables.md | 163 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 163 insertions(+)
 create mode 100644 text/0000-destroyables.md

diff --git a/text/0000-destroyables.md b/text/0000-destroyables.md
new file mode 100644
index 0000000000..6993be6e85
--- /dev/null
+++ b/text/0000-destroyables.md
@@ -0,0 +1,163 @@
+- Start Date: 2020-01-10
+- Relevant Team(s): Ember.js
+- RFC PR: (after opening the RFC PR, update this with a link to it and update the file name)
+- Tracking: (leave this empty)
+
+# Destroyables
+
+## Summary
+
+Adds an API for registering destroyables and destructors with Ember's built in
+destruction hierarchy.
+
+```js
+class MyComponent extends Component {
+  constructor() {
+    let timeoutId = setTimeout(() => console.log('hello'), 1000);
+
+    registerDestructor(this, () => clearTimeout(timeoutId));
+  }
+}
+```
+
+The API will also enable users to create and manage their own destroyables, and
+associate them with a parent destroyable.
+
+```js
+class TimeoutManager {
+  constructor(parent, fn, timeout = 1000) {
+    let timeoutId = setTimeout(fn, timeout);
+
+    registerLifetime(parent, this);
+    registerDestructor(this, () => clearTimeout(timeoutId))
+  }
+}
+
+class MyComponent extends Component {
+  manager = new TimeoutManager(this, () => console.log('hello'));
+}
+```
+
+## Motivation
+
+Ember manages the lifecycles and lifetimes of many built in constructs, such as
+components, and does so in a hierarchical way - when a parent component is
+destroyed, all of its children are destroyed as well. This is a well established
+software pattern that is useful for many applications, and there are a variety
+of libraries, such as `ember-lifeline` and `ember-concurrency`, that would
+benefit from having a way to extend this hierarchy, adding their own "children"
+that are cleaned up whenever their parents are removed.
+
+Historically, Ember has exposed this cleanup lifecycle via _hooks_, such as the
+`willDestroy` hook on components. However, methods like these have a number of
+downsides:
+
+1. Since they are named, they can have collisions with other properties. This is
+   historically what led to the actions hash on classic components, in order to
+   avoid collisions between actions named `destroy` and the `destroy` lifecyle
+   hook.
+2. On a related note, relying on property names means that all framework classes
+   _must_ implement the `willDestroy` function (or another name), making it very
+   difficult to change APIs in the future.
+3. Methods are difficult for _libraries_ to instrument. For instance,
+   `ember-concurrency` currently replaces the `willDestroy` method on any class
+   with a task, with logic that looks similar to:
+
+   ```js
+   let PATCHED = new WeakSet();
+
+   function patchWillDestroy(obj) {
+     if (PATCHED.has(obj)) return;
+
+     let oldWillDestroy = obj.willDestroy;
+
+     obj.willDestroy = function() {
+       if (oldWillDestroy) oldWillDestroy.call(this);
+
+       teardownTasks(this);
+     }
+
+     PATCHED.add(obj);
+   }
+   ```
+
+   This logic becomes especially convoluted if _multiple_ libraries are
+   attempting to patch `willDestroy` in this way.
+
+4. Finally, since this isn't a standard, it's difficult to add _layers_ of new
+   destroyable values that can interoperate with one another. For instance,
+   there is no way for `ember-concurrency` to know how to destroy tasks on
+   non-framework classes that users may have added themselves.
+
+This RFC proposes a streamlined API that disconnects the exact implementation
+from any interface, allows for multiple destructors per-destroyable, and
+maximizes interoperability in general.
+
+## Detailed design
+
+The API consists of 3 functions:
+
+```ts
+declare function registerLifetime(parent: object, child: object): void;
+declare function registerDestructor(destroyable: object, destructor: () => void): void;
+declare function destroy(destroyable: object): void;
+```
+
+### `registerLifetime`
+
+This function is used to associate a destroyable object's lifetime with a
+parent. When the parent is destroyed, all registered children will also be
+destroyed.
+
+### `registerDestructor`
+
+Receives a destroyable and a destructor function, and associates the function
+with it. When the destroyable is destroyed, or when its parent is destroyed, the
+destructor function will be called. Multiple destructors can be associated with
+a given destroyable, and they can be associated over time, allowing libraries
+like `ember-lifeline` to dynamically add destructors as needed.
+
+### `destroy`
+
+`destroy` manually initiates the destruction of a destroyable. This allows users
+to manually manage the lifecycles of destroyables that are shorter lived than
+their parents (for instance, destroyables that are defined an a service, which
+lives as long as an application does), cleaning them up as needed.
+
+### Built In Destroyables
+
+The root destroyable of an Ember application will be the instance of the
+application itself. All framework managed classes are destroyables, including:
+
+* Components
+* Services
+* Routes
+* Controllers
+* Helpers
+* Modifiers
+
+Any future classes that are added and have a container managed lifecycle should
+also be marked as destroyables.
+
+## How we teach this
+
+This is a generally low level API, meant for usage in the framework and in
+addons. It should be taught primarily through API documentation, and potentially
+through and in-depth guide.
+
+## Drawbacks
+
+- Adds another destruction API which may conflict with the existing destruction
+  hooks. Since this is a low-level API, it shouldn't be too problematic - most
+  users will be guided toward using the standard lifecycle hooks, and this API
+  will exist for libraries like `ember-concurrency` and `ember-lifeline`.
+
+## Alternatives
+
+- Continue using existing lifecycle hooks for public API, and don't provide an
+  independent API.
+
+## Unresolved questions
+
+- Should `willDestroy` and `didDestroy` (e.g. mark-and-sweep style semantics) be
+  exposed?

From a0ef34e9f96a0437cff0ac108522f3afb2fb42ad Mon Sep 17 00:00:00 2001
From: Chris Garrett <cgarrett@linkedin.com>
Date: Fri, 10 Jan 2020 16:27:08 -0800
Subject: [PATCH 02/11] rename

---
 text/{0000-destroyables.md => 0580-destroyables.md} | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
 rename text/{0000-destroyables.md => 0580-destroyables.md} (98%)

diff --git a/text/0000-destroyables.md b/text/0580-destroyables.md
similarity index 98%
rename from text/0000-destroyables.md
rename to text/0580-destroyables.md
index 6993be6e85..2d8971e05d 100644
--- a/text/0000-destroyables.md
+++ b/text/0580-destroyables.md
@@ -1,6 +1,6 @@
 - Start Date: 2020-01-10
 - Relevant Team(s): Ember.js
-- RFC PR: (after opening the RFC PR, update this with a link to it and update the file name)
+- RFC PR: https://github.com/emberjs/rfcs/pull/580
 - Tracking: (leave this empty)
 
 # Destroyables

From 4a1d22ceb0f04498b3b27a5ef1c41e64dfbb61b0 Mon Sep 17 00:00:00 2001
From: Chris Garrett <cgarrett@linkedin.com>
Date: Thu, 27 Feb 2020 19:51:11 -0800
Subject: [PATCH 03/11] Update text/0580-destroyables.md

Co-Authored-By: Robert Jackson <me@rwjblue.com>
---
 text/0580-destroyables.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/text/0580-destroyables.md b/text/0580-destroyables.md
index 2d8971e05d..87a62057dd 100644
--- a/text/0580-destroyables.md
+++ b/text/0580-destroyables.md
@@ -44,7 +44,7 @@ Ember manages the lifecycles and lifetimes of many built in constructs, such as
 components, and does so in a hierarchical way - when a parent component is
 destroyed, all of its children are destroyed as well. This is a well established
 software pattern that is useful for many applications, and there are a variety
-of libraries, such as `ember-lifeline` and `ember-concurrency`, that would
+of libraries, such as [ember-lifeline](https://github.com/ember-lifeline/ember-lifeline) and [ember-concurrency](https://github.com/machty/ember-concurrency), that would
 benefit from having a way to extend this hierarchy, adding their own "children"
 that are cleaned up whenever their parents are removed.
 

From 09e3befd79d66d785d0fc4faf0da81feff341b89 Mon Sep 17 00:00:00 2001
From: Chris Garrett <cgarrett@linkedin.com>
Date: Thu, 27 Feb 2020 20:33:11 -0800
Subject: [PATCH 04/11] updates based on feedback

---
 text/0580-destroyables.md | 106 +++++++++++++++++++++++++++++++++-----
 1 file changed, 93 insertions(+), 13 deletions(-)

diff --git a/text/0580-destroyables.md b/text/0580-destroyables.md
index 87a62057dd..f7b2a14530 100644
--- a/text/0580-destroyables.md
+++ b/text/0580-destroyables.md
@@ -28,7 +28,7 @@ class TimeoutManager {
   constructor(parent, fn, timeout = 1000) {
     let timeoutId = setTimeout(fn, timeout);
 
-    registerLifetime(parent, this);
+    associateDestroyableChild(parent, this);
     registerDestructor(this, () => clearTimeout(timeoutId))
   }
 }
@@ -44,7 +44,8 @@ Ember manages the lifecycles and lifetimes of many built in constructs, such as
 components, and does so in a hierarchical way - when a parent component is
 destroyed, all of its children are destroyed as well. This is a well established
 software pattern that is useful for many applications, and there are a variety
-of libraries, such as [ember-lifeline](https://github.com/ember-lifeline/ember-lifeline) and [ember-concurrency](https://github.com/machty/ember-concurrency), that would
+of libraries, such as [ember-lifeline](https://github.com/ember-lifeline/ember-lifeline)
+and [ember-concurrency](https://github.com/machty/ember-concurrency), that would
 benefit from having a way to extend this hierarchy, adding their own "children"
 that are cleaned up whenever their parents are removed.
 
@@ -95,21 +96,53 @@ maximizes interoperability in general.
 
 ## Detailed design
 
-The API consists of 3 functions:
+The API consists of 6 main functions, imported from `@ember/application`:
 
 ```ts
-declare function registerLifetime(parent: object, child: object): void;
+declare function associateDestroyableChild(parent: object, child: object): void;
 declare function registerDestructor(destroyable: object, destructor: () => void): void;
+declare function unregisterDestructor(destroyable: object, destructor: () => void): void;
+
 declare function destroy(destroyable: object): void;
+declare function isDestroying(destroyable: object): boolean;
+declare function isDestroyed(destroyable: object): boolean;
+```
+
+In addition, there is a debug-only mode function used for testing:
+
+```ts
+declare function assertDestroyablesDestroyed(): void;
 ```
 
-### `registerLifetime`
+#### `associateDestroyableChild`
+
+This function is used to associate a destroyable object with a parent. When the
+parent is destroyed, all registered children will also be destroyed.
 
-This function is used to associate a destroyable object's lifetime with a
-parent. When the parent is destroyed, all registered children will also be
-destroyed.
+- Attempting to associate a parent or child that has already been destroyed will
+  throw an error.
+- Attempting to associate a child with a parent that is not a destroyable should
+  throw an error.
 
-### `registerDestructor`
+##### Multiple Inheritance
+
+Attempting to associate a child to multiple parents should currently throw an
+error. This could be changed in the future, but for the time being multiple
+inheritance of destructors is tricky and not scoped in. Instead, users can add
+destructors to accomplish this goal:
+
+```js
+let parent1 = {}, parent2 = {}, child = {};
+
+registerDestructor(parent1, () => destroy(child));
+registerDestructor(parent2, () => destroy(child));
+```
+
+The exact timing semantics here will be a bit different, but for most use cases
+this should be fine. If we find that it would be useful to have multiple
+inheritance baked in in the future, it can be added in a followup RFC.
+
+#### `registerDestructor`
 
 Receives a destroyable and a destructor function, and associates the function
 with it. When the destroyable is destroyed, or when its parent is destroyed, the
@@ -117,17 +150,64 @@ destructor function will be called. Multiple destructors can be associated with
 a given destroyable, and they can be associated over time, allowing libraries
 like `ember-lifeline` to dynamically add destructors as needed.
 
-### `destroy`
+- Registering a destructor on a destroyed object should throw an error.
+- Attempting to register the same destructor multiple times should throw an
+  error.
+- Attempting to register a destructor on a non-destroyable should throw an
+  error.
+
+#### `unregisterDestructor`
+
+Receives a destroyable and a destructor function, and de-associates the
+destructor from the destroyable.
+
+- Calling `unregisterDestructor` on a destroyed object should throw an error.
+- Calling `unregisterDestructor` with a destructor that is not associated with
+  the object should throw an error.
+- Calling `unregisterDestructor` on a non-destroyable should throw an error.
+
+#### `destroy`
 
 `destroy` manually initiates the destruction of a destroyable. This allows users
 to manually manage the lifecycles of destroyables that are shorter lived than
 their parents (for instance, destroyables that are defined an a service, which
 lives as long as an application does), cleaning them up as needed.
 
+Calling `destroy` multiple times on the same object is safe. It will not throw
+an error, and will not take any further action.
+
+- Calling `destroy` on a non-destroyable should throw an error.
+
+#### `isDestroying`
+
+This would return true as soon as the destroyable begins destruction, before any
+of its destructors are called. Returns false otherwise.
+
+- Calling `isDestroying` on a non-destroyable should throw an error.
+
+#### `isDestroyed`
+
+This would return true as soon after the destroyable has been fully destroyed,
+including all of its child destroyables. Returns false otherwise.
+
+- Calling `isDestroyed` on a non-destroyable should throw an error.
+
+#### `assertDestroyablesDestroyed`
+
+This function asserts that all active destroyables have been destroyed at the
+time it is called. It is meant to be a low level hook that testing frameworks
+like `ember-qunit` and `ember-mocha` can use to hook into and validate that all
+destroyables have in fact been destroyed.
+
+### Timing Semantics
+
+Destroyables first call their own destructors, then destroy their associated
+children.
+
 ### Built In Destroyables
 
-The root destroyable of an Ember application will be the instance of the
-application itself. All framework managed classes are destroyables, including:
+The root destroyable of an Ember application will be the instance of the owner.
+All framework managed classes are destroyables, including:
 
 * Components
 * Services
@@ -143,7 +223,7 @@ also be marked as destroyables.
 
 This is a generally low level API, meant for usage in the framework and in
 addons. It should be taught primarily through API documentation, and potentially
-through and in-depth guide.
+through an in-depth guide.
 
 ## Drawbacks
 

From de77ed6631211e8105d55ca806c834d40fe55b11 Mon Sep 17 00:00:00 2001
From: Chris Garrett <cgarrett@linkedin.com>
Date: Fri, 28 Feb 2020 13:25:02 -0800
Subject: [PATCH 05/11] incorporate feedback

---
 text/0580-destroyables.md | 71 +++++++++++++++++++--------------------
 1 file changed, 35 insertions(+), 36 deletions(-)

diff --git a/text/0580-destroyables.md b/text/0580-destroyables.md
index f7b2a14530..53b298bf49 100644
--- a/text/0580-destroyables.md
+++ b/text/0580-destroyables.md
@@ -114,15 +114,18 @@ In addition, there is a debug-only mode function used for testing:
 declare function assertDestroyablesDestroyed(): void;
 ```
 
+For the remainder of this RFC, the terms "destroyable" and "destroyable object"
+will be used to mean any object which is a valid `WeakMap` key
+(e.g. `typeof obj === 'object' || typeof obj === 'function'`). Any JS object
+that fulfills this property can be used with this system.
+
 #### `associateDestroyableChild`
 
 This function is used to associate a destroyable object with a parent. When the
 parent is destroyed, all registered children will also be destroyed.
 
-- Attempting to associate a parent or child that has already been destroyed will
-  throw an error.
-- Attempting to associate a child with a parent that is not a destroyable should
-  throw an error.
+- Attempting to associate a parent or child that has already been destroyed
+  should throw an error.
 
 ##### Multiple Inheritance
 
@@ -144,17 +147,16 @@ inheritance baked in in the future, it can be added in a followup RFC.
 
 #### `registerDestructor`
 
-Receives a destroyable and a destructor function, and associates the function
-with it. When the destroyable is destroyed, or when its parent is destroyed, the
-destructor function will be called. Multiple destructors can be associated with
-a given destroyable, and they can be associated over time, allowing libraries
-like `ember-lifeline` to dynamically add destructors as needed.
+Receives a destroyable object and a destructor function, and associates the
+function with it. When the destroyable is destroyed with `destroy`, or when its
+parent is destroyed, the destructor function will be called. Multiple
+destructors can be associated with a given destroyable, and they can be
+associated over time, allowing libraries like `ember-lifeline` to dynamically
+add destructors as needed.
 
 - Registering a destructor on a destroyed object should throw an error.
 - Attempting to register the same destructor multiple times should throw an
   error.
-- Attempting to register a destructor on a non-destroyable should throw an
-  error.
 
 #### `unregisterDestructor`
 
@@ -164,45 +166,42 @@ destructor from the destroyable.
 - Calling `unregisterDestructor` on a destroyed object should throw an error.
 - Calling `unregisterDestructor` with a destructor that is not associated with
   the object should throw an error.
-- Calling `unregisterDestructor` on a non-destroyable should throw an error.
 
 #### `destroy`
 
-`destroy` manually initiates the destruction of a destroyable. This allows users
-to manually manage the lifecycles of destroyables that are shorter lived than
-their parents (for instance, destroyables that are defined an a service, which
-lives as long as an application does), cleaning them up as needed.
+`destroy` initiates the destruction of an destroyable object. It runs all
+associated destructors, and then destroys all children recursively.
 
-Calling `destroy` multiple times on the same object is safe. It will not throw
-an error, and will not take any further action.
+Destruction via `destroy()` follows these steps:
 
-- Calling `destroy` on a non-destroyable should throw an error.
+1. Mark the destroyable such that `isDestroying(destroyable)` returns `true`
+2. Call the destroyable's destructors
+3. Call `destroy()` on each of the destroyable's associated children
+4. Mark the destroyable such that `isDestroyed(destroyable)` returns `true`
 
-#### `isDestroying`
+Calling `destroy` multiple times on the same destroyable is safe. It will not
+throw an error, and will not take any further action.
+
+Calling `destroy` with an destroyable that has no destructors or associated children
+will not throw an error, and will do nothing.
 
-This would return true as soon as the destroyable begins destruction, before any
-of its destructors are called. Returns false otherwise.
+#### `isDestroying`
 
-- Calling `isDestroying` on a non-destroyable should throw an error.
+Receives an destroyable, and returns `true` if the destroyable has begun
+destroying. Otherwise returns false.
 
 #### `isDestroyed`
 
-This would return true as soon after the destroyable has been fully destroyed,
-including all of its child destroyables. Returns false otherwise.
-
-- Calling `isDestroyed` on a non-destroyable should throw an error.
+Receives an destroyable, and returns `true` if the destroyable has finished
+destroying. Otherwise returns false.
 
 #### `assertDestroyablesDestroyed`
 
-This function asserts that all active destroyables have been destroyed at the
-time it is called. It is meant to be a low level hook that testing frameworks
-like `ember-qunit` and `ember-mocha` can use to hook into and validate that all
-destroyables have in fact been destroyed.
-
-### Timing Semantics
-
-Destroyables first call their own destructors, then destroy their associated
-children.
+This function asserts that all objects which has associated destructors or
+associated children have been destroyed at the time it is called. It is meant to
+be a low level hook that testing frameworks like `ember-qunit` and `ember-mocha`
+can use to hook into and validate that all destroyables have in fact been
+destroyed.
 
 ### Built In Destroyables
 

From 062f557733cdfd4dcea276fdda4e32c9ce32981f Mon Sep 17 00:00:00 2001
From: Chris Garrett <cgarrett@linkedin.com>
Date: Wed, 4 Mar 2020 10:59:27 -0800
Subject: [PATCH 06/11] update import path

---
 text/0580-destroyables.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/text/0580-destroyables.md b/text/0580-destroyables.md
index 53b298bf49..1f048759be 100644
--- a/text/0580-destroyables.md
+++ b/text/0580-destroyables.md
@@ -96,7 +96,7 @@ maximizes interoperability in general.
 
 ## Detailed design
 
-The API consists of 6 main functions, imported from `@ember/application`:
+The API consists of 6 main functions, imported from `@ember/destroyable`:
 
 ```ts
 declare function associateDestroyableChild(parent: object, child: object): void;

From aa3ae58c714a2e1d7bf72a37dd50e2b8b3ec9a48 Mon Sep 17 00:00:00 2001
From: Chris Garrett <cgarrett@linkedin.com>
Date: Wed, 4 Mar 2020 11:08:46 -0800
Subject: [PATCH 07/11] fix grammar

---
 text/0580-destroyables.md | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/text/0580-destroyables.md b/text/0580-destroyables.md
index 1f048759be..a8248edee3 100644
--- a/text/0580-destroyables.md
+++ b/text/0580-destroyables.md
@@ -169,7 +169,7 @@ destructor from the destroyable.
 
 #### `destroy`
 
-`destroy` initiates the destruction of an destroyable object. It runs all
+`destroy` initiates the destruction of a destroyable object. It runs all
 associated destructors, and then destroys all children recursively.
 
 Destruction via `destroy()` follows these steps:
@@ -182,17 +182,17 @@ Destruction via `destroy()` follows these steps:
 Calling `destroy` multiple times on the same destroyable is safe. It will not
 throw an error, and will not take any further action.
 
-Calling `destroy` with an destroyable that has no destructors or associated children
+Calling `destroy` with a destroyable that has no destructors or associated children
 will not throw an error, and will do nothing.
 
 #### `isDestroying`
 
-Receives an destroyable, and returns `true` if the destroyable has begun
+Receives a destroyable, and returns `true` if the destroyable has begun
 destroying. Otherwise returns false.
 
 #### `isDestroyed`
 
-Receives an destroyable, and returns `true` if the destroyable has finished
+Receives a destroyable, and returns `true` if the destroyable has finished
 destroying. Otherwise returns false.
 
 #### `assertDestroyablesDestroyed`

From 6f189cbdbe75c66789a3822ae1be7febcd78a5bb Mon Sep 17 00:00:00 2001
From: Chris Garrett <cgarrett@linkedin.com>
Date: Wed, 4 Mar 2020 11:09:27 -0800
Subject: [PATCH 08/11] Update text/0580-destroyables.md

Co-Authored-By: Robert Jackson <me@rwjblue.com>
---
 text/0580-destroyables.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/text/0580-destroyables.md b/text/0580-destroyables.md
index a8248edee3..420a58562f 100644
--- a/text/0580-destroyables.md
+++ b/text/0580-destroyables.md
@@ -197,7 +197,7 @@ destroying. Otherwise returns false.
 
 #### `assertDestroyablesDestroyed`
 
-This function asserts that all objects which has associated destructors or
+This function asserts that all objects which have associated destructors or
 associated children have been destroyed at the time it is called. It is meant to
 be a low level hook that testing frameworks like `ember-qunit` and `ember-mocha`
 can use to hook into and validate that all destroyables have in fact been

From c2e8c958933997ad6538d18545ca67806f2fab45 Mon Sep 17 00:00:00 2001
From: Chris Garrett <cgarrett@linkedin.com>
Date: Wed, 4 Mar 2020 12:13:34 -0800
Subject: [PATCH 09/11] make destroy async + scheduled

---
 text/0580-destroyables.md | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/text/0580-destroyables.md b/text/0580-destroyables.md
index a8248edee3..a44c6f91c7 100644
--- a/text/0580-destroyables.md
+++ b/text/0580-destroyables.md
@@ -175,9 +175,14 @@ associated destructors, and then destroys all children recursively.
 Destruction via `destroy()` follows these steps:
 
 1. Mark the destroyable such that `isDestroying(destroyable)` returns `true`
-2. Call the destroyable's destructors
-3. Call `destroy()` on each of the destroyable's associated children
-4. Mark the destroyable such that `isDestroyed(destroyable)` returns `true`
+2. Schedule calling the destroyable's destructors
+4. Call `destroy()` on each of the destroyable's associated children
+3. Schedule setting destroyable such that `isDestroyed(destroyable)` returns `true`
+
+This algorithm results in the entire tree of destroyables being first marked as
+destroyed, then having all of their destructors called, and finally all being
+marked as `isDestroyed`. There won't be any in between states where some items
+are marked as `isDestroying` while destroying, while others are not.
 
 Calling `destroy` multiple times on the same destroyable is safe. It will not
 throw an error, and will not take any further action.

From cf3fed0af373ff2a9a87dd2d39b544e8e5151571 Mon Sep 17 00:00:00 2001
From: Chris Garrett <cgarrett@linkedin.com>
Date: Thu, 9 Apr 2020 18:48:16 -0700
Subject: [PATCH 10/11] respond to feedback

---
 text/0580-destroyables.md | 192 ++++++++++++++++++++++++++++++++------
 1 file changed, 165 insertions(+), 27 deletions(-)

diff --git a/text/0580-destroyables.md b/text/0580-destroyables.md
index 85162f0bf9..24568d98b1 100644
--- a/text/0580-destroyables.md
+++ b/text/0580-destroyables.md
@@ -29,7 +29,7 @@ class TimeoutManager {
     let timeoutId = setTimeout(fn, timeout);
 
     associateDestroyableChild(parent, this);
-    registerDestructor(this, () => clearTimeout(timeoutId))
+    registerDestructor(this, () => clearTimeout(timeoutId));
   }
 }
 
@@ -72,11 +72,11 @@ downsides:
 
      let oldWillDestroy = obj.willDestroy;
 
-     obj.willDestroy = function() {
+     obj.willDestroy = function () {
        if (oldWillDestroy) oldWillDestroy.call(this);
 
        teardownTasks(this);
-     }
+     };
 
      PATCHED.add(obj);
    }
@@ -99,9 +99,17 @@ maximizes interoperability in general.
 The API consists of 6 main functions, imported from `@ember/destroyable`:
 
 ```ts
-declare function associateDestroyableChild(parent: object, child: object): void;
-declare function registerDestructor(destroyable: object, destructor: () => void): void;
-declare function unregisterDestructor(destroyable: object, destructor: () => void): void;
+declare function associateDestroyableChild<T extends object>(parent: object, child: T): T;
+
+declare function registerDestructor<T extends object>(
+  destroyable: T,
+  destructor: (destroyable: T) => void
+): (destroyable: T) => void;
+
+declare function unregisterDestructor<T extends object>(
+  destroyable: T,
+  destructor: (destroyable: T) => void
+): void;
 
 declare function destroy(destroyable: object): void;
 declare function isDestroying(destroyable: object): boolean;
@@ -124,6 +132,18 @@ that fulfills this property can be used with this system.
 This function is used to associate a destroyable object with a parent. When the
 parent is destroyed, all registered children will also be destroyed.
 
+```js
+class CustomSelect extends Component {
+  constructor() {
+    // obj is now a child of the component. When the component is destroyed,
+    // obj will also be destroyed, and have all of its destructors triggered.
+    this.obj = associateDestroyableChild(this, {});
+  }
+}
+```
+
+Returns the associated child for convenience.
+
 - Attempting to associate a parent or child that has already been destroyed
   should throw an error.
 
@@ -135,7 +155,9 @@ inheritance of destructors is tricky and not scoped in. Instead, users can add
 destructors to accomplish this goal:
 
 ```js
-let parent1 = {}, parent2 = {}, child = {};
+let parent1 = {},
+  parent2 = {},
+  child = {};
 
 registerDestructor(parent1, () => destroy(child));
 registerDestructor(parent2, () => destroy(child));
@@ -149,10 +171,48 @@ inheritance baked in in the future, it can be added in a followup RFC.
 
 Receives a destroyable object and a destructor function, and associates the
 function with it. When the destroyable is destroyed with `destroy`, or when its
-parent is destroyed, the destructor function will be called. Multiple
-destructors can be associated with a given destroyable, and they can be
+parent is destroyed, the destructor function will be called.
+
+```js
+import { registerDestructor } from '@ember/destroyable';
+
+class Modal extends Component {
+  @service resize;
+
+  constructor() {
+    this.resize.register(this, this.layout);
+
+    registerDestructor(this, () => this.resize.unregister(this));
+  }
+}
+```
+
+Multiple destructors can be associated with a given destroyable, and they can be
 associated over time, allowing libraries like `ember-lifeline` to dynamically
-add destructors as needed.
+add destructors as needed. `registerDestructor` also returns the associated
+destructor function, for convenience.
+
+The destructor function is passed a single argument, which is the destroyable
+itself. This allows the function to be reused multiple times for many
+destroyables, rather than creating a closure function per destroyable.
+
+```js
+import { registerDestructor } from '@ember/destroyable';
+
+function unregisterResize(instance) {
+  instance.resize.unregister(instance);
+}
+
+class Modal extends Component {
+  @service resize;
+
+  constructor() {
+    this.resize.register(this, this.layout);
+
+    registerDestructor(this, unregisterResize);
+  }
+}
+```
 
 - Registering a destructor on a destroyed object should throw an error.
 - Attempting to register the same destructor multiple times should throw an
@@ -163,6 +223,24 @@ add destructors as needed.
 Receives a destroyable and a destructor function, and de-associates the
 destructor from the destroyable.
 
+```js
+import { unregisterDestructor } from '@ember/destroyable';
+
+class Modal extends Component {
+  @service modals;
+
+  constructor() {
+    this.modals.add(this);
+
+    this.modalDestructor = registerDestructor(this, () => this.modals.remove(this));
+  }
+
+  @action pinModal() {
+    unregisterDestructor(this, this.modalDestructor);
+  }
+}
+```
+
 - Calling `unregisterDestructor` on a destroyed object should throw an error.
 - Calling `unregisterDestructor` with a destructor that is not associated with
   the object should throw an error.
@@ -172,15 +250,27 @@ destructor from the destroyable.
 `destroy` initiates the destruction of a destroyable object. It runs all
 associated destructors, and then destroys all children recursively.
 
+```js
+let obj = {};
+
+registerDestructor(obj, () => console.log('destroyed!'));
+
+destroy(obj); // this will schedule the destructor to be called
+
+// ...some time later, during scheduled destruction
+
+// destroyed!
+```
+
 Destruction via `destroy()` follows these steps:
 
 1. Mark the destroyable such that `isDestroying(destroyable)` returns `true`
 2. Schedule calling the destroyable's destructors
-4. Call `destroy()` on each of the destroyable's associated children
-3. Schedule setting destroyable such that `isDestroyed(destroyable)` returns `true`
+3. Call `destroy()` on each of the destroyable's associated children
+4. Schedule setting destroyable such that `isDestroyed(destroyable)` returns `true`
 
 This algorithm results in the entire tree of destroyables being first marked as
-destroyed, then having all of their destructors called, and finally all being
+destroying, then having all of their destructors called, and finally all being
 marked as `isDestroyed`. There won't be any in between states where some items
 are marked as `isDestroying` while destroying, while others are not.
 
@@ -195,11 +285,30 @@ will not throw an error, and will do nothing.
 Receives a destroyable, and returns `true` if the destroyable has begun
 destroying. Otherwise returns false.
 
+```js
+let obj = {};
+
+isDestroying(obj); // false
+destroy(obj);
+isDestroying(obj); // true
+```
+
 #### `isDestroyed`
 
 Receives a destroyable, and returns `true` if the destroyable has finished
 destroying. Otherwise returns false.
 
+```js
+let obj = {};
+
+isDestroyed(obj); // false
+destroy(obj);
+
+// ...sometime later, after scheduled destruction
+
+isDestroyed(obj); // true
+```
+
 #### `assertDestroyablesDestroyed`
 
 This function asserts that all objects which have associated destructors or
@@ -213,21 +322,55 @@ destroyed.
 The root destroyable of an Ember application will be the instance of the owner.
 All framework managed classes are destroyables, including:
 
-* Components
-* Services
-* Routes
-* Controllers
-* Helpers
-* Modifiers
+- Components
+- Services
+- Routes
+- Controllers
+- Helpers
+- Modifiers
 
 Any future classes that are added and have a container managed lifecycle should
 also be marked as destroyables.
 
 ## How we teach this
 
-This is a generally low level API, meant for usage in the framework and in
-addons. It should be taught primarily through API documentation, and potentially
-through an in-depth guide.
+Destroyables are not a very commonly used primitive, but they are fairly core to
+Ember applications. Most destruction lifecycle hooks will be rationalized as
+destroyables under the hood, and and it is key to how the application manages
+lifecycles. As such, destroyables should be covered in an _In-Depth Guide_ in
+the Core Concepts section of the guides.
+
+### Guide Outline
+
+The guide should start by discussing lifecycle, in particular focusing on in the
+existing lifecycle hooks that users will already know about, such as
+`willDestroy` on components. It should cover how at a high level, every
+framework concept exists in a _lifecycle tree_, where children are tied to the
+lifecyles of their parents. When something in the tree is destroyed, like a
+component so are all of its children.
+
+The destroyable APIs can then be brought in to discuss how one might add to the
+tree, if they have concepts whose lifecycles would logically belong to it. This
+should be done primarily through examples. Some ideas for possible examples
+include:
+
+1. A simple remote data fetcher. The request needs to be cancelled if the parent
+   is destroyed, which is a perfect use case for a destroyable.
+2. A task manager that manages a variety of long lived tasks.
+3. Possibly another example where a completely independent tree is made, for
+   some sort of library that would be otherwise external to Ember.
+
+The rest of the guide could show in detail how the user would use the APIs to
+accomplish this goal, and how it would be better and more scalable than doing it
+with lifecycle hooks.
+
+There should also be a section on _when_ to use the low-level destroyable APIs,
+vs the standard lifecycle hooks.
+
+### API Docs
+
+The descriptions of the APIs above in the RFC are sufficient detail for the bulk
+of API documentation, with some light editing.
 
 ## Drawbacks
 
@@ -240,8 +383,3 @@ through an in-depth guide.
 
 - Continue using existing lifecycle hooks for public API, and don't provide an
   independent API.
-
-## Unresolved questions
-
-- Should `willDestroy` and `didDestroy` (e.g. mark-and-sweep style semantics) be
-  exposed?

From 1ef3267f36dc5e2bd5589c2f69a941f320efb320 Mon Sep 17 00:00:00 2001
From: Chris Garrett <cgarrett@linkedin.com>
Date: Fri, 17 Apr 2020 12:00:12 -0700
Subject: [PATCH 11/11] Apply suggestions from code review
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-Authored-By: Jan Buschtöns <buschtoens@gmail.com>
---
 text/0580-destroyables.md | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/text/0580-destroyables.md b/text/0580-destroyables.md
index 24568d98b1..1414ca8089 100644
--- a/text/0580-destroyables.md
+++ b/text/0580-destroyables.md
@@ -145,7 +145,7 @@ class CustomSelect extends Component {
 Returns the associated child for convenience.
 
 - Attempting to associate a parent or child that has already been destroyed
-  should throw an error.
+  or is being destroyed should throw an error.
 
 ##### Multiple Inheritance
 
@@ -214,7 +214,7 @@ class Modal extends Component {
 }
 ```
 
-- Registering a destructor on a destroyed object should throw an error.
+- Registering a destructor on a destroyed object or object that is being destroyed should throw an error.
 - Attempting to register the same destructor multiple times should throw an
   error.
 
@@ -287,10 +287,12 @@ destroying. Otherwise returns false.
 
 ```js
 let obj = {};
-
 isDestroying(obj); // false
 destroy(obj);
 isDestroying(obj); // true
+// ...sometime later, after scheduled destruction
+isDestroyed(obj); // true
+isDestroying(obj); // true
 ```
 
 #### `isDestroyed`