First draft of component hook argument deprecation guide #2765
Conversation
You can see that Godfrey used |
There was a problem hiding this comment.
Great first draft, some comments that need addressing. On a macro level, I was thinking if maybe we should shift the organization a bit. The difference between didReceiveAttrs and didUpdateAttrs is when they run, so maybe we can coalesce them into the same header? What's important, IMO, is to address the use cases people used the arguments for, as seen in the discussion for emberjs/rfcs#191.
| ##### until: 2.13.0 | ||
| ##### id: ember-views.lifecycle-hook-arguments | ||
|
|
||
| It is possible for component lifecycle hooks `didInitAttrs`, `didReceiveAttrs`, and `didUpdateAttrs` to receive arguments, but this functionality is part of private API, which is subject to change without notice. Those arguments will be removed because their use is harmful to component performance. However, since the arguments were accessible and some apps may be unknowingly depending on private API, this change will follow the formal deprecation process. Alternative approaches for all three hooks are below: |
There was a problem hiding this comment.
Since this documentation will be post-deprecation, I think we should slightly re-word this. Something in the spirit of http://emberjs.com/deprecations/v2.x/#toc_initializer-arity, maybe.
|
|
||
| ``` javascript | ||
| Ember.Component.extend({ | ||
| didReceiveAttrs({ highTemp, lowTemp }) { |
There was a problem hiding this comment.
You can't change this, or the destructuring won't work. didReceiveAttrs receives an object with oldAttrs and newAttrs attributes, so:
didReceiveAttrs({ oldAttrs, newAttrs }) {
}Is equivalent to:
didReceiveAttrs(attrObj) {
let oldAttrs = attrObj.oldAttrs;
let newAttrs = attrObj.newAttrs;
}Did you mean to destructure oldAttrs/newAttrs themselves, like didReceiveAttrs({ newAttrs: { lowTemp, highTemp }}) {?
| } | ||
| }); | ||
| ``` | ||
| After: |
There was a problem hiding this comment.
Can you add an empty line above "After:"?
|
@jenweber let me know if you need help resolving the deprecation |
|
Thanks @locks. Just got back from the holidays and will update the PR tonight. |
|
@locks let me know if anything else should be changed, or if I should resolve the conflicts too. Thanks! |
|
@locks I think it's in the right place now. But I'm a bit confused by the "files changed" and "commits" associated with the PR (particularly some deletions that I didn't make and don't show up elsewhere), so please look them over and let me know if that looks normal. I haven't really worked on forks of actively developed projects. |
@locks here is the PR to address #2761
I mostly used your examples from the RFC. I changed the "map" example to "thermometer" because when I read map, my mind jumps to enumerating through arrays. I don't think about navigational maps. I simplified what happens inside the component hooks. I see why there were more lines in terms of a real-world example but I think keeping it as simple as possible may help the reader to hone in on what's different between the before and after.
I am also wondering what the best heading level is for each hook. They don't stand out well but they also should not be as prominent as the label.
What's the official deprecation name? I made one up.
Thanks for walking me through the process!