Skip to content
This repository has been archived by the owner on Mar 22, 2019. It is now read-only.

Add deprecation guide for {{render helper #2700

Merged
merged 1 commit into from
Nov 28, 2016
Merged

Add deprecation guide for {{render helper #2700

merged 1 commit into from
Nov 28, 2016

Conversation

josemarluedke
Copy link
Sponsor Contributor

This adds the deprecation guide for emberjs/ember.js#14441.

This guide could be expanded and reworded with some feedbacks.

@ErikCH has shown interest in taking this documentation, but I wanted to get at least a basic documentation for this deprecation. Anyone else with more experience writing deprecations and using the {{render helper is welcome to take this further.

@locks locks self-assigned this Oct 15, 2016
@josemarluedke
Copy link
Sponsor Contributor Author

I have added more guide for the deprecation of outlet orphaning (route this.render({into: ...})) requested here: emberjs/ember.js#14441 (comment)

@josemarluedke
Copy link
Sponsor Contributor Author

@locks could you take a look on this one? Since emberjs/ember.js#14441 has been merged and the the release of Ember 2.10 is coming soon, I think we should have a look at merging it or doing the required changes.

##### id: ember-template-compiler.deprecate-render

Using the `{{render` helper is deprecated in favor of using components.
Please refactor to a component and invoke thusly:
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and invoke thusly: followed by another line with a : seems odd. Perhaps just "Please refactor uses of this helper to components".

});
```

Would be refactored to:
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example, if you had:

Would be refactored to:

These fragments should read as a sentence. "For example, if you had:" "You would refactor to a component like so:"


```hbs
{{! app/templates/sidebar.hbs }}
<div class="sidebar">{{outlet}}</div>'
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra ' on this line


```hbs
{{! app/templates/application.hbs }}
{{render "sidebar"}}
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use single ' for JS and HBS, " for HTML content. {{render 'sidebar'}}

this.render({ into: 'sidebar' });
},
actions: {
disconnect: function() {
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shorthand function syntax was used for renderTemplate, the same should be used here. disconnect() {

##### until: 3.0.0
##### id: ember-routing.top-level-render-helper

For example, if you had:
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to explain what the functionality is before giving an example. I expect most developers are not familiar with this functionality, and if you don't explain what it is you'll give them palpitations trying to guess if their codebase needs to change :-)

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I do agree with you on this. However, I unfortunately don't have much knowledge of this feature to write about it. If anyone can stop by and take a look, it would be great.

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@josemarluedke I don't think anyone is very familiar with this API :-) Hence the strooong motivation to remove it :-)

I took a pass at an updated section in this gist. Please give it a review, I may have missed something. It should be a good start to replace this section though, if you can merge that draft into this after review that seems good 👍

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is perfect @mixonic! Thank you for getting that gist. I have applied your suggestions in this PR. Should be way better now.

// app/components/my-sidebar.js
export default Ember.Component.extend({
});
```
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we list some common gotchas of making this migration? For example:

Note that the render helper has several unique behaviors that may require further refactoring work during migration to a component.

  • When using the render helper with no model argument, the controller instance is a singleton. For example the same controller instance is shared between {{render 'post'}}, any other helper usage of {{render 'post'}}, a route template named post, and dependency injections using Ember.inject.service('post').
  • When sendAction is called in a rendered controller, or when {{action is used in a render helper template, the bubbling target for those actions is the router and current active route. With components, those same actions would target only the component instance without bubbling.

What else?

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

@mixonic
Copy link
Sponsor Member

mixonic commented Nov 28, 2016

@josemarluedke Thanks for bringing this up again. I've left a number of comments to tighten things. Do try to think of these docs from the perspective of a possibly overwhelmed developer who didn't keep track of the discussions we've had here on Github. You should provide some small amount of motivation for the change, liberally link back to discussions on GitHub you think are relevant, and try to provide as much actionable information as is possible.

@josemarluedke josemarluedke changed the title Add deprecation guide for {{render heper Add deprecation guide for {{render helper Nov 28, 2016
@mixonic
Copy link
Sponsor Member

mixonic commented Nov 28, 2016

@josemarluedke thanks :-D 👏

@mixonic mixonic merged commit 1c0e8ba into emberjs:master Nov 28, 2016
@josemarluedke josemarluedke deleted the deprecate-render-helper branch November 28, 2016 23:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants