respond to feedback

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?