From b9f390cb98f560d9cf876e3b1d67226fe0e1613b Mon Sep 17 00:00:00 2001
From: Christopher Garrett <opensource@pzuraq.com>
Date: Wed, 12 Dec 2018 20:53:44 -0800
Subject: [PATCH 1/7] Render Element Modifiers

---
 text/0000-render-element-modifiers.md | 570 ++++++++++++++++++++++++++
 1 file changed, 570 insertions(+)
 create mode 100644 text/0000-render-element-modifiers.md

diff --git a/text/0000-render-element-modifiers.md b/text/0000-render-element-modifiers.md
new file mode 100644
index 0000000000..643254b31a
--- /dev/null
+++ b/text/0000-render-element-modifiers.md
@@ -0,0 +1,570 @@
+- Start Date: 2018-12-13
+- RFC PR: (leave this empty)
+- Ember Issue: (leave this empty)
+
+# Render Element Modifiers
+
+## Summary
+
+Element modifiers are a recently introduced concept in Ember that allow users to
+run code that is tied to the lifecycle of an _element_ in a template, rather
+than the component's lifecycle. They allow users to write self-contained logic
+for manipulating the state of elements, and in many cases can be fully
+independent of component code and state.
+
+However, there are many cases where users wish to run some component code when
+an element is setting up or tearing down. Today, this logic conventionally lives
+in the `didInsertElement`, `didRender`, `didUpdate`, and  `willDestroyElement`
+hooks in components, but there are cases where these hooks are not ideal.
+
+This RFC proposes adding two new generic element modifiers, `{{did-render}}` and
+`{{will-destroy}}`, which users can use to run code during the most common
+phases of any element's lifecycle.
+
+## Motivation
+
+The primary component hooks for interacting with the DOM today are:
+
+* `didInsertElement`
+* `didRender`
+* `didUpdate`
+* `willDestroyElement`
+
+These render hooks cover many use cases. However, there are some cases which
+they do not cover, such as setting up logic for conditional elements, or tagless
+components. There also is no easy way to share element setup logic, aside from
+mixins or pure functions (which require some amount of boilerplate).
+
+### Conditionals
+
+Render code for elements which exist conditionally is fairly tricky. Consider a
+simple popover component:
+
+```hbs
+{{#if this.isOpen}}
+  <div class="popover">
+    {{yield}}
+  </div>
+{{/if}}
+```
+
+If the developer decides to use an external library like [Popper.js](https://popper.js.org)
+to position the popover, they have to add a fair amount of boilerplate. On each
+render, they need to check if the popover was added to the DOM or removed from
+it, and setup or teardown accordingly.
+
+```js
+export default Component.extend({
+  didRender() {
+    if (this.isOpen && !this._popper) {
+      let popoverElement = this.element.querySelector('.popover');
+
+      this._popper = new Popper(document, popoverElement);
+    } else if (this._popper) {
+      this._popper.destroy();
+    }
+  },
+
+  willDestroyElement() {
+    if (this._popper) {
+      this._popper.destroy();
+    }
+  }
+});
+```
+
+At this level of complexity, most developers would reasonably choose to create
+a second component to be used within the `{{if}}` block so they can use standard
+lifecycle hooks. Sometimes this makes sense as it helps to separate concerns and
+organize code, but other times it is clearly working around the limitations of
+render hooks, and can feel like more components are being created than are
+necessary.
+
+With render modifiers, hooks are run whenever the _element_ they are applied to
+is setup and torn down, which means we can focus on the setup and teardown code
+without worrying about the overall lifecycle:
+
+```hbs
+{{#if this.isOpen}}
+  <div
+    {{did-render (action this.setupPopper)}}
+    {{will-destroy (action this.teardownPopper)}}
+
+    class="popover"
+  >
+    {{yield}}
+  </div>
+{{/if}}
+```
+```js
+export default Component.extend({
+  setupPopper(element) {
+    this._popper = new Popper(document, element);
+  },
+
+  teardownPopper() {
+    this._popper.destroy();
+  }
+});
+```
+
+The element that the modifiers are applied to is also passed to the function, so
+there is no longer a need to use `querySelector`. Overall the end result is a
+fair amount simpler, without the need for an additional component.
+
+These same issues are also present for collections items within an `{{each}}`
+loop, and the render modifiers can be used to solve them as well:
+
+```hbs
+<ul>
+  {{#each items as |item|}}
+    <li
+      {{did-render (action this.registerElement)}}
+      {{will-destroy (action this.unregisterElement)}}
+    >
+      ...
+    </li>
+  {{/each}}
+</ul>
+```
+
+### Tagless Components
+
+Additionally, render hooks do not provide great support for tagless components
+(`tagName: ''`). While the hooks fire when the component is rendered, they have
+no way to target any of the elements which are in the component's template,
+meaning users must use `querySelector` and setup some unique id or class to
+target the element by:
+
+```js
+export default Component.extend({
+  tagName: '',
+
+  listId: computed(function() {
+    return generateId();
+  }),
+
+  didInsertElement() {
+    let element = document.querySelector(`#${this.listId}`);
+
+    // ...
+  },
+
+  willDestroyElement() {
+    let element = document.querySelector(`#${this.listId}`);
+
+// ...
+  }
+});
+```
+```hbs
+<ul id={{listId}}>
+  ...
+</ul>
+
+<div>
+  ...
+</div>
+```
+
+The render modifiers can be used to add hooks to the appropriate main element in
+tagless components:
+
+```js
+export default Component.extend({
+  tagName: '',
+
+  didInsertList(element) {
+    // ...
+  },
+
+  willDestroyList(element) {
+    // ...
+  }
+});
+```
+```hbs
+<ul
+  {{did-render (action this.didInsertList)}}
+  {{will-destroy (action this.willDestroyList)}}
+>
+  ...
+</ul>
+
+<div>
+  ...
+</div>
+```
+
+### Reusable Helpers
+
+Currently, the best ways to share element setup code are either via mixins,
+which are somewhat opaque and can encourage problematic patterns, or standard JS
+functions, which generally require some amount of boilerplate.
+
+Developers will be able to define element modifiers in the future with modifier
+managers provided by addons. However, the proposed modifier APIs are fairly
+verbose (with good reason) and not stabilized.
+
+However, `{{did-render}}` and `{{will-destroy}}` can receive _any_ function as
+their first parameter, allowing users to share and reuse common element setup
+code with helpers. For instance, a simple `scrollTo` helper could be created to
+set the scroll position of an element:
+
+```js
+// helpers/scroll-to.js
+export default function scrollTo([element, scrollPosition]) {
+  element.scrollTop = scrollPosition;
+}
+```
+```hbs
+<div {{did-render scroll-to @scrollPosition}} class="scroll-container">
+  ...
+</div>
+```
+
+## Detailed design
+
+This RFC proposes adding two element modifiers, `{{did-render}}` and
+`{{will-destroy}}`.
+
+### `{{did-render}}`
+
+This modifier runs immediately after the element is inserted, and again whenever
+any of the arguments passed to it update, including the function passed as the
+first argument. It has the following timing semantics:
+
+* **Always**
+  * called _after_ the enclosing component's `didUpdateArgs` hook
+  * called _after_ any child element's `{{did-render}}` modifiers
+  * called in definition order in the template
+* **May or May Not**
+  * be called in the same tick as DOM insertion
+  * have the sibling nodes fully initialized in DOM
+
+> Note: These timing semantics were mostly defined in the
+> [element modifier manager RFC](https://github.com/emberjs/rfcs/blob/master/text/0373-Element-Modifier-Managers.md)
+> and are repeated here for clarity.
+
+`{{did-render}}` receives a function with the following signature as the first
+positional parameter:
+
+```ts
+type DidRenderHandler = (element: Element, ...args): void;
+```
+
+The `element` argument is the element that the modifier is applied to, and the
+rest of the arguments are any remaining positional parameters passed to
+`{{did-render}}`. If the first positional parameter is not a callable function,
+`{{did-render}}` will throw an error.
+
+### `{{will-destroy}}`
+
+This modifier runs immediately before the element is removed. It has the
+following timing semantics:
+
+* **Always**
+  * called _after_ the enclosing component's `didUpdateArgs` hook
+  * called _before_ the enclosing component's `willDestroy` hook
+  * called _after_ any child element's `{{will-destroy}}` modifiers
+  * called in definition order in the template
+* **May or May Not**
+  * be called in the same tick as DOM insertion
+  * have the sibling nodes fully initialized in DOM
+
+> Note: These timing semantics were mostly defined in the
+> [element modifier manager RFC](https://github.com/emberjs/rfcs/blob/master/text/0373-Element-Modifier-Managers.md)
+> and are repeated here for clarity.
+
+`{{will-destroy}}` receives a function with the following signature as the first
+positional parameter:
+
+```ts
+type WillDestroyHandler = function(element: Element, ...args): void;
+```
+
+The `element` argument is the element that the modifier is applied to, and the
+rest of the arguments are any remaining positional parameters passed to
+`{{will-destroy}}`. If the first positional parameter is not a callable function,
+`{{will-destroy}}` will throw an error.
+
+### Function Binding
+
+Functions which are passed to these element modifiers will _not_ be bound to any
+context by default. Users can bind them using the `(action)` helper:
+
+```hbs
+<div {{did-render (action this.teardownElement)}}></div>
+```
+
+Currently, neither modifiers nor helpers in Glimmer are given the context of the
+template at any point. Both the `{{action}}` helper and modifier are given the
+context as an implicit first argument, via an AST transform. The above becomes
+the following in the final template, before it is compiled into the Glimmer byte
+code:
+
+```hbs
+<div {{did-render (action this this.teardownElement)}}></div>
+```
+
+This gives `{{action}}` the correct context to bind the function it is passed
+and was done purely for backwards compatibility, since `{{action}}` existed
+before modifiers and helpers were fully rationalized as features.
+
+Adding this implicit context to other helpers and modifiers would require
+changes to the Glimmer VM and is a much larger language design problem. As such,
+we believe it is out of scope for this RFC. Default binding behavior could be
+added in the future, if a context API is decided on.
+
+## How we teach this
+
+Element modifiers will be new to everyone, so we're starting with a mostly blank
+slate. The only modifier that exists in classic Ember is `{{action}}`, and while
+most existing users will be familiar with it, that familiarity may not translate
+to the more general idea of modifiers.
+
+The first thing we should focus on is teaching _modifiers in general_. Modifiers
+should be seen as the place for any logic which needs to act directly on an
+element, or when an element is added to or removed from the DOM. Modifiers can
+be fully independent (for instance, a `scroll-to` modifier that transparently
+manages the scroll position of the element) or they can interact with the
+component (like the `did-render` and `will-destroy` modifiers). In all cases
+though, they are _tied to the render lifecycle of the element_, and they
+generally contain _side-effects_ (though these may be transparent and
+declarative, as in the case of `{{action}}` or the theoretical `{{scroll-to}}`).
+
+Second, we should teach the render modifiers specifically. We can do this by
+illustrating common use cases which can currently be solved with render hooks,
+and comparing them to using modifiers for the same solution.
+
+One thing we should definitely avoid teaching except in advanced cases is the
+_ordering_ of element modifiers. Ideally, element modifiers should be
+commutative, and order should not be something users have to think about. When
+custom element modifiers become widely available, this should be considered best
+practice.
+
+### Example: Scrolling an element to a position
+
+This sets the scroll position of an element, and updates it whenever the scroll
+position changes.
+
+Before:
+
+```hbs
+{{yield}}
+```
+```js
+export default Component.extend({
+  classNames: ['scroll-container'],
+
+  didRender() {
+    this.element.scrollTop = this.scrollPosition;
+  }
+});
+```
+
+After:
+
+```hbs
+<div {{did-render this.setScrollPosition @scrollPosition}} class="scroll-container">
+  {{yield}}
+</div>
+```
+```js
+export default class Component.extend({
+  setScrollPosition(element, scrollPosition) {
+    element.scrollTop = scrollPosition;
+  }
+})
+```
+
+#### Example: Adding a class to an element after render for CSS animations
+
+This adds a CSS class to an alert element in a conditional whenever it renders
+to fade it in, which is a bit of an extra hoop.
+
+Before:
+
+```hbs
+{{#if shouldShow}}
+  <div class="alert">
+    {{yield}}
+  </div>
+{{/if}}
+```
+```js
+export default Component.extend({
+  didRender() {
+    let alert = this.element.querySelector('.alert');
+
+    if (alert) {
+      alert.classList.add('fade-in');
+    }
+  }
+});
+```
+
+After:
+
+```hbs
+{{#if shouldShow}}
+  <div {{did-render this.fadeIn}} class="alert">
+    {{yield}}
+  </div>
+{{/if}}
+```
+```js
+export default Component.extend({
+  fadeIn(element) {
+    element.classList.add('fade-in');
+  }
+});
+```
+
+#### Example: Resizing text area
+
+One key thing to know about `{{did-render}}` is it will not rerun whenever the
+_contents_ or _attributes_ on the element change. For instance, `{{did-render}}`
+will _not_ rerun when `@type` changes here:
+
+```hbs
+<div {{did-render this.setupType}} class={{@type}}></div>
+```
+
+If `{{did-render}}` should rerun whenever a value changes, the value should be
+passed as a parameter to the modifier. For instance, a textarea which wants to
+resize itself to fit text whenever the text is modified could be setup like
+this:
+
+```hbs
+<textarea {{did-render this.resizeArea @text}}>
+  {{@text}}
+</textarea>
+```
+```js
+export default Component.extend({
+  resizeArea(element) {
+    element.css.height = `${element.scrollHeight}px`;
+  }
+});
+```
+
+#### Example: `ember-composability-tools` style rendering
+
+This is the type of rendering done by libraries like `ember-leaflet`, which use
+components to control the _rendering_ of the library, but without any templates
+themselves. The underlying library for this is [here](https://github.com/miguelcobain/ember-composability-tools).
+This is a simplified example of how you could accomplish this with Glimmer
+components and element modifiers.
+
+Node component:
+
+```js
+// components/node.js
+export default Component.extend({
+  init() {
+    super(...arguments);
+    this.children = new Set();
+
+    this.args.parent.registerChild(this);
+  }
+
+  willDestroy() {
+    super(...arguments);
+
+    this.args.parent.unregisterChild(this);
+  }
+
+  registerChild(child) {
+    this.children.add(child);
+  }
+
+  unregisterChild(child) {
+    this.children.delete(child);
+  }
+
+  didInsertNode(element) {
+    // library setup code goes here
+
+    this.children.forEach(c => c.didInsertNode(element));
+  }
+
+  willDestroyNode(element) {
+    // library teardown code goes here
+
+    this.children.forEach(c => c.willDestroyNode(element));
+  }
+}
+```
+```hbs
+<!-- components/node.hbs -->
+{{yield (component "node" parent=this)}}
+```
+
+Root component:
+
+```js
+// components/root.js
+import NodeComponent from './node.js';
+
+export default NodeComponent.extend();
+```
+```hbs
+<!-- components/root.hbs -->
+<div
+  {{did-render (action this.didInsertNode)}}
+  {{will-destroy (action this.willDestroyNode)}}
+>
+  {{yield (component "node" parent=this)}}
+</div>
+```
+
+Usage:
+
+```hbs
+<Root as |node|>
+  <node as |node|>
+    <node />
+  </node>
+</Root>
+```
+
+## Drawbacks
+
+* Element modifiers are a new concept that haven't been fully stabilized as of
+  yet. It may be premature to add default modifiers to the framework.
+
+* Adding these modifiers means that there are more ways to accomplish similar
+  goals, which may be confusing to developers. It may be less clear which is the
+  conventional solution in a given situation.
+
+* Relying on users binding via `action` is somewhat unintuitive, and may feel
+  like it's getting in the way, especially considering sometimes methods will
+  work without binding (if they never access `this`).
+
+## Alternatives
+
+* Stick with only lifecycle hooks for these situations, and don't add generic
+  modifiers for them.
+
+* Add an implicit context to modifiers and helpers, instead of relying on users
+  to bind functions manually. Doing this should take into account a few
+  constraints and considerations:
+
+  * Adding an implicit context may make it more difficult to optimize modifiers
+    and helpers in the future. If possible, this should be something they opt
+    _into_, so only helpers which _need_ a context will deoptimize.
+
+  * Binding can be counterintuitive in some cases. For instance:
+
+    ```hbs
+    <button {{action this.service.reloadData}}>Reload</button>
+    ```
+
+    This example will likely error, because the `reloadData` function will be
+    bound to the _component_, not the service. Likewise, binding helpers doesn't
+    really make sense, since they should be pure functions. Solutions like the
+    [`{{bind}}` helper](https://github.com/Serabe/ember-bind-helper) attempt to
+    address this, but may not be something that can be fully rationalized (what
+    happens if there are multiple contexts?)
+

From 371a16dc56e1c5754e5020f57ad77a3554001d52 Mon Sep 17 00:00:00 2001
From: Robert Jackson <me@rwjblue.com>
Date: Thu, 13 Dec 2018 17:14:24 -0800
Subject: [PATCH 2/7] Apply suggestions from code review

Co-Authored-By: pzuraq <me@pzuraq.com>
---
 text/0000-render-element-modifiers.md | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/text/0000-render-element-modifiers.md b/text/0000-render-element-modifiers.md
index 643254b31a..522114f5e3 100644
--- a/text/0000-render-element-modifiers.md
+++ b/text/0000-render-element-modifiers.md
@@ -213,12 +213,12 @@ set the scroll position of an element:
 
 ```js
 // helpers/scroll-to.js
-export default function scrollTo([element, scrollPosition]) {
-  element.scrollTop = scrollPosition;
+export default function scrollTo() {
+  (element, scrollPosition) => element.scrollTop = scrollPosition;
 }
 ```
 ```hbs
-<div {{did-render scroll-to @scrollPosition}} class="scroll-container">
+<div {{did-render (scroll-to) @scrollPosition}} class="scroll-container">
   ...
 </div>
 ```
@@ -428,7 +428,7 @@ _contents_ or _attributes_ on the element change. For instance, `{{did-render}}`
 will _not_ rerun when `@type` changes here:
 
 ```hbs
-<div {{did-render this.setupType}} class={{@type}}></div>
+<div {{did-render this.setupType}} class="{{@type}}"></div>
 ```
 
 If `{{did-render}}` should rerun whenever a value changes, the value should be
@@ -466,13 +466,13 @@ export default Component.extend({
     super(...arguments);
     this.children = new Set();
 
-    this.args.parent.registerChild(this);
+    this.parent.registerChild(this);
   }
 
   willDestroy() {
     super(...arguments);
 
-    this.args.parent.unregisterChild(this);
+    this.parent.unregisterChild(this);
   }
 
   registerChild(child) {

From 5fc752111be0d983005ec3687189ddef11299ee2 Mon Sep 17 00:00:00 2001
From: Christopher Garrett <opensource@pzuraq.com>
Date: Thu, 13 Dec 2018 17:36:53 -0800
Subject: [PATCH 3/7] update based on feedback

---
 text/0000-render-element-modifiers.md | 38 +++++++++++++++------------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/text/0000-render-element-modifiers.md b/text/0000-render-element-modifiers.md
index 522114f5e3..845c85afed 100644
--- a/text/0000-render-element-modifiers.md
+++ b/text/0000-render-element-modifiers.md
@@ -226,7 +226,12 @@ export default function scrollTo() {
 ## Detailed design
 
 This RFC proposes adding two element modifiers, `{{did-render}}` and
-`{{will-destroy}}`.
+`{{will-destroy}}`. Note that element modifiers do _not_ run in SSR mode - this
+code is only run on clients.
+
+> Note: The timing semantics in the following section were mostly defined in the
+> [element modifier manager RFC](https://github.com/emberjs/rfcs/blob/master/text/0373-Element-Modifier-Managers.md)
+> and are repeated here for clarity and convenience.
 
 ### `{{did-render}}`
 
@@ -235,16 +240,17 @@ any of the arguments passed to it update, including the function passed as the
 first argument. It has the following timing semantics:
 
 * **Always**
-  * called _after_ the enclosing component's `didUpdateArgs` hook
+  * called after DOM insertion
   * called _after_ any child element's `{{did-render}}` modifiers
+  * called _after_ the enclosing component's `willRender` hook
+  * called _before_ the enclosing component's `didRender` hook
   * called in definition order in the template
+  * called after the arguments to the modifier have changed
 * **May or May Not**
   * be called in the same tick as DOM insertion
   * have the sibling nodes fully initialized in DOM
-
-> Note: These timing semantics were mostly defined in the
-> [element modifier manager RFC](https://github.com/emberjs/rfcs/blob/master/text/0373-Element-Modifier-Managers.md)
-> and are repeated here for clarity.
+* **Never**
+  * called if the arguments to the modifier are constants
 
 `{{did-render}}` receives a function with the following signature as the first
 positional parameter:
@@ -264,17 +270,11 @@ This modifier runs immediately before the element is removed. It has the
 following timing semantics:
 
 * **Always**
-  * called _after_ the enclosing component's `didUpdateArgs` hook
-  * called _before_ the enclosing component's `willDestroy` hook
   * called _after_ any child element's `{{will-destroy}}` modifiers
+  * called _before_ the enclosing component's `willDestroy` hook
   * called in definition order in the template
 * **May or May Not**
   * be called in the same tick as DOM insertion
-  * have the sibling nodes fully initialized in DOM
-
-> Note: These timing semantics were mostly defined in the
-> [element modifier manager RFC](https://github.com/emberjs/rfcs/blob/master/text/0373-Element-Modifier-Managers.md)
-> and are repeated here for clarity.
 
 `{{will-destroy}}` receives a function with the following signature as the first
 positional parameter:
@@ -316,6 +316,10 @@ changes to the Glimmer VM and is a much larger language design problem. As such,
 we believe it is out of scope for this RFC. Default binding behavior could be
 added in the future, if a context API is decided on.
 
+> Note: It's worth calling out that action's binding behavior can be confusing
+> in cases as well, check out [ember-bind-helper](https://github.com/Serabe/ember-bind-helper)
+> for an example and alternatives.
+
 ## How we teach this
 
 Element modifiers will be new to everyone, so we're starting with a mostly blank
@@ -366,7 +370,7 @@ export default Component.extend({
 After:
 
 ```hbs
-<div {{did-render this.setScrollPosition @scrollPosition}} class="scroll-container">
+<div {{did-render (action this.setScrollPosition) @scrollPosition}} class="scroll-container">
   {{yield}}
 </div>
 ```
@@ -408,7 +412,7 @@ After:
 
 ```hbs
 {{#if shouldShow}}
-  <div {{did-render this.fadeIn}} class="alert">
+  <div {{did-render (action this.fadeIn)}} class="alert">
     {{yield}}
   </div>
 {{/if}}
@@ -428,7 +432,7 @@ _contents_ or _attributes_ on the element change. For instance, `{{did-render}}`
 will _not_ rerun when `@type` changes here:
 
 ```hbs
-<div {{did-render this.setupType}} class="{{@type}}"></div>
+<div {{did-render (action this.setupType)}} class="{{@type}}"></div>
 ```
 
 If `{{did-render}}` should rerun whenever a value changes, the value should be
@@ -437,7 +441,7 @@ resize itself to fit text whenever the text is modified could be setup like
 this:
 
 ```hbs
-<textarea {{did-render this.resizeArea @text}}>
+<textarea {{did-render (action this.resizeArea) @text}}>
   {{@text}}
 </textarea>
 ```

From f2ef0e9339b58ff7eeedd92e192fb9eb82f472f2 Mon Sep 17 00:00:00 2001
From: Christopher Garrett <opensource@pzuraq.com>
Date: Thu, 13 Dec 2018 17:38:17 -0800
Subject: [PATCH 4/7] add CSS animation note

---
 text/0000-render-element-modifiers.md | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/text/0000-render-element-modifiers.md b/text/0000-render-element-modifiers.md
index 845c85afed..751a67f5c7 100644
--- a/text/0000-render-element-modifiers.md
+++ b/text/0000-render-element-modifiers.md
@@ -385,7 +385,9 @@ export default class Component.extend({
 #### Example: Adding a class to an element after render for CSS animations
 
 This adds a CSS class to an alert element in a conditional whenever it renders
-to fade it in, which is a bit of an extra hoop.
+to fade it in, which is a bit of an extra hoop. For CSS transitions to work, we
+need to append the element _without_ the class, then add the class after it has
+been appended.
 
 Before:
 

From af106cde8a163b610f1a7ed08c1375568d6d0518 Mon Sep 17 00:00:00 2001
From: Chris Garrett <cgarrett@linkedin.com>
Date: Thu, 3 Jan 2019 16:33:42 -0800
Subject: [PATCH 5/7] clarify timing semantics

---
 text/0000-render-element-modifiers.md | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/text/0000-render-element-modifiers.md b/text/0000-render-element-modifiers.md
index 751a67f5c7..9e5841fad0 100644
--- a/text/0000-render-element-modifiers.md
+++ b/text/0000-render-element-modifiers.md
@@ -235,9 +235,13 @@ code is only run on clients.
 
 ### `{{did-render}}`
 
-This modifier runs immediately after the element is inserted, and again whenever
-any of the arguments passed to it update, including the function passed as the
-first argument. It has the following timing semantics:
+This modifier is activated:
+
+1. When The element is inserted in the DOM
+2. Whenever any of the arguments passed to it update, including the function
+   passed as the first argument.
+
+It has the following timing semantics when activated:
 
 * **Always**
   * called after DOM insertion
@@ -252,6 +256,10 @@ first argument. It has the following timing semantics:
 * **Never**
   * called if the arguments to the modifier are constants
 
+Note that these statements do not refer to when the modifier is _activated_,
+only to when it will be run relative to other hooks and modifiers _should it be
+activated_. The modifier is only activated on insertion and arg changes.
+
 `{{did-render}}` receives a function with the following signature as the first
 positional parameter:
 
@@ -266,8 +274,11 @@ rest of the arguments are any remaining positional parameters passed to
 
 ### `{{will-destroy}}`
 
-This modifier runs immediately before the element is removed. It has the
-following timing semantics:
+This modifier is activated:
+
+1. immediately before the element is removed from the DOM.
+
+It has the following timing semantics when activated:
 
 * **Always**
   * called _after_ any child element's `{{will-destroy}}` modifiers

From e80bdd134d0963870aeb57bd2df185c354f4325d Mon Sep 17 00:00:00 2001
From: Chris Garrett <cgarrett@linkedin.com>
Date: Tue, 8 Jan 2019 15:21:39 -0800
Subject: [PATCH 6/7] Updates based on feedback, convert to official addon

---
 text/0000-render-element-modifiers.md | 205 +++++++++++++-------------
 1 file changed, 104 insertions(+), 101 deletions(-)

diff --git a/text/0000-render-element-modifiers.md b/text/0000-render-element-modifiers.md
index 9e5841fad0..a5b6add383 100644
--- a/text/0000-render-element-modifiers.md
+++ b/text/0000-render-element-modifiers.md
@@ -17,9 +17,10 @@ an element is setting up or tearing down. Today, this logic conventionally lives
 in the `didInsertElement`, `didRender`, `didUpdate`, and  `willDestroyElement`
 hooks in components, but there are cases where these hooks are not ideal.
 
-This RFC proposes adding two new generic element modifiers, `{{did-render}}` and
-`{{will-destroy}}`, which users can use to run code during the most common
-phases of any element's lifecycle.
+This RFC proposes creating an official Ember addon which provides three new
+generic element modifiers: `{{did-insert}}`, `{{did-update}}`, and
+`{{will-destroy}}`. Users will be able to use these to run code during the most
+common phases of any element's lifecycle.
 
 ## Motivation
 
@@ -87,7 +88,7 @@ without worrying about the overall lifecycle:
 ```hbs
 {{#if this.isOpen}}
   <div
-    {{did-render (action this.setupPopper)}}
+    {{did-insert (action this.setupPopper)}}
     {{will-destroy (action this.teardownPopper)}}
 
     class="popover"
@@ -119,7 +120,7 @@ loop, and the render modifiers can be used to solve them as well:
 <ul>
   {{#each items as |item|}}
     <li
-      {{did-render (action this.registerElement)}}
+      {{did-insert (action this.registerElement)}}
       {{will-destroy (action this.unregisterElement)}}
     >
       ...
@@ -185,7 +186,7 @@ export default Component.extend({
 ```
 ```hbs
 <ul
-  {{did-render (action this.didInsertList)}}
+  {{did-insert (action this.didInsertList)}}
   {{will-destroy (action this.willDestroyList)}}
 >
   ...
@@ -206,71 +207,106 @@ Developers will be able to define element modifiers in the future with modifier
 managers provided by addons. However, the proposed modifier APIs are fairly
 verbose (with good reason) and not stabilized.
 
-However, `{{did-render}}` and `{{will-destroy}}` can receive _any_ function as
-their first parameter, allowing users to share and reuse common element setup
-code with helpers. For instance, a simple `scrollTo` helper could be created to
-set the scroll position of an element:
+However, the render modifiers can receive _any_ function as their first
+parameter, allowing users to share and reuse common element setup code with
+helpers. For instance, a simple `scrollTo` helper could be created to set the
+scroll position of an element:
 
 ```js
 // helpers/scroll-to.js
 export default function scrollTo() {
-  (element, scrollPosition) => element.scrollTop = scrollPosition;
+  return (element, scrollPosition) => element.scrollTop = scrollPosition;
 }
 ```
 ```hbs
-<div {{did-render (scroll-to) @scrollPosition}} class="scroll-container">
+<div
+  {{did-insert (scroll-to) @scrollPosition}}
+  {{did-update (scroll-to) @scrollPosition}}
+  class="scroll-container"
+>
   ...
 </div>
 ```
 
+### Official Addon
+
+While these modifiers will be generally useful, modifiers are meant to be a more
+generic API that can be used to create libraries for solving specific problems.
+Unfortunately, the community hasn't had much time to experiment with modifiers,
+since the public API for them hasn't been finalized.
+
+The modifiers in this RFC will provide an basic stepping stone for users who
+want to emulate lifecycle hooks and incrementally convert their applications to
+modifiers while modifiers in general are being experimented with in the
+community. In time, users should be able to pick and choose the modifiers that
+suit their needs more directly and effectively, and they shouldn't have to
+include these modifiers in the payload. These modifiers should also not be seen
+as the "Ember way" - they are just another addon, a basic one supported by
+the Ember core team, but one which may or may not be appropriate for a given
+application.
+
 ## Detailed design
 
-This RFC proposes adding two element modifiers, `{{did-render}}` and
-`{{will-destroy}}`. Note that element modifiers do _not_ run in SSR mode - this
-code is only run on clients.
+This RFC proposes adding three element modifiers:
+
+* `{{did-insert}}`
+* `{{did-update}}`
+* `{{will-destroy}}`
+
+Note that element modifiers do _not_ run in SSR mode - this code is only run on
+clients. Each of these modifiers receives a callback as it's first positional
+parameter:
+
+```ts
+type RenderModifierCallback = (element: Element, positionalArgs: [any], namedArgs: object): void;
+```
+
+The `element` argument is the element that the modifier is applied to,
+`positionalArgs` contains any remaining positional arguments passed to the
+modifier besides the callback, and `namedArgs` contains any named arguments
+passed to the modifier. If the first positional argument is not a callable
+function, the modifier will throw an error.
 
 > Note: The timing semantics in the following section were mostly defined in the
 > [element modifier manager RFC](https://github.com/emberjs/rfcs/blob/master/text/0373-Element-Modifier-Managers.md)
 > and are repeated here for clarity and convenience.
 
-### `{{did-render}}`
-
-This modifier is activated:
+### `{{did-insert}}`
 
-1. When The element is inserted in the DOM
-2. Whenever any of the arguments passed to it update, including the function
-   passed as the first argument.
+This modifier is activated only when The element is inserted in the DOM.
 
 It has the following timing semantics when activated:
 
 * **Always**
   * called after DOM insertion
-  * called _after_ any child element's `{{did-render}}` modifiers
+  * called _after_ any child element's `{{did-insert}}` modifiers
   * called _after_ the enclosing component's `willRender` hook
   * called _before_ the enclosing component's `didRender` hook
   * called in definition order in the template
-  * called after the arguments to the modifier have changed
 * **May or May Not**
   * be called in the same tick as DOM insertion
   * have the sibling nodes fully initialized in DOM
-* **Never**
-  * called if the arguments to the modifier are constants
 
 Note that these statements do not refer to when the modifier is _activated_,
 only to when it will be run relative to other hooks and modifiers _should it be
-activated_. The modifier is only activated on insertion and arg changes.
+activated_. The modifier is only activated on insertion.
 
-`{{did-render}}` receives a function with the following signature as the first
-positional parameter:
+### `{{did-update}}`
 
-```ts
-type DidRenderHandler = (element: Element, ...args): void;
-```
+This modifier is activated only on _updates_ to it's arguments (both positional
+and named). It does _not_ run during or after initial render, or before
+element destruction.
 
-The `element` argument is the element that the modifier is applied to, and the
-rest of the arguments are any remaining positional parameters passed to
-`{{did-render}}`. If the first positional parameter is not a callable function,
-`{{did-render}}` will throw an error.
+It has the following timing semantics when activated:
+
+* **Always**
+  * called after the arguments to the modifier have changed
+  * called _after_ any child element's `{{did-update}}` modifiers
+  * called _after_ the enclosing component's `willUpdate` hook
+  * called _before_ the enclosing component's `didUpdate` hook
+  * called in definition order in the template
+* **Never**
+  * called if the arguments to the modifier are constants
 
 ### `{{will-destroy}}`
 
@@ -285,19 +321,7 @@ It has the following timing semantics when activated:
   * called _before_ the enclosing component's `willDestroy` hook
   * called in definition order in the template
 * **May or May Not**
-  * be called in the same tick as DOM insertion
-
-`{{will-destroy}}` receives a function with the following signature as the first
-positional parameter:
-
-```ts
-type WillDestroyHandler = function(element: Element, ...args): void;
-```
-
-The `element` argument is the element that the modifier is applied to, and the
-rest of the arguments are any remaining positional parameters passed to
-`{{will-destroy}}`. If the first positional parameter is not a callable function,
-`{{will-destroy}}` will throw an error.
+  * be called in the same tick as DOM removal
 
 ### Function Binding
 
@@ -305,32 +329,25 @@ Functions which are passed to these element modifiers will _not_ be bound to any
 context by default. Users can bind them using the `(action)` helper:
 
 ```hbs
-<div {{did-render (action this.teardownElement)}}></div>
+<div {{did-insert (action this.setupElement)}}></div>
 ```
 
-Currently, neither modifiers nor helpers in Glimmer are given the context of the
-template at any point. Both the `{{action}}` helper and modifier are given the
-context as an implicit first argument, via an AST transform. The above becomes
-the following in the final template, before it is compiled into the Glimmer byte
-code:
+Or by using the `@action` decorator provided by the
+[Decorators RFC](https://github.com/emberjs/rfcs/pull/408) to bind the function
+in the class itself:
 
+```js
+export default class ExampleComponent extends Component {
+  @action
+  setupElement() {
+    // ...
+  }
+}
+```
 ```hbs
-<div {{did-render (action this this.teardownElement)}}></div>
+<div {{did-insert this.setupElement}}></div>
 ```
 
-This gives `{{action}}` the correct context to bind the function it is passed
-and was done purely for backwards compatibility, since `{{action}}` existed
-before modifiers and helpers were fully rationalized as features.
-
-Adding this implicit context to other helpers and modifiers would require
-changes to the Glimmer VM and is a much larger language design problem. As such,
-we believe it is out of scope for this RFC. Default binding behavior could be
-added in the future, if a context API is decided on.
-
-> Note: It's worth calling out that action's binding behavior can be confusing
-> in cases as well, check out [ember-bind-helper](https://github.com/Serabe/ember-bind-helper)
-> for an example and alternatives.
-
 ## How we teach this
 
 Element modifiers will be new to everyone, so we're starting with a mostly blank
@@ -343,14 +360,18 @@ should be seen as the place for any logic which needs to act directly on an
 element, or when an element is added to or removed from the DOM. Modifiers can
 be fully independent (for instance, a `scroll-to` modifier that transparently
 manages the scroll position of the element) or they can interact with the
-component (like the `did-render` and `will-destroy` modifiers). In all cases
+component (like the `did-insert` and `will-destroy` modifiers). In all cases
 though, they are _tied to the render lifecycle of the element_, and they
 generally contain _side-effects_ (though these may be transparent and
 declarative, as in the case of `{{action}}` or the theoretical `{{scroll-to}}`).
 
 Second, we should teach the render modifiers specifically. We can do this by
 illustrating common use cases which can currently be solved with render hooks,
-and comparing them to using modifiers for the same solution.
+and comparing them to using modifiers for the same solution. We should also
+emphasize that these are an addon, not part of the core framework, and are
+useful as solutions for _specific_ problems. As more modifiers become available,
+we should create additional guides that focus on using the _best_ modifier for
+the job, rather than these generic ones.
 
 One thing we should definitely avoid teaching except in advanced cases is the
 _ordering_ of element modifiers. Ideally, element modifiers should be
@@ -381,7 +402,12 @@ export default Component.extend({
 After:
 
 ```hbs
-<div {{did-render (action this.setScrollPosition) @scrollPosition}} class="scroll-container">
+<div
+  {{did-insert this.setScrollPosition @scrollPosition}}
+  {{did-update this.setScrollPosition @scrollPosition}}
+
+  class="scroll-container"
+>
   {{yield}}
 </div>
 ```
@@ -425,7 +451,7 @@ After:
 
 ```hbs
 {{#if shouldShow}}
-  <div {{did-render (action this.fadeIn)}} class="alert">
+  <div {{did-insert this.fadeIn}} class="alert">
     {{yield}}
   </div>
 {{/if}}
@@ -440,21 +466,21 @@ export default Component.extend({
 
 #### Example: Resizing text area
 
-One key thing to know about `{{did-render}}` is it will not rerun whenever the
-_contents_ or _attributes_ on the element change. For instance, `{{did-render}}`
+One key thing to know about `{{did-update}}` is it will not rerun whenever the
+_contents_ or _attributes_ on the element change. For instance, `{{did-update}}`
 will _not_ rerun when `@type` changes here:
 
 ```hbs
-<div {{did-render (action this.setupType)}} class="{{@type}}"></div>
+<div {{did-update this.setupType}} class="{{@type}}"></div>
 ```
 
-If `{{did-render}}` should rerun whenever a value changes, the value should be
+If `{{did-update}}` should rerun whenever a value changes, the value should be
 passed as a parameter to the modifier. For instance, a textarea which wants to
 resize itself to fit text whenever the text is modified could be setup like
 this:
 
 ```hbs
-<textarea {{did-render (action this.resizeArea) @text}}>
+<textarea {{did-update this.resizeArea @text}}>
   {{@text}}
 </textarea>
 ```
@@ -529,7 +555,7 @@ export default NodeComponent.extend();
 ```hbs
 <!-- components/root.hbs -->
 <div
-  {{did-render (action this.didInsertNode)}}
+  {{did-insert (action this.didInsertNode)}}
   {{will-destroy (action this.willDestroyNode)}}
 >
   {{yield (component "node" parent=this)}}
@@ -548,9 +574,6 @@ Usage:
 
 ## Drawbacks
 
-* Element modifiers are a new concept that haven't been fully stabilized as of
-  yet. It may be premature to add default modifiers to the framework.
-
 * Adding these modifiers means that there are more ways to accomplish similar
   goals, which may be confusing to developers. It may be less clear which is the
   conventional solution in a given situation.
@@ -564,24 +587,4 @@ Usage:
 * Stick with only lifecycle hooks for these situations, and don't add generic
   modifiers for them.
 
-* Add an implicit context to modifiers and helpers, instead of relying on users
-  to bind functions manually. Doing this should take into account a few
-  constraints and considerations:
-
-  * Adding an implicit context may make it more difficult to optimize modifiers
-    and helpers in the future. If possible, this should be something they opt
-    _into_, so only helpers which _need_ a context will deoptimize.
-
-  * Binding can be counterintuitive in some cases. For instance:
-
-    ```hbs
-    <button {{action this.service.reloadData}}>Reload</button>
-    ```
-
-    This example will likely error, because the `reloadData` function will be
-    bound to the _component_, not the service. Likewise, binding helpers doesn't
-    really make sense, since they should be pure functions. Solutions like the
-    [`{{bind}}` helper](https://github.com/Serabe/ember-bind-helper) attempt to
-    address this, but may not be something that can be fully rationalized (what
-    happens if there are multiple contexts?)
 

From bfa940f2714f8ca11b85be0a58156959e86c11eb Mon Sep 17 00:00:00 2001
From: Jan Bobisud <me@bobisjan.com>
Date: Sun, 20 Jan 2019 08:55:41 -0800
Subject: [PATCH 7/7] Update text/0000-render-element-modifiers.md

Co-Authored-By: pzuraq <me@pzuraq.com>
---
 text/0000-render-element-modifiers.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/text/0000-render-element-modifiers.md b/text/0000-render-element-modifiers.md
index a5b6add383..a8ecea5296 100644
--- a/text/0000-render-element-modifiers.md
+++ b/text/0000-render-element-modifiers.md
@@ -215,7 +215,7 @@ scroll position of an element:
 ```js
 // helpers/scroll-to.js
 export default function scrollTo() {
-  return (element, scrollPosition) => element.scrollTop = scrollPosition;
+  return (element, [scrollPosition]) => element.scrollTop = scrollPosition;
 }
 ```
 ```hbs