Side navigation
#11230 closed bug (fixed)
Opened January 26, 2012 11:21AM UTC
Closed December 13, 2012 02:27PM UTC
Last modified December 13, 2012 02:38PM UTC
.appendTo .prependTo .insertAfter .insertBefore returns incorrect data for $('not_existing_element')
| Reported by: | yurk | Owned by: | yurk |
|---|---|---|---|
| Priority: | low | Milestone: | 1.9 |
| Component: | manipulation | Version: | 1.7.1 |
| Keywords: | Cc: | ||
| Blocked by: | Blocking: |
Description
$('<h3>3</h3>').appendTo('not_existing_element');
$('<h3>3</h3>').prependTo('not_existing_element');
$('<h3>3</h3>').insertAfter('not_existing_element');
$('<h3>3</h3>').insertBefore('not_existing_element');
expected: return collection with <h3> element
actually: returns an empty collection
Maybe related with http://bugs.jquery.com/ticket/11226
Attachments (0)
Change History (9)
Changed January 26, 2012 11:26AM UTC by comment:1
| owner: | → yurk |
|---|---|
| status: | new → pending |
Changed January 26, 2012 11:55AM UTC by comment:2
Changed January 26, 2012 03:11PM UTC by comment:3
| _comment0: | This one seems to have been around since the dawn of jQuery; I agree that it seems wrong for the set to disappear. Most likely it's *very* rare for anyone to want to continue a chain in that situation. Often I do `var x = $("html").appendTo("body")` but of course the body is always there. :) → 1327590723034546 |
|---|---|
| component: | unfiled → manipulation |
| priority: | undecided → low |
| status: | pending → open |
This one seems to have been around since the dawn of jQuery; I agree that it seems wrong for the set to disappear. Most likely it's *very* rare for anyone to want to continue a chain in that situation. Often I do var x = $("<html stuff>").appendTo("body") but of course the body is always there. :)
Changed November 02, 2012 02:56AM UTC by comment:4
| milestone: | None → 1.9 |
|---|
Okay, studying the docs for appendTo I am starting to think this is working as documented, even if it seems stupid:
If an element selected this way is inserted elsewhere, it will be moved into the target (not cloned) ... If there is more than one target element, however, cloned copies of the inserted element will be created for each target after the first, and that new set (the original element plus clones) is returned.
Reading between the lines on that, when there are *no* elements, no elements are returned. I would expect the original set to always be returned, but that's not what it's documented to do unless there is only 1 element.
Looking at the current code, it's more distressing that it does a pushStack unless there is just 1 element. That means it's impossible to tell whether to use .end() on an arbitrary set.
Because of the latter issue I could be convinced to change the documented behavior here so that it always returns the original set.
Changed December 12, 2012 08:54PM UTC by comment:5
I spent way too much time looking at this, and am convincing myself we need a breaking change. Consider this case, what does it return?
$("<p>hello</p>").appendTo(".status");
It turns out the answer depends on whether .status selects 0, 1, or N elements, whether the incoming set has 0 or 1 elements, and whether the first element in the incoming set is connected to the document or not.
If .status selects nothing, we get a new set that is empty as this ticket shows. If .status selects one element, and there's only one element in the set, we return the original set and don't .pushStack(). Otherwise we return a new set consisting of the original plus all the clones.
And that's the simplified description ... I think I got that right.
I'm proposing that we always return the original set and document this as a breaking change. At least in my use, I am generally using .appendTo() for appending to one element which will have the same behavior in the new world order.
Changed December 12, 2012 09:00PM UTC by comment:6
Should we always return the new set? Otherwise, you can't sanely chain methods. I think the only confusing thing would be that single element .appendTo() would need to .pushStack(), but at least it would be consistent and useful.
Changed December 12, 2012 09:18PM UTC by comment:7
Scott and I talked in IRC ... I can see the case for stacking. My main concern is that we are consistent here, since we already got rid of the maybe-stack-who-knows behavior of .before(), .after() and .replaceWith().
The benefit of consistently stacking the appended elements is that it's somewhat like .clone() in that it selects the clones, although a case like the one here will get you NOTHING because nothing was appended to anything. It's is probably the least likely to break things because it will return the same elements as before, just stacking the one case it didn't used to stack.
The benefit of returning the current set is that it's smaller and faster, plus it won't create a useless stacked set for a case like var markup = $("<div>").appendTo("body") which we certainly use all over the place in unit tests at least.
I sure wish we knew what people were doing out there. It could turn out that these cases are pretty rare, given how inconsistently they currently behave.
Changed December 13, 2012 02:27PM UTC by comment:8
| resolution: | → fixed |
|---|---|
| status: | open → closed |
Fix #11230. .appendTo and pals should always stack.
Changeset: 0018f7700bf8004d573c752ecbf66f299769c4de
Changed December 13, 2012 02:38PM UTC by comment:9
And just to be clear here, the behavior of returning a new empty set is correct, we need to update the docs to clarify this change which was to return a new set when there is only one element and several other special cases.
Thanks for taking the time to contribute to the jQuery project! Please provide a complete reduced test case on jsFiddle to help us assess your ticket.
Additionally, be sure to test against the jQuery Edge version to ensure the issue still exists. To get you started, use this boilerplate: http://jsfiddle.net/FrKyN/ Open the link and click to "Fork" (in the top menu) to get started.