Add an eslint rule to check if users use functions returning live node lists
Categories
(Developer Infrastructure :: Lint and Formatting, enhancement, P3)
Tracking
(Not tracked)
People
(Reporter: lchang, Unassigned)
References
Details
(Whiteboard: [fxperf:p3:responsiveness])
Comment 1•7 years ago
|
||
Updated•7 years ago
|
Comment 2•3 years ago
|
||
I think as this stands, we don't have a good replacement/suggestion for alternatives. Nor do we have any suggestions that we need to work on these/clean these up at this time.
Comment 3•3 years ago
|
||
(In reply to Luke Chang [:lchang] (inactive) from comment #0)
Functions like "getElementsByTagName" return a live node list, which
registers a mutation observer that slows down all of the DOM operations
happening inside the document from that point on.
This sounds terrible for perf, if the observers don't get removed when the collection object gets GC'd (which is what I'd expect, meaning I wouldn't expect such observers to live very long, pretty much "ever", in Firefox front-end use). Olli, can you confirm if there is a lasting impact to perf as soon as anything creates a live DOM node list in a doc?
If the mutation observer does get removed, I wonder if there's any kind of runtime analysis we could do to check if we leave any lists (and therefore observers) around "long-term" so that mistakes like that get picked up by automated tests... I guess with async functions, it's hard to know what is "long-term", one can easily imagine something like:
let labels = doc.getElementsByTagName("label");
let promises;
for (let l of labels) {
promises.push(someAsyncOperation(l));
}
await Promise.all(promises);
but perhaps we should outlaw even that with runtime analysis (as I assume the mutation observer will stick around until after labels
goes out of scope, and because of the await
any code that runs while at least one of the promises is pending is going to be sad...).
(In reply to Mark Banner (:standard8) from comment #2)
I think as this stands, we don't have a good replacement/suggestion for alternatives. Nor do we have any suggestions that we need to work on these/clean these up at this time.
querySelectorAll
does not return a live node list, and is pretty much a drop in replacement (though also, some of the calls in frontend use getElementsByTagName("foo")[0]
so could just use querySelector
in that case!)
Gonna reopen because there's a replacement we can use, and this seems like a valid concern; just curious how severe an issue it is...
Updated•3 years ago
|
Comment 4•3 years ago
|
||
When the live lists go away, they are unregistered as being mutation observers.
https://searchfox.org/mozilla-central/source/dom/base/nsContentList.cpp#405-406,539-540
NodeLists' wrappers are also allocated from the nursery, so that the objects are deleted as soon as possible.
https://searchfox.org/mozilla-central/rev/235982cdb78d0dc3ab4e48326b084ce087302cab/dom/webidl/NodeList.webidl#13
But yes, if the list if kept alive, it has overhead, similar to Range object.
Updated•2 years ago
|
Updated•2 years ago
|
Description
•