This repository has been archived by the owner on Apr 12, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 27.5k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(jqLite): correctly monkey-patch core jQuery methods
When real jQuery is present, Angular monkey patch it to fire `$destroy` event. This commit fixes two issues in the jQuery patch: - passing a selector to the $.fn.remove method (only fire `$destroy` on the matched elements) - using `$.fn.html` without parameters as a getter (do not fire `$destroy`)
- Loading branch information
Showing
3 changed files
with
72 additions
and
24 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -107,37 +107,38 @@ function camelCase(name) { | |
///////////////////////////////////////////// | ||
// jQuery mutation patch | ||
// | ||
// In conjunction with bindJQuery intercepts all jQuery's DOM destruction apis and fires a | ||
// In conjunction with bindJQuery intercepts all jQuery's DOM destruction apis and fires a | ||
// $destroy event on all DOM nodes being removed. | ||
// | ||
///////////////////////////////////////////// | ||
|
||
function JQLitePatchJQueryRemove(name, dispatchThis) { | ||
function JQLitePatchJQueryRemove(name, dispatchThis, filterElems, getterIfNoArguments) { | ||
var originalJqFn = jQuery.fn[name]; | ||
originalJqFn = originalJqFn.$original || originalJqFn; | ||
removePatch.$original = originalJqFn; | ||
jQuery.fn[name] = removePatch; | ||
|
||
function removePatch() { | ||
var list = [this], | ||
function removePatch(param) { | ||
var list = filterElems && param ? [this.filter(param)] : [this], | ||
fireEvent = dispatchThis, | ||
set, setIndex, setLength, | ||
element, childIndex, childLength, children, | ||
fns, events; | ||
|
||
while(list.length) { | ||
set = list.shift(); | ||
for(setIndex = 0, setLength = set.length; setIndex < setLength; setIndex++) { | ||
element = jqLite(set[setIndex]); | ||
if (fireEvent) { | ||
element.triggerHandler('$destroy'); | ||
} else { | ||
fireEvent = !fireEvent; | ||
} | ||
for(childIndex = 0, childLength = (children = element.children()).length; | ||
childIndex < childLength; | ||
childIndex++) { | ||
list.push(jQuery(children[childIndex])); | ||
element, childIndex, childLength, children; | ||
|
||
if (!getterIfNoArguments || param != null) { | ||
while(list.length) { | ||
set = list.shift(); | ||
for(setIndex = 0, setLength = set.length; setIndex < setLength; setIndex++) { | ||
element = jqLite(set[setIndex]); | ||
if (fireEvent) { | ||
element.triggerHandler('$destroy'); | ||
} else { | ||
fireEvent = !fireEvent; | ||
} | ||
for(childIndex = 0, childLength = (children = element.children()).length; | ||
This comment has been minimized.
Sorry, something went wrong.
This comment has been minimized.
Sorry, something went wrong.
btesser
Contributor
|
||
childIndex < childLength; | ||
childIndex++) { | ||
list.push(jQuery(children[childIndex])); | ||
} | ||
} | ||
} | ||
} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This loop seem to cause performance issues and seems like it can easily be replaced by a more efficient code.
Is there any reason why it can't be written like so?
Why do you need to wrap every child in jQuery just to re-wrap the original element in jqLite (line 131)?
Every time I use .html() to replace a big chunk of code it take roughly 300ms just to go through this loop.