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

Add deprecation guide for renderToElement #2702

Merged
merged 1 commit into from
Nov 13, 2016
Merged

Add deprecation guide for renderToElement #2702

merged 1 commit into from
Nov 13, 2016

Conversation

josemarluedke
Copy link
Contributor

@josemarluedke josemarluedke commented Oct 16, 2016 •
edited
Loading

Deprecation for emberjs/ember.js#14482.

@josemarluedke josemarluedke mentioned this pull request Oct 16, 2016
Merged
@josemarluedke
Copy link
Contributor Author

Note to self: We should explain that both functions are private and no public api is been deprecated here.

@josemarluedke
Copy link
Contributor Author

Added note about both api been private and updated the until to reflect the change in the Ember pr.

rwjblue
rwjblue suggested changes Nov 13, 2016
View reviewed changes
source/deprecations/v2.x.html.md
@@ -729,3 +729,27 @@ export default Router.map(function() {
});
});
```

### Deprecations Added in 2.10
Copy link
Member

Choose a reason for hiding this comment

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

Can you tweak this to 2.11 (it didn't make the 2.10 cycle)?

@rwjblue rwjblue merged commit 4f8a16a into emberjs:master Nov 13, 2016
@rwjblue
Copy link
Member

rwjblue commented Nov 13, 2016

Landed for now (so the URL referenced in Ember is still present), but would still love a followup PR to tweak that version...

@josemarluedke josemarluedke deleted the deprecate-render-to-element branch November 13, 2016 19:14
@rwjblue rwjblue mentioned this pull request Nov 28, 2016
Closed
@chancancode
Copy link
Member

@josemarluedke Thanks for working on this!

I noticed a problem with the deprecation guide – I think the renderToElement usage here is incorrect. The method takes a tag name as argument (e.g. "div"), creates a new orphan element of that type then append the component to it. So I think the comparable replacement is to first create the element you wanted and then pass it to appendTo.

@rwjblue does that sound right to you?

@josemarluedke josemarluedke mentioned this pull request Nov 29, 2016
Merged
@josemarluedke
Copy link
Contributor Author

@chancancode I got a PR opening a quick fix for it. Let me know if that change does make sense.

thewalkingtoast
thewalkingtoast reviewed Dec 14, 2016
View reviewed changes
source/deprecations/v2.x.html.md
component.appendTo('.my-component-wrapper');
```

Note that both APIs are private, so no public API is been deprecated here.
Copy link

Choose a reason for hiding this comment

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

been => being

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you send a PR for that change please?

Copy link

Choose a reason for hiding this comment

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

No

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@locks locks locks left review comments

@thewalkingtoast thewalkingtoast thewalkingtoast left review comments

@rwjblue rwjblue rwjblue requested changes

Assignees
No one assigned
Labels
documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@josemarluedke @rwjblue @chancancode @locks @thewalkingtoast