Repository navigation
Drop memoized queries when a node changes document - #4246
Conversation
getElementsByTagName bakes the document's HTML-ness into the collection it returns, which is what the spec wants for a collection that already exists. The memoized copy was handed back after the node moved to a document of the other kind, so a fresh call kept the old lowercasing behaviour. Co-authored-by: Claude Opus 5 <[email protected]>
domenic
left a comment
domenic
left a comment
There was a problem hiding this comment.
Nice work, although it could be better in a couple ways.
|
|
||
| // Collections built before the move captured the old document's HTML-ness, and the spec | ||
| // keeps those working. A fresh call has to see the new document, so drop the memoized ones. | ||
| inclusiveDescendant._memoizedQueries = {}; |
There was a problem hiding this comment.
| inclusiveDescendant._memoizedQueries = {}; | |
| inclusiveDescendant._memoizedQueries = Object.create(null); |
Bonus points if you also fix
jsdom/lib/jsdom/living/nodes/Node-impl.js
Line 254 in b23f10d
| inclusiveDescendant._ownerDocument = newDocument; | ||
|
|
||
| // Collections built before the move captured the old document's HTML-ness, and the spec | ||
| // keeps those working. A fresh call has to see the new document, so drop the memoized ones. |
There was a problem hiding this comment.
Add a comment explaining why we don't use _clearMemoizedQueries().
|
Adding the spec text this rests on, since I should have quoted it up front. "The list of elements with qualified name qualifiedName for a node root" picks its filter once, from whether root's node document is an HTML document, and the collection keeps that filter. The caching allowance right below it is the part jsdom was over-reaching:
Collections already handed out are deliberately left alone, which the same test checks in its first two assertions. |
Co-authored-by: Claude Opus 5 <[email protected]>
|
All three done —
|
getElementsByTagNamebakes the document's HTML-ness into the collection it returns, and per the spec a collection that already exists keeps behaving that way after its root moves to a document of the other kind. A new call has to reflect the new document, but the memoized collection was handed back instead, so it kept lowercasing.Adoption now drops the memoized queries on the nodes it moves. Collections already handed out are untouched, which is what
Element-getElementsByTagName-change-document-HTMLNess.htmlchecks.Note this touches the same loop in
_adoptNodeas #4245, so whichever lands second will need a trivial rebase.Motivation: still working through the failures recorded in
to-run.yamlrather than hitting this in an application.