Pre-RFC: render helpers (analogous to render modifiers)
buschtoens opened this issue · 13 comments
👉 I implemented this as
ember-render-helpers
RFC #415 Render Element Modifiers introduced the following render modifiers which, are available via the official @ember/render-modifiers package:
{{did-insert fn ...args}}: Activated only when the element is inserted in the DOM.{{did-update fn ...args}}: Activated only on updates to its arguments (both positional and named). It does not run during or after initial render, or before element destruction.{{will-destroy fn ...args}}: Activated immediately before the element is removed from the DOM
These helpers allowed to reduce the API surface area of the new @glimmer/component. The commonly used classic Ember Component hooks (didInsertElement, didReceiveAttrs, willDestroyElement) are replaced with the respective render modifiers.
In my opinion, this works extremely well for {{did-insert}} (didInsertElement) and {{will-destroy}} (willDestroyElement()), which are used to setup and teardown element related state.
There also are very valid applications for {{did-update}}, like this example from the RFC:
export default class Component {
@action
setScrollPosition(element, [scrollPosition]) {
element.scrollTop = scrollPosition;
}
}Another big advantage of {{did-update}} over a generic didReceiveAttrs() / didUpdate() hook is that you explicitly list the arguments you want to observe, whereas the generic hook would be re-evaluated whenever any argument changes. With {{did-update}} you can also observe any other property on the component, whereas the hook only gets called when arguments to the component change.
However, besides all the things {{did-update}} has going for it, I believe that it will often only be used as a workaround for the missing didReceiveAttrs() / didUpdate() hook and that users will not actually use the element that is passed to the fn then. For these scenarios, {{did-update}} as an element modifier is just a hack. It also forces you to render elements to the DOM, which is not an option for "renderless" components that only {{yield}} state.
To better support these scenarios, I think we should provide complimentary template helpers, that behave exactly the same way, except for that they don't pass an element to fn.
For {{did-insert}} and {{did-update}} this should be easily achieved with the public classic Ember Helper API. For {{will-destroy}} I don't think that it'll be possible with the public API.
{{did-insert}}
import Helper from '@ember/component/helper';
import { assert } from '@ember/debug';
export default class DidInsertHelper extends Helper {
didRun = false;
compute(positional: any[], named: Record<string, any>): void {
const fn = positional[0] as (positional: any[], named: typeof named) => void;
assert(
`\`{{did-insert fn}}\` expects a function as the first parameter. You provided: ${fn}`,
typeof fn === 'function'
);
if (this.didRun) return;
this.didRun = true;
fn(positional.slice(1), named);
}
}{{did-update}}
import Helper from '@ember/component/helper';
import { assert } from '@ember/debug';
export default class DidUpdateHelper extends Helper {
didRun = false;
compute(positional: any[], named: Record<string, any>): void {
const fn = positional[0] as (positional: any[], named: typeof named) => void;
assert(
`\`{{did-insert fn}}\` expects a function as the first parameter. You provided: ${fn}`,
typeof fn === 'function'
);
if (!this.didRun) {
this.didRun = true;
return;
}
fn(positional.slice(1), named);
}
}{{will-destroy}}
Assuming that willDestroy is called for instances of Helper, which I am not sure about.
import Helper from '@ember/component/helper';
import { assert } from '@ember/debug';
export default class DidUpdateHelper extends Helper {
fn?: (positional: any[], named: typeof named) => void;
positional?: any[];
named?: Record<string, any>;
compute(positional: any[], named: Record<string, any>): void {
const fn = positional[0] as ;
assert(
`\`{{did-insert fn}}\` expects a function as the first parameter. You provided: ${fn}`,
typeof fn === 'function'
);
this.fn = fn;
this.positional = positional;
this.named = named;
}
willDestroy() {
if (this.fn && this.positional && this.named)
this.fn.call(null, this.positional, this.named);
super.willDestroy();
}
}I personally don't care much for {{did-insert}} and {{will-destroy}}, but I think {{did-update}} is crucial.
For instance, I cannot update my ember-link addon to the Octane programming model, if I want to keep asserting the arguments that were provided to the <Link> component. It was based on sparkles-component, which still had an didUpdate hook, which I used like:
didUpdate() {
super.didUpdate();
assert(
`You provided '@queryParams', but the argument you mean is just '@query'.`,
!('queryParams' in this.args)
);
// more assertions here...
}I can't use the {{did-update}} element modifier, since the template contains no DOM tags and only {{yield}}s some state.
I noticed a similar thing emerging with {{did-update}} when we updated our codebase to use render modifiers instead of lifecycle hooks. Every now and then you'd end up with a {{did-update}} not directly related to the HTML element it was on, e.g. when you want to run code in response to changes to a specific argument. These generally ended up getting added to the root element of the component's template.
I do worry a little that such patterns encourage users back towards using observer-style patterns. although at least this time they're not synchronous!
The did-update helper does seem like the most relevant one. did-insert and will-destroy are directly linked to element lifecycle, and it may be best to keep it that way, whereas with did-update nothing has actually happened to the DOM element itself in order for it to fire its fn, so in some ways having it as an element modifier at all is an interesting choice.
For a component that cares about lifecycle but has no template I expect the recommendation is to use constructor() and willDestroy().
For instance, I cannot update my
ember-linkaddon to the Octane programming model, if I want to keep asserting the arguments that were provided to the<Link>component. It was based onsparkles-component, which still had andidUpdatehook, which I used like:didUpdate() { super.didUpdate(); assert( `You provided '@queryParams', but the argument you mean is just '@query'.`, !('queryParams' in this.args) ); // more assertions here... }I can't use the
{{did-update}}element modifier, since the template contains no DOM tags and only{{yield}}s some state.
In the meantime I did manage to update ember-link. I went with creating an extra validation template helper:
Unfortunately, this pollutes the global namespace. However, once template imports come along, this won't be an issue any more. To me it still feels a bit weird to off-load all this to the template, but I am trying align my brain with the "the template is the driver of the program / state" mentality.
Even when template imports land, I still think that {{did-update}} is a worthwhile helper to have. I am totally open to have my mind changed though.
Tangentially related: I made ember-on-helper, which is the helper equivalent of the {{on}} modifier. It allows you to pass an arbitrary EventTarget:
I needed it in order to bind a global keydown listener on window.
In my opinion, a {{did-update}} helper makes much more sense than a modifier in most use cases. In the following example, I've implemented the {{did-update}} helper proposed by @buschtoens to solve the clear problem with listening to args mutations when there's no top-level DOM element.
// components/overlay.js
import Component from '@glimmer/component'
import { tracked } from '@glimmer/tracking'
import { action } from '@ember/object'
export default class OverlayComponent extends Component {
// @visible?: boolean
// @destinationElement?: HTMLElement
@tracked visible = this.args.visible
@action
didUpdateVisible () {
this.visible = this.args.visible
}
@action
close () {
this.visible = false
}
}In the current programming model, what's the alternative? I tried using the following in the constructor, but that didn't track mutations:
addObserver(this.args, 'visible', this.didUpdateVisible.bind(this))I just published ember-render-helpers. 🌟😊
This would be a very welcome addition for renderless / provider components. E.g. you had to use an arbitrary, invisible element to which you bound a modifier that updates back to your component. So, elements are not needed anymore.
before:
after:
I still think {{did-insert}} still makes sense as modifier as it establishes the connection between html element and component. However, once you have the element reference in your component, you can use the willDestroy() hook from the component. If you do use handlbars properly I do not even see a need for {{will-destroy}} helper or modifier, unless you kill the element with javascript before the component is destroyed anyway (which you also should not do).
I also question the {{did-insert}} helper, that's stuff that can be done in the constructor of your component, unless there are timing constraints I cannot evaluate properly at the moment.
I personally don't care much for
{{did-insert}}and{{will-destroy}}, but I think{{did-update}}is crucial.
There are only very few cases where {{did-update}} as a modifier makes sense. And it's the other way around with {{did-insert}} and {{will-destroy}} as helpers.
However, if you imagine a world where templates are not necessarily backed by a component class instance, they might become more useful.
Either way, now we have full feature parity in both directions. Just because something is there, doesn't mean that you have to use it. And luckily, with embroider on the horizon, unused helpers and modifiers will be stripped from the build. 🙌
@willviles I think that pattern should be discouraged in general. That was actually a major reason why the lifecycle hooks were removed, and a major discussion point on the Glimmer Components RFC. Fundamentally, that pattern is better rewritten as a getter based on derived state, rather than manually, imperatively updating state based on observed changes.
I think in that example, this is pretty clear. If you have a more nuanced/difficult example that you're struggling with, I'd love to dig in, we're building out guides for this type of thing right now. Totally understand that in many cases, it's going to be difficult or counterintuitive to figure out the path forward, so we want to help 😄 I actually misread the example, it is a good one! I'll add a followup that digs in.
It's also probably important to note, in general we did not expect the render modifiers to be the recommended path forward in the future. They were seen as a way to unblock users who are converting to Octane and who have lifecycle-hook heavy apps, an escape-hatch essentially. In the long run, we wanted to push users toward either deriving state using autotracking, or writing modifiers specifically designed for solving targeted use cases. I'd say the same thing about these helpers if we make them (and I definitely think we should! They're also a valuable escape-hatch).
There are only very few cases where {{did-update}} as a modifier makes sense. And it's the other way around with {{did-insert}} and {{will-destroy}} as helpers.
I'm wondering if this is an indicator that the design of the escape hatches is off. {{did-update}} always did feel a bit awkward, but the idea was that it was one of the base modifier lifecycle hooks so it would be a 1-1 mapping, and would let people write logic that could later be extracted to a custom modifier.
There was an initial proposal for a {{did-render}} modifier, that would run on insertion and also update. I'm wondering if this might be more ergonomic, and help users "think in modifiers" like we're aiming for.
Followup to @willviles example (which I misread in my previous comment, edited above). Let's say we have the following as our component today:
// components/overlay.js
import Component from '@ember/component';
export default Component.extend({
visible: false,
destinationElement: null,
actions: {
close () {
this.set('visible', false)
}
}
}This should generally work pretty well. When we initially toggle this.overlayVisible in our surrounding context to true, the overlay will begin showing. The close action that we pass through will also allow the overlay to toggle itself off. Awesome 😄
Note: This would also activate Ember's old 2-way-binding system, causing the parent's value,
isVisible, to toggle back to false as well. We'll revisit this in a bit.
When we first try to convert this to Glimmer Components, we run into an issue with the visible argument though. It's no longer mutable:
// components/overlay.js
import Component from '@glimmer/component';
import { action } from '@ember/object';
export default class Overlay extends Component {
@action
close () {
this.args.visible = false; // this will throw an error
}
}So, what do we do here? The issue is that we're using a single class field for state that is both internal to this component, and external to it, in the surrounding context. So, one option we could try is to make a getter that combines internal component state and external argument state:
// components/overlay.js
import Component from '@glimmer/component';
import { tracked } from '@glimmer/tracking';
import { action } from '@ember/object';
export default class Overlay extends Component {
@tracked _visible = true;
get visible() {
return this.args.visible && this._visible;
}
@action
close () {
this.visible = false;
}
}This will work at first! However, when we toggle the overlay off, we'll find that it's never possible to turn it back on. It's internal state is now frozen. This is what led to the didUpdateVisible solution from the previous example:
// components/overlay.js
import Component from '@glimmer/component'
import { tracked } from '@glimmer/tracking'
import { action } from '@ember/object'
export default class OverlayComponent extends Component {
// @visible?: boolean
// @destinationElement?: HTMLElement
@tracked visible = this.args.visible
@action
didUpdateVisible () {
this.visible = this.args.visible
}
@action
close () {
this.visible = false
}
}This works, but it definitely feels a bit strange. We have an action that is called from the template, that runs when the parent component changes a value, and then updates the state of our component. This is a codesmell in general, and the issue it's pointing to is that we have a mismatch in state management. This component is trying to treat the same piece of state, visibility, as if it was both local state and external state.
This is going to cause headaches in the long run, since the developer must manually keep local state in sync with external state whenever it changes. For example, since Glimmer Components are not 2-way data bound, there's no way for parents to know about child state changes, so for instance if this was how we activated the overlay in the parent:
if (!this.isVisible) {
this.isVisible = true;
// do some other setup
}Then we would be back in the same boat as the previous example, where the overlay would never show again after it was closed the first time.
So, how do we fix this, in a way that is not error prone? The answer is to always mutate the state where it originates, in this case in the parent component. We can update the Overlay component to receive a @close argument which toggles the isVisible value in the parent context, instead of in this component:
The end result is actually a Template-Only component, since we're just passing arguments through.
There are definitely times when this type of refactor will be a bit painful, and it can be tricky to figure out if something is local state, or if it belongs to the parent. I'm hoping we can get more examples so it'll be easier for users to figure out how to update their components, we're planning on building up a library of them in the Ember Atlas (I'll be adopting this one shortly!)
While writing a brand new app in all-Octane has been very pleasant so far, this specific issue mentioned here has continued to be rather weird to me. I very much second the sentiment that {{did-insert}} and {{will-destroy}} make a lot of sense and feel great to use as modifiers, but {{did-update}} doesn't really make a lot of sense to me in most cases. I end up just adding it to a random DOM element, which is not even possible in some scenarios.
As an example, we have a component that loads some data from an API. We use a task to load the data, and there are some arguments passed into the components that are used for the data loading, so if they change we want to trigger a reload. It would be hard to rewrite this with a getter, especially if you're at the same time trying to avoid juggling with promises everywhere.
Here is a simplified example:
export default class MyComponent extends Component {
@tracked items = [];
constructor() {
super(...arguments);
this.loadData.perform();
}
@restartableTask
*loadData() {
// Imagine this depends on some args...
let items = yield fetch(...);
this.items = items;
}
}Without adding an unnecessary wrapper div, this is not really possible at all. With helpers, it is possible, but honestly, it feels weird to put this into the template at all, as this data loading is not really a template concern. I do appreciate the manual specifying of the values that should be watched, though. Still, a helper makes 100% more sense to me for {{did-update}} than a modifier.
@mydea FWIW, the idea when we designed modifiers and removed lifecycle hooks was that in the long run, we should ideally develop helpers that allow us to consume data like tasks without manually restarting them. So for example, ideally your sample code would update the promise automatically if args changed, no need to call loadData.perform() at all.
Some exploration in this space has been done with libraries like async-await-helpers, and I think that a consumption type API would make more sense for most of these use cases where args are triggering promises.
Essentially, the goal is to take some imperative, side-effecting type code (like making an async request for data), and wrap it for usage in a declarative way, where the state is derived directly from incoming arguments and not necessarily manually called or started. ember-concurrency almost does this, and it's a great example of the benefits, but I think we can iterate to something just a bit better.
render-modifiers were seen as a temporary stepping stone during the design discussion. I think render-helpers are also useful in this period, as we build out/explore new models as a community, but I think long term we should be trying to build a better abstraction here.
Does this addon/experiment by @pzuraq solve this issue? If so, maybe we can close this issue?
https://github.com/pzuraq/ember-could-get-used-to-this
Yes, definitely! ✅