From ff3e8bd4ab290e3e7c83bfc6e6a6b3c17d0632b4 Mon Sep 17 00:00:00 2001 From: Ricardo Mendes Date: Wed, 14 Dec 2016 08:58:57 -0800 Subject: [PATCH 1/3] Deprecate component lifecycle hook arguments. --- ...deprecate-component-lifecycle-hook-args.md | 139 ++++++++++++++++++ 1 file changed, 139 insertions(+) create mode 100644 text/0000-deprecate-component-lifecycle-hook-args.md diff --git a/text/0000-deprecate-component-lifecycle-hook-args.md b/text/0000-deprecate-component-lifecycle-hook-args.md new file mode 100644 index 0000000000..53a2f0e712 --- /dev/null +++ b/text/0000-deprecate-component-lifecycle-hook-args.md @@ -0,0 +1,139 @@ +- Start Date: 2016-12-14 +- RFC PR: (leave this empty) +- Ember Issue: (leave this empty) + +# Summary + +We would like to deprecate and remove the **arguments** passed to the `didInitAttrs`, `didReceiveAttrs` and `didUpdateAttrs` component lifecycle hooks. These arguments are currently undocumented on purpose and considered a private API, imposes an unnecessary performance hit on *all* components whether they are used or not, and can be easily replicated by the users in cases where they are needed. + +# Motivation + +In the road leading up to Ember.js 2.0, [new lifecycle hooks](http://emberjs.com/blog/2015/06/12/ember-1-13-0-released.html#toc_component-lifecycle-hooks) were introduced to components in order to help users shift to a new mental model, dubbed Data Down Actions Up. The hooks were introduced by name, and their semantics explained, but there were no mentions of possible arguments received by them. + +This lack of documentation for lifecycle hook arguments was deliberate. The hooks were introduced as an experiment with an eye to the then-upcoming angle bracket components, so the arguments to the hooks were considered private by the framework maintainers, as their design was still ongoing. + +However, references to the lifecycle hook arguments started appearing in community resources. Users started betting on these arguments as the way forward, which in conjunction with the lack of an RFC process and clear messaging from the Ember.js maintainers lead to confusion. + +This left the core team in a difficult position. Despite no longer endorsing lifecycle hook arguments, trying to communicate such could have the reverse effect by pointing a spotlight at them. The purpose of this RFC is then to clarify that lifecycle hook arguments have no future in the framework, and you should update your code to not make use of them. + +The reason to officially deprecate lifecycle hook arguments is not only about messaging, but also because providing these arguments imposes an unnecessary performance penalty to every component in your application even if the arguments are not used. + +To provide the arguments to the lifecycle hooks, Ember.js has to eagerly "reify" and save-off any passed-in attributes to allow diffing and construct several wrapper objects. In the few occasions where this logic is actually necessary, developers should be able to use programmatic patterns familiar to them and manually track changes as needed, as exemplified in the Transition Path section below. + +# Transition Path + +The transition path followed will be the standard one, which encompasses using the deprecation API to deprecate the feature and the related deprecation guide. While the lifecycle hooks share a deprecation identifier, they will be addressed in turn. + +### `didInitAttrs` + +Since this lifecycle hook is [already deprecated](http://emberjs.com/deprecations/v2.x/#toc_ember-component-didinitattrs), we suggest taking this chance to address two deprecations at the same time. Imagine you have a component that stores a timestamp when it's initialized for later comparison. + +Before: + +``` javascript +Ember.Component.extend({ + didInitAttrs({ attrs }) { + this.set('initialTimestamp', attrs.timestamp); + } +}); +``` +After: + +``` javascript +Ember.Component.extend({ + init() { + this._super(...arguments); + + this.set('initialTimestamp', this.get('timestamp')); + } +}); +``` +### `didReceiveAttrs` + +Let's say you want to animate a map widget from the old coordinates to the new coordinates. + +Before: + +``` javascript +Ember.Component.extend({ + didReceiveAttrs({ oldAttrs, newAttrs }) { + if (oldAttrs && oldAttrs.coordinates !== newAttrs.coordinates) { + this.map.move({ from: oldAttrs.coordinates, to: newAttrs.coordinates }); + } + } +}); +``` +After: + +``` javascript +Ember.Component.extend({ + didReceiveeAttrs() { + let oldCoordinates = this.get('_previousCoordinates'); + let newCoordinates = this.get('coordinates'); + + if (oldCoordinates && oldCoordinates !== newCoordinates) { + this.map.move({ from: oldCoordinates, to: newCoordinates }); + } + + this.set('_previousCoordinates', newCoordinates); + } +}); +``` +### `didUpdateAttrs` + +This hook is very similar to `didReceiveAttrs`, except it only runs on re-renders and not the initial render. + +Before: + +``` javascript +Ember.Component.extend({ + didUpdateAttrs({ oldAttrs, newAttrs }) { + if (oldAttrs && oldAttrs.coordinates !== newAttrs.coordinates) { + this.map.move({ from: oldAttrs.coordinates, to: newAttrs.coordinates }); + } + } +}); +``` +After: + +``` javascript +Ember.Component.extend({ + didUpdateAttrs() { + let oldCoordinates = this.get('_previousCoordinates'); + let newCoordinates = this.get('coordinates'); + + if (oldCoordinates && oldCoordinates !== newCoordinates) { + this.map.move({ from: oldCoordinates, to: newCoordinates }); + } + + this.set('_previousCoordinates', newCoordinates); + } +}); +``` +# How We Teach This + +Due to the previous undocumented nature of the arguments, there is no official documentation that will require updating deprecated usage. + +As required for framework deprecations, there will be a deprecation guide written up and linked from within the deprecation message. This deprecation guide will address the more common usage patterns associated with lifecycle hook arguments, such as the Transition Path example. + +Additionally, the usage patterns present in the deprecation guide could also be documented in the component section of the official Guides, as a proactive approach for teaching newcomers. + +# Drawbacks + +One immediate drawback of this proposal is that due to references to the arguments in community resources, there are uses of them in the wild. Updating deprecated code will have to be done mostly manually, as automation might prove difficult. + +Another drawback is that by the very nature of publishing this RFC, attention will be drawn to the arguments. It is our hope that this increase of awareness will be a net positive due to the clear guidance gained by users of the framework. + +It is then our assessment that these drawbacks are outweighed by the benefits of the change. + +# Alternatives + +There are two standout alternatives to the proposal presented here which are doing nothing, or making the arguments public and supporting them going forward, both of which are less than ideal for reasons stated previously. + +Doing nothing would perpetuate the confusion surrounding lifecycle hook arguments. While it might be argued that that ship has sailed, we prefer to think that it's never too late to provide users of the framework with clearer messaging regarding usage of certain features. + +Making the arguments public and supported would mean supporting APIs that did not go through the RFC process, meaning they do not align with some of the current values of the framework, nor would iteration on them would be possible without introducing breakage. Additionally, there are some performance penalties to supporting these arguments, mentioned in the Motivation section. + +# Unresolved questions + +None. From 57a203723d6ecf3a26b8946b165580a531c8fde6 Mon Sep 17 00:00:00 2001 From: Ricardo Mendes Date: Thu, 15 Dec 2016 02:31:37 +0000 Subject: [PATCH 2/3] Update 0000-deprecate-component-lifecycle-hook-args.md --- text/0000-deprecate-component-lifecycle-hook-args.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-deprecate-component-lifecycle-hook-args.md b/text/0000-deprecate-component-lifecycle-hook-args.md index 53a2f0e712..28a309d927 100644 --- a/text/0000-deprecate-component-lifecycle-hook-args.md +++ b/text/0000-deprecate-component-lifecycle-hook-args.md @@ -122,7 +122,7 @@ Additionally, the usage patterns present in the deprecation guide could also be One immediate drawback of this proposal is that due to references to the arguments in community resources, there are uses of them in the wild. Updating deprecated code will have to be done mostly manually, as automation might prove difficult. -Another drawback is that by the very nature of publishing this RFC, attention will be drawn to the arguments. It is our hope that this increase of awareness will be a net positive due to the clear guidance gained by users of the framework. +Another drawback is that by the very nature of publishing this RFC, attention will be drawn to the arguments. It is our hope that the increased awareness will be a net positive due to the clear guidance gained by users of the framework. It is then our assessment that these drawbacks are outweighed by the benefits of the change. From 083cb7be8d66203c7579a16bf607e15c224c497a Mon Sep 17 00:00:00 2001 From: Ricardo Mendes Date: Thu, 15 Dec 2016 15:31:51 +0000 Subject: [PATCH 3/3] Fixes typo in code block --- text/0000-deprecate-component-lifecycle-hook-args.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-deprecate-component-lifecycle-hook-args.md b/text/0000-deprecate-component-lifecycle-hook-args.md index 28a309d927..3647457148 100644 --- a/text/0000-deprecate-component-lifecycle-hook-args.md +++ b/text/0000-deprecate-component-lifecycle-hook-args.md @@ -67,7 +67,7 @@ After: ``` javascript Ember.Component.extend({ - didReceiveeAttrs() { + didReceiveAttrs() { let oldCoordinates = this.get('_previousCoordinates'); let newCoordinates = this.get('coordinates');